diff --git a/README.md b/README.md index dadc8ef..97b8851 100644 --- a/README.md +++ b/README.md @@ -12,15 +12,37 @@ book and any similarly-structured PDF), and [docs/progress-log.md](docs/progress-log.md) for a running log of what's been done and what's next. -> **Status**: directory scaffold only — no business logic implemented yet. -> See the build roadmap in `docs/architecture.md` for the phased plan. +Dated planning and audit documents (`docs/v1-delivery-plan.md`, +`docs/rag-rebuild-plan.md`, `docs/current-rag-pipeline-audit.md`, +`docs/answer-experience-implementation-plan.md`) record what was known on +their date and are kept for their reasoning rather than as current status — +this README and `git log` are the better reference for where things stand +today. + +> **Status** (2026-08-11): **live in production at +> [realvuxbaro.me](https://realvuxbaro.me)** — a real RAG chatbot over the +> whole formulary, not a scaffold. What exists and what does not: +> +> | Part | State | +> |---|---| +> | `ingestion/` | Done — 15,100 chunks embedded and loaded into Qdrant `duocthu_v1` | +> | `apps/ai-service/` | Done — live grounded RAG (retrieval, generation, grounding, abstention, citations, traces) | +> | `apps/web/` | Done — chat UI with citation/evidence panel | +> | `apps/api-gateway`, `auth-service`, `user-service`, `chat-service` | **Not built** — `README.md` + `package.json` only | +> | `apps/mobile/` | **Not built** — reserved | +> | `infra/docker/` | Done — this is what production actually runs | +> | `infra/k8s`, `helm`, `terraform`, `argocd` | **Not built yet** — empty scaffold. Still the target (ADR 0002), not abandoned: the plan is the team's self-hosted Gitea + ArgoCD; the current EC2/Compose setup is an interim stopgap | +> +> Because the gateway and auth services do not exist, `apps/web` talks +> **directly** to `apps/ai-service`; there is no authentication layer. See +> the build roadmap in `docs/architecture.md`. ## Directory map ``` apps/ - web/ Next.js frontend - ai-service/ Python FastAPI — RAG orchestration + OpenAI calls + web/ Next.js frontend (also hosts the BFF route the browser calls) + ai-service/ Python FastAPI — RAG orchestration + AWS Bedrock calls api-gateway/ NestJS — public entry point, routes to internal services auth-service/ NestJS — signup/login/JWT user-service/ NestJS — profile/preferences @@ -36,9 +58,59 @@ infra/ docker-compose, k8s/Helm, Terraform, CI docs/ architecture docs and ADRs ``` -## Prerequisites (once implementation starts) +## Prerequisites -- Node.js + pnpm (JS workspace: `apps/web`, `apps/api-gateway`, `apps/auth-service`, - `apps/user-service`, `apps/chat-service`, `packages/*`) +- Node.js + pnpm (JS workspace: `apps/web`, `packages/*`; the NestJS service + directories are unbuilt placeholders) - Python 3.11+ (`apps/ai-service`, `ingestion`) -- Docker (local Postgres/Qdrant/Redis via `infra/docker/docker-compose.yml`) +- Docker (local Postgres + Qdrant via `infra/docker/docker-compose.yml`) +- AWS credentials with Bedrock invoke permission, for anything that generates + an answer. Without them `ai-service` still starts, but every answer abstains + rather than falling back to raw source text. + +## Running it locally + +```powershell +docker compose -f infra\docker\docker-compose.yml up -d postgres qdrant +cd apps\ai-service +python -m migrate +python -m uvicorn main:app --port 8079 # NOT --reload, see below +``` + +```powershell +pnpm install +pnpm --filter web dev # http://localhost:3000 +``` + +`ai-service` needs a populated Qdrant collection to serve answers: it verifies +a `duocthu_v1__manifest` sidecar at startup and refuses to run against a corpus +whose sha/model/dimensions do not match. A fresh machine either restores a +Qdrant snapshot or re-runs `ingestion/` (the latter costs real Bedrock spend). + +> **Prefer a plain restart over `uvicorn --reload` on Windows here.** The +> reloader has been observed serving the previous code after an edit on this +> project, which makes it hard to tell whether a change took effect. + +Tests: `cd apps/ai-service && python -m pytest -q` — 230 pass. `test_api.py` and +`test_live_datastores.py` need Postgres and Qdrant actually running; skip them +with `--ignore` when the stack is down. `apps/web` has **no test setup at all**, +so a green suite says nothing about the frontend — drive it in a browser. + +## Production + +Live at [realvuxbaro.me](https://realvuxbaro.me): a single EC2 `t3.large` +running `infra/docker/docker-compose.prod.yml` (postgres, qdrant, ai-service, +web, Caddy for automatic Let's Encrypt TLS). Bedrock is reached through an IAM +instance role — there are no long-lived AWS keys on the box or in any env file. + +Pushing to `master` deploys: `.github/workflows/deploy.yml` SSHes in, resets to +the pushed commit, rebuilds only `ai-service`/`web`, runs migrations and +health-checks both. Postgres/Qdrant/Caddy are left untouched, so the vector +data survives deploys (it lives in a named volume, not the container). + +This is **interim infrastructure**, not the end state. The intended target is +still the team's self-hosted **Gitea** (company domain) plus their **ArgoCD** +instance, per `docs/adr/0002-argocd-gitops.md` — that work is *not started*, +not cancelled. Until it is deliberately started, the project stays on private +GitHub, and the team's existing `git.vinmec.tech/ai-team/gitops` repository is +reference-only: never push this project into it. diff --git a/apps/ai-service/README.md b/apps/ai-service/README.md index b205ba7..63d73eb 100644 --- a/apps/ai-service/README.md +++ b/apps/ai-service/README.md @@ -1,23 +1,73 @@ # ai-service -FastAPI service for drug resolution, guarded retrieval, printed-page citations, -and PostgreSQL retrieval traces. +FastAPI service holding the entire live RAG path: query understanding, drug +resolution, deterministic section routing, guarded retrieval, grounded +generation, claim/citation verification, printed-page citations, and +PostgreSQL conversation + retrieval traces. In production this is what +`apps/web` calls — there is no gateway in front of it. -Local infrastructure: +## Local infrastructure ```powershell docker compose -f ..\..\infra\docker\docker-compose.yml up -d postgres qdrant python -m migrate -uvicorn main:app --reload +python -m uvicorn main:app --port 8079 ``` -`GET /health` is always available. `POST /v1/rag/query` requires structured -`subject_scope` and `intent`; unknown/non-human/recommendation requests fail -closed. The default `EMBEDDING_PROVIDER=disabled` intentionally leaves the RAG -backend unavailable until the collection and matching query embedder are -configured. +> **Use a plain restart rather than `--reload` on Windows.** The reloader has +> been observed serving the previous code after an edit on this project, so a +> change can appear to have no effect (or appear to work when it has not been +> loaded). Restarting the process avoids the ambiguity. -`EMBEDDING_PROVIDER=local-smoke` is only for local plumbing checks. Its hashing -vectors are deterministic but not semantic and must not be used for retrieval -quality claims. Bedrock is not called by this service and no IAM change is -required. +`GET /health` is always available. `POST /v1/rag/query` is the live agent +endpoint. Structured `subject_scope`/`intent` requests fail closed on +unknown/non-human input. + +## Bedrock is required for real answers + +Since 2026-08-05 the live path calls AWS Bedrock and needs credentials with +invoke permission: + +| Role | Default | Setting | +|---|---|---| +| Query embedding | `cohere.embed-v4:0` | `embedding_provider` (default `cohere-v4`) | +| Answer generation | Bedrock Converse | `generation_provider` (`bedrock-converse` / `bedrock-claude`) | +| Rerank | Cohere rerank | `rerank_enabled` — **off** by default; only affects the similarity fallback, never the deterministic section route | + +Credentials come from the environment (an IAM instance role in production, no +long-lived keys). With no working generator configured, a rejected generation +**abstains** — it must never degrade into dumping raw source text at a +clinician. + +`EMBEDDING_PROVIDER=local-smoke` is only for local plumbing checks. Its +hashing vectors are deterministic but not semantic and must not be used for +any retrieval-quality claim. + +## Corpus coupling + +Startup verifies the `duocthu_v1__manifest` sidecar and **refuses to run** +against a collection whose corpus sha / model / dimensions do not match +(F-05). A machine with an empty Qdrant cannot serve answers until a snapshot +is restored or `ingestion/` is re-run — the latter costs real Bedrock spend. + +## Request budget and timeouts + +A turn makes several sequential Bedrock calls, bounded by +`max_wall_clock_ms` (40s) and `max_llm_calls_per_turn` (8) in `config.py`. +The budget is checked **between** calls and cannot cancel one already in +flight, which is separately bounded by `read_timeout` in +`adapters/bedrock_converse.py`. Real ceiling ≈ 40s + one in-flight call, so +any client calling this service must allow more than that — `apps/web`'s +`ChatPanel.tsx` uses 65s for exactly this reason. + +## Tests + +```powershell +python -m pytest -q # needs Postgres + Qdrant up +python -m pytest -q --ignore=tests/test_api.py --ignore=tests/test_live_datastores.py +``` + +230 pass without live datastores. Coverage is uneven and worth checking +before relying on it: `rag/agent.py`'s dosing state machine has been covered +only since 2026-08-11, and the repo has no frontend test setup, so a green +suite does not by itself confirm a user-visible change works. diff --git a/apps/ai-service/rag/agent.py b/apps/ai-service/rag/agent.py index 3c012da..4e75717 100644 --- a/apps/ai-service/rag/agent.py +++ b/apps/ai-service/rag/agent.py @@ -259,12 +259,27 @@ class RagAgent: if frame.population in {"tre_em", "tre_so_sinh"} and ( frame.age_text is None or frame.weight_kg is None ): + # Both fields stay required. The formulary branches pediatric + # dosing on BOTH — paracetamol prints an age band ("Trẻ em + # 4-6 tuổi: 240 mg") *and* a weight rule ("10-50 kg: 15 + # mg/kg") — so answering with only one of them would mean + # picking a regimen the source does not let us pick. + # + # What changed on 2026-08-11 is the *question*, not the gate. + # The fallback used to ask for age and weight every time, + # including for whichever field the user had already given + # (reproduced 5/5 live: "Bé 18 ký ...", "Bé nặng 18 kg ...", + # "Trẻ 5 tuổi ..." all received the same sentence). The + # effect was most visible when understanding.py did well: a + # frame that parsed the weight and set needs_clarify=false + # reaches this fallback, so a clearer question from the user + # produced a more redundant question back. return AgentReply( "clarify", "missing_pediatric_age_or_weight", clarification=( frame.clarify_reason if frame.needs_clarify and frame.clarify_reason - else "Bé bao nhiêu tuổi và cân nặng bao nhiêu kg?" + else _pediatric_clarify_question(frame) ), drugs=frame.drugs, turn_type=tt, quick_replies=( @@ -498,6 +513,32 @@ def _display_name(drug_id: str) -> str: return drug_id.replace("_", " ").title() +def _pediatric_clarify_question(frame: QueryFrame) -> str: + """Ask for the pediatric field that is actually missing. + + The caller still requires both age and weight; this only stops the + question from asking for something the user already supplied in the very + same sentence, which reads as not having been listened to and invites + them to repeat themselves into the clarify-loop breaker. + + When a field IS known it is echoed back, so the user can see the value + was received and correct it if the parse was wrong (e.g. a colloquial + "18 ký" read as 18 kg). + """ + has_age = frame.age_text is not None + has_weight = frame.weight_kg is not None + if has_age and not has_weight: + return f"Bé {frame.age_text} nặng bao nhiêu kg?" + if has_weight and not has_age: + return f"Bé nặng {_format_kg(frame.weight_kg)} kg, vậy bé bao nhiêu tuổi?" + return "Bé bao nhiêu tuổi và cân nặng bao nhiêu kg?" + + +def _format_kg(weight_kg: float) -> str: + """Render a weight without a trailing '.0' on whole kilograms.""" + return f"{weight_kg:g}" + + def _is_section_overview(turn: str, frame: QueryFrame) -> bool: """Separate a handbook survey from a patient-specific decision. diff --git a/apps/ai-service/rag/answer.py b/apps/ai-service/rag/answer.py index 3a0db56..48f5727 100644 --- a/apps/ai-service/rag/answer.py +++ b/apps/ai-service/rag/answer.py @@ -22,7 +22,13 @@ from .routing import QueryRoutingService # Repeating the same temperature-0 prompt against the same model is a # correlated retry, not an independent vote. One fail-closed semantic pass is # kept after structured-claim parsing and deterministic grounding. -_ENTAILMENT_MAX_ATTEMPTS = 1 +# +# This is expressed as a single call rather than a `range(1)` loop on +# purpose: the loop this replaced returned on its first iteration on every +# path, so raising the constant looked like it added retries while silently +# doing nothing. If a future change genuinely wants more passes, they must +# be different prompts (or a different judge) to be independent evidence — +# see this function's own reasoning above. _QUICK_REPLY_MAX_ITEMS = 4 _QUICK_REPLY_MAX_CHARS = 40 @@ -112,6 +118,29 @@ class GroundedAnswer: plan: AnswerPlan | None = None +@dataclass(frozen=True) +class _CheckNotRun: + """The entailment judge could not be consulted at all. + + Distinct from a negative verdict on purpose. Fail-closed behaviour is + identical either way — the answer is still discarded — but the reason + code should not report an unsupported claim when the check never ran. + `docs/current-rag-pipeline-audit.md` §4's failure taxonomy keeps + availability and content failures separate for this reason. Observed + live 2026-08-11: a request that ran out of wall-clock budget mid- + verification reached the user as "bước đối chiếu chưa xác nhận được câu + trả lời khớp với nguồn", which describes the answer rather than the + timeout that actually occurred. + + `reason` is deliberately one of the codes that already exist and are + already mapped in `apps/web/app/api/chat/route.ts`; a code with no entry + there falls back to wording that reads as "no data in the formulary", + which would misdescribe these cases. + """ + + reason: str + + @dataclass(frozen=True) class _GenOutcome: answer: str | None = None @@ -714,7 +743,15 @@ class GroundedAnswerService: verification = self._verify_entailment( query, attempt.claims, shown_evidence, budget=budget ) - if verification is None or not verification.supported: + if isinstance(verification, _CheckNotRun): + # The judge never ran. Still fail closed, but report why: a + # timeout or outage recorded as "unsupported claim" would sit in + # the content-failure bucket and be hard to spot in metrics. + self._metrics.increment( + metric_names.GENERATION_REJECTED, reason=verification.reason + ) + return _GenOutcome(reject_reason=verification.reason) + if not verification.supported: self._metrics.increment( metric_names.GENERATION_REJECTED, reason="unsupported_claim" ) @@ -737,6 +774,25 @@ class GroundedAnswerService: schema=request.schema, ) repaired = self._attempt_generation(repair_request, budget) + # The repair roughly doubles a turn's model calls, so it is the + # most likely place to run out of wall-clock budget. Observed + # live 2026-08-11 (Isosorbid dinitrat dosage, 40.3s against a 40s + # budget): running out here fell through to `incomplete_answer`, + # which describes the answer as missing source information rather + # than reporting that the repair did not finish. Report the + # availability failure as itself, as the first attempt above + # already does. + if repaired.budget_exhausted: + self._metrics.increment( + metric_names.GENERATION_REJECTED, + reason="request_budget_exhausted", + ) + return _GenOutcome(reject_reason="request_budget_exhausted") + if repaired.outage: + self._metrics.increment( + metric_names.GENERATION_REJECTED, reason="provider_unavailable" + ) + return _GenOutcome(reject_reason="provider_unavailable") if repaired.answer is not None: repaired_report = grounding.verify(repaired.answer, evidence_texts) repaired_verification = self._verify_entailment( @@ -747,9 +803,14 @@ class GroundedAnswerService: repaired.claims, repaired_verification, ) + if isinstance(repaired_verification, _CheckNotRun): + self._metrics.increment( + metric_names.GENERATION_REJECTED, + reason=repaired_verification.reason, + ) + return _GenOutcome(reject_reason=repaired_verification.reason) if ( repaired_report.grounded - and repaired_verification is not None and repaired_verification.supported and repaired_verification.complete ): @@ -766,7 +827,7 @@ class GroundedAnswerService: structured_claims: tuple[tuple[str, tuple[int, ...]], ...], evidence_texts: tuple[str, ...], budget: RequestBudget | None = None, - ) -> _VerificationOutcome | None: + ) -> _VerificationOutcome | _CheckNotRun: """A second, adversarial LLM pass over an answer that already passed `grounding.verify`. @@ -808,49 +869,56 @@ class GroundedAnswerService: return _VerificationOutcome(supported=True, complete=True) request = build_entailment_request(query, claims, evidence_texts) - for _ in range(_ENTAILMENT_MAX_ATTEMPTS): - verdict = self._run_entailment_check( - request, evidence_texts=evidence_texts, budget=budget - ) - if verdict is None: - return None - if ( - verdict.supported - and not verdict.complete - and _missing_is_already_explicit(verdict.missing, structured_claims) - ): - return _VerificationOutcome(supported=True, complete=True) + verdict = self._run_entailment_check( + request, evidence_texts=evidence_texts, budget=budget + ) + if isinstance(verdict, _CheckNotRun): return verdict - return False + if ( + verdict.supported + and not verdict.complete + and _missing_is_already_explicit(verdict.missing, structured_claims) + ): + return _VerificationOutcome(supported=True, complete=True) + return verdict def _run_entailment_check( self, request, evidence_texts: tuple[str, ...], budget: RequestBudget | None = None, - ) -> _VerificationOutcome | None: - """One entailment call. `None` = outage/malformed/budget-exhausted - (fails closed by the caller without a retry); `True`/`False` = the - judge's verdict.""" + ) -> _VerificationOutcome | _CheckNotRun: + """One entailment call. + + Returns `_CheckNotRun` when the judge could not be consulted at all + (budget exhausted, provider outage, or a reply this code cannot + parse) and a `_VerificationOutcome` when it ran and reached a + verdict. Both make the caller fail closed; they differ only in the + reason reported, which used to collapse into `unsupported_claim` + for all of them. + """ try: if budget is not None: budget.require() raw = self._generator.generate(request.system, request.user, request.schema) + except RequestBudgetExhausted: + # Must be caught before its parent below — see `budget.py`. + return _CheckNotRun("request_budget_exhausted") except AnswerGenerationUnavailable: - return None + return _CheckNotRun("provider_unavailable") try: payload = json.loads(raw) entailed = payload["entailed"] unsupported = payload["unsupported"] missing_evidence = payload.get("missing_evidence", []) except (ValueError, TypeError, KeyError): - return None + return _CheckNotRun("malformed_output") if ( not isinstance(entailed, bool) or not isinstance(unsupported, list) or not isinstance(missing_evidence, list) ): - return None + return _CheckNotRun("malformed_output") if not entailed or unsupported: return _VerificationOutcome(supported=False, complete=False) # A completeness objection is itself a factual claim about the raw @@ -866,11 +934,11 @@ class GroundedAnswerService: grounded_missing: list[str] = [] for item in missing_evidence: if not isinstance(item, dict): - return None + return _CheckNotRun("malformed_output") description = item.get("description") evidence_quote = item.get("evidence_quote") if not isinstance(description, str) or not isinstance(evidence_quote, str): - return None + return _CheckNotRun("malformed_output") description = description.strip() quote = _normalise_for_coverage(evidence_quote) if ( diff --git a/apps/ai-service/tests/test_agent.py b/apps/ai-service/tests/test_agent.py index 57e4662..2a226c9 100644 --- a/apps/ai-service/tests/test_agent.py +++ b/apps/ai-service/tests/test_agent.py @@ -142,6 +142,84 @@ def test_drug_attribute_without_an_attribute_does_not_fall_into_overview_retriev assert retrieval.calls == [] +# --- the pediatric dosing gate. This code path gained its first test +# coverage on 2026-08-11, after driving production reproduced the same +# behaviour 5/5: the clarify question asked for both age and weight every +# time, including the field the user had just supplied ("Bé 18 ký ...", +# "Bé nặng 18 kg ...", "Trẻ 5 tuổi ..." all received "Bé bao nhiêu tuổi và +# cân nặng bao nhiêu kg?"). Requiring BOTH fields is deliberate and stays — +# the formulary bands paracetamol by age *and* by mg/kg — so these tests pin +# the question text without loosening the requirement. -- + + +def _pediatric_agent(**frame_kwargs) -> RagAgent: + return _agent(QueryFrame( + turn_type="dosing_calc", + drugs=("paracetamol_acetaminophen",), + population="tre_em", + **frame_kwargs, + )) + + +def test_pediatric_gate_with_a_known_weight_asks_only_for_the_age(): + reply = _pediatric_agent(weight_kg=18.0).handle("bé 18 ký sốt cao uống paracetamol liều bao nhiêu") + + assert reply.decision == "clarify" + assert reply.reason == "missing_pediatric_age_or_weight" + assert "tuổi" in reply.clarification + # The weight is echoed back so the user can see it was received (and + # catch a mis-parse), but is never asked for again. + assert "18 kg" in reply.clarification + assert "bao nhiêu kg" not in reply.clarification + + +def test_pediatric_gate_with_a_known_age_asks_only_for_the_weight(): + reply = _pediatric_agent(age_text="5 tuổi").handle("trẻ 5 tuổi sốt cao uống paracetamol liều bao nhiêu") + + assert reply.decision == "clarify" + assert "kg" in reply.clarification + assert "bao nhiêu tuổi" not in reply.clarification + + +def test_pediatric_gate_with_neither_field_still_asks_for_both(): + reply = _pediatric_agent().handle("liều paracetamol cho trẻ em") + + assert reply.decision == "clarify" + assert "tuổi" in reply.clarification + assert "kg" in reply.clarification + + +def test_pediatric_gate_still_requires_both_fields_before_answering(): + """The safety property, asserted directly: knowing only one of the two + must NOT be treated as enough to pick a regimen. Both single-field cases + above stop at `clarify` — this states the invariant so a future change + that "helpfully" answers with weight alone fails here loudly.""" + for kwargs in ({"weight_kg": 18.0}, {"age_text": "5 tuổi"}, {}): + reply = _pediatric_agent(**kwargs).handle("liều paracetamol cho bé") + assert reply.decision == "clarify", kwargs + assert reply.reason == "missing_pediatric_age_or_weight", kwargs + + +def test_a_whole_number_weight_is_not_echoed_with_a_trailing_zero(): + reply = _pediatric_agent(weight_kg=18.0).handle("liều paracetamol cho bé 18 ký") + + assert "18 kg" in reply.clarification + assert "18.0" not in reply.clarification + + +def test_the_models_own_clarify_question_still_wins_over_the_generated_one(): + """Unchanged precedence: when understanding.py produced a question of its + own it is still preferred, because it can see phrasing/context this + code-level fallback cannot.""" + reply = _pediatric_agent( + weight_kg=18.0, + needs_clarify=True, + clarify_reason="Bé mấy tháng tuổi rồi ạ?", + ).handle("liều paracetamol cho bé 18 ký") + + assert reply.clarification == "Bé mấy tháng tuổi rồi ạ?" + + def test_needs_clarify_frame_is_surfaced_directly(): agent = _agent(QueryFrame( turn_type="dosing_calc", drugs=("paracetamol",), diff --git a/apps/ai-service/tests/test_grounded_generation.py b/apps/ai-service/tests/test_grounded_generation.py index 98372a6..5e10fd8 100644 --- a/apps/ai-service/tests/test_grounded_generation.py +++ b/apps/ai-service/tests/test_grounded_generation.py @@ -12,6 +12,7 @@ import pytest from rag import grounding from rag.answer import GroundedAnswerService +from rag.budget import RequestBudgetExhausted from rag.metrics import GENERATION_REJECTED, GENERATION_SERVED, InMemoryMetrics from rag.models import ( Evidence, @@ -319,6 +320,16 @@ def test_entailment_accepts_after_one_semantic_pass(): def test_entailment_provider_outage_fails_closed_to_abstain(): + """Fail-closed is unchanged; only the label it fails closed *under* is. + + This previously asserted `unsupported_claim`, which reports a claim the + evidence did not support, in a case where the judge was never reachable. + `apps/web/app/api/chat/route.ts` renders that as "bước đối chiếu chưa + xác nhận được câu trả lời khớp với nguồn", describing the answer rather + than the outage, and places it in the content-failure bucket that the + failure taxonomy in `docs/current-rag-pipeline-audit.md` §4 keeps + separate from availability. + """ grounded, metrics = _answer( {"claims": [{"text": "Người lớn: 500 mg, 2 lần/ngày", "citations": [1]}], "evidence_sufficient": True}, @@ -328,7 +339,125 @@ def test_entailment_provider_outage_fails_closed_to_abstain(): assert grounded.generated is False assert grounded.answer is None assert grounded.result.decision == EvidenceDecision.ABSTAIN + assert grounded.result.reason == "provider_unavailable" + assert metrics.total(GENERATION_REJECTED, reason="provider_unavailable") == 1 + # And specifically NOT counted as a content failure. + assert metrics.total(GENERATION_REJECTED, reason="unsupported_claim") == 0 + + +def test_entailment_budget_exhaustion_is_reported_as_a_timeout_not_a_bad_claim(): + """Observed live 2026-08-11 against production. + + `RequestBudgetExhausted` subclasses `AnswerGenerationUnavailable`, so it + has to be caught first to be distinguishable from an ordinary outage; + previously both arrived as `unsupported_claim`. The user-facing string + for `request_budget_exhausted` already exists in the BFF mapping, so no + new reason code is introduced here. + """ + grounded, metrics = _answer( + {"claims": [{"text": "Người lớn: 500 mg, 2 lần/ngày", "citations": [1]}], + "evidence_sufficient": True}, + entailment_payload=RequestBudgetExhausted(), + ) + + assert grounded.generated is False + assert grounded.answer is None + assert grounded.result.decision == EvidenceDecision.ABSTAIN + assert grounded.result.reason == "request_budget_exhausted" + assert metrics.total(GENERATION_REJECTED, reason="request_budget_exhausted") == 1 + assert metrics.total(GENERATION_REJECTED, reason="unsupported_claim") == 0 + assert metrics.total(GENERATION_REJECTED, reason="provider_unavailable") == 0 + + +def test_unparseable_judge_reply_is_reported_as_malformed_not_as_a_bad_claim(): + """A judge reply this code cannot read is not a verdict against the answer.""" + grounded, metrics = _answer( + {"claims": [{"text": "Người lớn: 500 mg, 2 lần/ngày", "citations": [1]}], + "evidence_sufficient": True}, + entailment_payload="{not json at all", + ) + + assert grounded.generated is False + assert grounded.answer is None + assert grounded.result.reason == "malformed_output" + assert metrics.total(GENERATION_REJECTED, reason="unsupported_claim") == 0 + + +def test_budget_running_out_during_completeness_repair_is_not_called_incomplete(): + """Pins the production case observed live 2026-08-11. + + "Liều dùng của Isosorbid dinitrat theo Dược thư là gì?" took 40.3s + against a 40s budget and returned `incomplete_answer`, whose user-facing + text says the answer was cancelled because the source had information it + left out — while what actually happened is that the repair generation did + not run to completion. The completeness repair roughly doubles a turn's + model calls, so it is the likeliest place to exhaust the budget, and it + reports that the same way the first attempt does. + """ + grounded, metrics = _answer( + [ + {"claims": [{"text": "Người lớn uống 500 mg", "citations": [1]}], + "evidence_sufficient": True}, + RequestBudgetExhausted(), + ], + entailment_payload={ + "entailed": True, + "unsupported": [], + "complete": False, + "missing_evidence": [{ + "description": "2 lần mỗi ngày và liều tối đa 2 g mỗi ngày", + "evidence_quote": EVIDENCE_TEXT, + }], + }, + ) + + assert grounded.answer is None + assert grounded.result.reason == "request_budget_exhausted" + assert metrics.total(GENERATION_REJECTED, reason="request_budget_exhausted") == 1 + # The user must not be told their answer was missing source information + # when the repair simply ran out of time. + assert metrics.total(GENERATION_REJECTED, reason="incomplete_answer") == 0 + + +def test_a_genuinely_incomplete_repair_is_still_called_incomplete(): + """Guards the other side of the split above: when the repair really does + run and still comes back incomplete, `incomplete_answer` must survive.""" + incomplete_verdict = { + "entailed": True, + "unsupported": [], + "complete": False, + "missing_evidence": [{ + "description": "2 lần mỗi ngày và liều tối đa 2 g mỗi ngày", + "evidence_quote": EVIDENCE_TEXT, + }], + } + grounded, metrics = _answer( + {"claims": [{"text": "Người lớn uống 500 mg", "citations": [1]}], + "evidence_sufficient": True}, + entailment_payload=[incomplete_verdict, incomplete_verdict], + ) + + assert grounded.answer is None + assert grounded.result.reason == "incomplete_answer" + assert metrics.total(GENERATION_REJECTED, reason="incomplete_answer") == 1 + assert metrics.total(GENERATION_REJECTED, reason="request_budget_exhausted") == 0 + + +def test_a_real_negative_verdict_is_still_an_unsupported_claim(): + """The counterpart to the three tests above: when the judge DID run and + said no, the reason must stay a content failure. Splitting the + availability cases out must not quietly reclassify genuine rejections.""" + grounded, metrics = _answer( + {"claims": [{"text": "Metformin chữa ung thư", "citations": [1]}], + "evidence_sufficient": True}, + entailment_payload={"entailed": False, "unsupported": [1]}, + ) + + assert grounded.generated is False + assert grounded.result.reason == "unsupported_claim" assert metrics.total(GENERATION_REJECTED, reason="unsupported_claim") == 1 + assert metrics.total(GENERATION_REJECTED, reason="provider_unavailable") == 0 + assert metrics.total(GENERATION_REJECTED, reason="request_budget_exhausted") == 0 def test_entailment_check_is_skipped_when_there_are_no_claims(): diff --git a/apps/web/README.md b/apps/web/README.md index 1a882d6..cee2b8b 100644 --- a/apps/web/README.md +++ b/apps/web/README.md @@ -1,4 +1,35 @@ # web -Next.js frontend. Chat UI, auth pages, citation/disclaimer rendering, session -history. Talks only to `api-gateway` — never calls internal services directly. +Next.js frontend: chat UI, citation/evidence panel, drug autocomplete, +persistent clinical disclaimer banner. + +**Current call path: browser → this app's own route handlers → `ai-service`.** +`api-gateway` is not built yet (its directory holds a `README.md` and a +`package.json`), so requests go through `app/api/` and on to `ai-service` +(`API_GATEWAY_URL ?? AI_SERVICE_URL`, set to `http://ai-service:8000` in +production). There is no authentication layer in this path today. When the +gateway and auth services are built, this is the seam that changes. + +`app/api/chat/route.ts` is more than a proxy: it maps `ai-service`'s abstain +`reason` codes to the Vietnamese text the user reads. A reason code with no +entry there falls back to wording that reads as "no data in the formulary", +which would misrepresent an outage or a timeout — so a new backend reason +code needs an entry here in the same change. + +Timeouts: `_components/ChatPanel.tsx` aborts a request at 65s, derived from +the backend's ceiling (a 40s request budget that can overrun by one in-flight +model call). A lower value can cut off answers the server has successfully +produced, so keep it above that ceiling if it is ever revisited. + +## Running + +```powershell +pnpm install +pnpm --filter web dev # http://localhost:3000, expects ai-service on :8079 +``` + +## Tests + +There are none, and no test runner is configured. Verify changes by driving +the real UI in a browser; `pytest` passing in `apps/ai-service` says nothing +about this app. diff --git a/apps/web/app/_components/ChatPanel.tsx b/apps/web/app/_components/ChatPanel.tsx index b353dea..7c86290 100644 --- a/apps/web/app/_components/ChatPanel.tsx +++ b/apps/web/app/_components/ChatPanel.tsx @@ -29,6 +29,28 @@ interface ChatPanelProps { className?: string; } +// The client must never be the thing that gives up first. +// +// The backend's own per-request budget is 40s (`max_wall_clock_ms` in +// `apps/ai-service/config.py`), and that budget is only checked *between* +// model calls — `rag/budget.py` says so explicitly: it cannot cancel a boto3 +// call already in flight, which is separately bounded by `read_timeout=20` +// in `adapters/bedrock_converse.py`. So the backend's real worst case is +// ~40s + one in-flight call ≈ 60s, and anything below that truncates work +// the server was still legitimately doing. +// +// Measured against production 2026-08-11 (n=8, sequential, one user): +// 6.2 / 6.4 / 8.4 / 10.9 / 12.4 / 21.7 / 25.1 / 40.3 seconds. A 25s limit +// cuts off 2 of those 8, including the 25.1s case, which had returned a +// correct grounded answer with 2 citations. +const REQUEST_TIMEOUT_MS = 65_000; + +// A single spinner for a minute reads as a hang, so the wait is made +// legible rather than merely longer. This is a stopgap for the real fix +// (streaming verified claims as they land); it does not make the request +// faster, it only stops it looking broken. +const SLOW_REQUEST_NOTICE_MS = 15_000; + const STARTER_QUESTIONS = [ { category: "Chỉ Định", @@ -65,6 +87,7 @@ export function ChatPanel({ }: ChatPanelProps) { const { resolvedTheme } = useTheme(); const [isLoading, setIsLoading] = useState(false); + const [elapsedMs, setElapsedMs] = useState(0); const [error, setError] = useState(null); const messagesEndRef = useRef(null); const abortControllerRef = useRef(null); @@ -98,7 +121,12 @@ export function ChatPanel({ abortControllerRef.current = new AbortController(); const timeoutId = window.setTimeout(() => { abortControllerRef.current?.abort(); - }, 25_000); + }, REQUEST_TIMEOUT_MS); + setElapsedMs(0); + const startedAt = Date.now(); + const tickId = window.setInterval(() => { + setElapsedMs(Date.now() - startedAt); + }, 1000); try { const res = await fetch("/api/chat", { @@ -128,13 +156,19 @@ export function ChatPanel({ setError( stopRequestedRef.current ? "Đã dừng chờ trên giao diện. Tác vụ đang chạy có thể cần vài giây để kết thúc an toàn." - : "Yêu cầu vượt quá 25 giây và đã được dừng. Vui lòng thử lại với câu hỏi cụ thể hơn." + // Describes the timing cause rather than suggesting the + // question needs rewording, which rephrasing would not fix. + : `Hệ thống xử lý quá ${Math.round( + REQUEST_TIMEOUT_MS / 1000 + )} giây nên đã dừng yêu cầu này. Vui lòng bấm gửi lại.` ); return; } setError("Không thể kết nối đến máy chủ AI Service. Vui lòng kiểm tra lại dịch vụ backend."); } finally { window.clearTimeout(timeoutId); + window.clearInterval(tickId); + setElapsedMs(0); setIsLoading(false); abortControllerRef.current = null; } @@ -363,8 +397,15 @@ export function ChatPanel({
-

Đang truy xuất Dược thư QGVN 2018...

-

Đang phân tích chuyên luận & xác thực Entailment

+

+ Đang truy xuất Dược thư QGVN 2018... + {elapsedMs >= 1000 && ` ${Math.floor(elapsedMs / 1000)}s`} +

+

+ {elapsedMs >= SLOW_REQUEST_NOTICE_MS + ? "Câu hỏi tra cả mục nên cần thêm thời gian đối chiếu nguồn — vẫn đang xử lý." + : "Đang phân tích chuyên luận & xác thực Entailment"} +

)} diff --git a/apps/web/app/_components/Composer.tsx b/apps/web/app/_components/Composer.tsx index 8b628eb..eb7b031 100644 --- a/apps/web/app/_components/Composer.tsx +++ b/apps/web/app/_components/Composer.tsx @@ -186,7 +186,7 @@ export function Composer({ value={value} onChange={(e) => setValue(e.target.value)} onKeyDown={handleKeyDown} - placeholder="Nhập tên thuốc hoặc thuộc tính cần tra (Ví dụ: Liều dùng Paracetamol, Chống chỉ định Amoxicillin...)" + placeholder="Nhập câu hỏi của bạn..." rows={2} className="w-full resize-none bg-transparent px-3 py-2 text-sm text-txt-primary placeholder:text-txt-muted focus:outline-none" /> diff --git a/coordination/CLAUDE_CLAIM_2026-08-11.md b/coordination/CLAUDE_CLAIM_2026-08-11.md new file mode 100644 index 0000000..6911e75 --- /dev/null +++ b/coordination/CLAUDE_CLAIM_2026-08-11.md @@ -0,0 +1,75 @@ +# Claude ownership claim — 2026-08-11 + +Read `CLAUDE_HANDOFF_2026-08-10.md` first. Two items in it have since moved +on (checked against the code and against live production on 2026-08-11): + +1. The structured-claims refactor it describes as in progress is finished and + shipped (`dfdbf52`, then `9c3acd0`). `pytest -q --ignore=tests/test_api.py + --ignore=tests/test_live_datastores.py` = 219 passed at the start of this + session. +2. Entailment majority vote (2-of-3) is no longer in the code. `df55af4` + introduced it; `9c3acd0` replaced it with a single pass + (`_ENTAILMENT_MAX_ATTEMPTS = 1`), with the reasoning in + `_verify_entailment`'s docstring: repeating an identical temperature-0 + prompt is a correlated retry rather than an independent vote. + +## What this session is changing, and why + +All five items come from driving production (`https://realvuxbaro.me`) by +hand plus direct `/api/chat` probes — measured, not inferred from docs. + +- **`rag/answer.py`** — availability failures during the entailment pass are + currently recorded and shown as `unsupported_claim`, so a timing or outage + problem reaches the user as a content failure. The failure taxonomy in + `docs/current-rag-pipeline-audit.md` §4 keeps these separate. Reusing + the reason codes that already exist and are already mapped in + `apps/web/app/api/chat/route.ts` (`request_budget_exhausted`, + `provider_unavailable`, `malformed_output`) — **no new reason code**, so + the frontend mapping needs no change. Fail-closed behaviour is unchanged; + only the label changes. +- **`rag/answer.py`** — `_verify_entailment`'s `for _ in + range(_ENTAILMENT_MAX_ATTEMPTS)` loop always returns on its first + iteration, so raising that constant silently does nothing, and the + unreachable `return False` after it returns a `bool` where every caller + reads `.supported`. Making the single-pass intent explicit. +- **`rag/agent.py`** — the pediatric dosing gate asks "Bé bao nhiêu tuổi và + cân nặng bao nhiêu kg?" even when the user just gave one of the two. + Reproduced 5/5 live ("18 ký", "18 cân", explicit "18 kg", "Trẻ 5 tuổi"). + **The gate itself is NOT being loosened** — both fields stay required, + which is clinically right here (the correct answer uses both an age band + and a mg/kg rule). Only the question text becomes specific to what is + actually missing. +- **`apps/web/app/_components/ChatPanel.tsx`** — a hard 25s client abort + against a backend whose own budget is 40s (`config.py:60`). Measured + n=8 sequential: 2/8 exceeded 25s, and one of those was a **correct** + `answerable`/grounded/2-citation reply at 25.1s that the user never saw. + Caddy (`reverse_proxy web:3000`, no timeout) and the BFF (`signal: + request.signal`, no own timeout) do not cap this, so the client constant + is the only binding limit. +- **`packages/ui/src/ChatBubble.tsx`** — chips dedupe by `chunkId` but the + label is only drug+section+page, so several distinct chunks render as + identical-looking chips. **Not** collapsing them by label: the click + handler maps to a specific citation index, so collapsing would make real + evidence blocks unreachable from the prose, and provenance is a hard + guardrail. + +## Files claimed + +`apps/ai-service/rag/answer.py`, `apps/ai-service/rag/agent.py`, +`apps/ai-service/tests/test_grounded_generation.py`, +`apps/ai-service/tests/test_agent.py`, +`apps/web/app/_components/ChatPanel.tsx`, +`packages/ui/src/ChatBubble.tsx`, and this file. + +Not touching `ingestion/`, retrieval adapters, `rag/service.py`, +`rag/understanding.py`, or anything sparse/BM25 related — task #4 (real +BM25 via Qdrant native sparse vectors) is still **not started**. + +## Test-coverage context for review + +These areas start with little automated cover: no tests currently touch +`missing_pediatric_age_or_weight` / `missing_population`, and the repo has no +frontend test setup (no `test` script in `apps/web/package.json`, no +ChatPanel/ChatBubble tests). A passing `pytest` run therefore is not on its +own sufficient evidence here. Backend tests are being added alongside the +changes, and the two frontend changes are verified by driving the real site. diff --git a/coordination/CLAUDE_HANDOFF_2026-08-10.md b/coordination/CLAUDE_HANDOFF_2026-08-10.md index 3e34c98..fdbdc84 100644 --- a/coordination/CLAUDE_HANDOFF_2026-08-10.md +++ b/coordination/CLAUDE_HANDOFF_2026-08-10.md @@ -1,5 +1,22 @@ # Claude handoff — 2026-08-10, in case of context/token cutoff +> **Update 2026-08-11 — two items below have moved on since this was +> written.** Checked against the code and against live production. +> +> 1. The structured-claims refactor described below as in progress shipped +> the same day (`dfdbf52`, then `9c3acd0`); the conversion is complete and +> the suite is at 230 passing. +> 2. Entailment majority vote (2-of-3) is no longer in the code. `df55af4` +> introduced it and `9c3acd0` replaced it with a single pass +> (`_ENTAILMENT_MAX_ATTEMPTS = 1`); `_verify_entailment`'s docstring gives +> the reasoning — repeating an identical temperature-0 prompt is a +> correlated retry rather than an independent vote. +> +> Task #4 (real BM25 via Qdrant native sparse vectors) further down is still +> accurate and still not started. For current state see +> `coordination/CLAUDE_CLAIM_2026-08-11.md` and the 2026-08-11 entry in +> `docs/progress-log.md`. + Read this before touching `apps/ai-service/rag/answer.py`, `rag/prompt.py`, `adapters/bedrock_claude.py`, or any test file under `apps/ai-service/tests/` that references the answer-generation schema. A structured-claims refactor diff --git a/coordination/README.md b/coordination/README.md index 64d4326..df25d1c 100644 --- a/coordination/README.md +++ b/coordination/README.md @@ -105,7 +105,28 @@ self-hosted embedding/vLLM plan (assumption GĐ-3 in ## Active ownership +- Claude: **2026-08-11** — see `CLAUDE_CLAIM_2026-08-11.md` for the full + claim and reasoning. Five production bugs found by driving + `https://realvuxbaro.me` (not by reading docs), fixed, deployed and + re-verified over 37 live cases: the 25s client abort that was discarding + correct grounded answers, availability failures mislabelled as + `unsupported_claim`/`incomplete_answer`, the pediatric clarify question + re-asking for fields the user had just given, a no-op entailment retry + loop, and identical-looking citation chips. Touched + `rag/answer.py`, `rag/agent.py`, their tests, `ChatPanel.tsx`, + `ChatBubble.tsx`. **Deliberately NOT changed**: the pediatric gate still + requires both age and weight, chips are not collapsed, the completeness + judge was not relaxed. 230 passed (was 219). Commits `93aa322`, `4e78363`. + +- **Note on reading status text here, 2026-08-11**: ownership entries in this + file and in `CLAUDE_HANDOFF_2026-08-10.md` are written at a point in time + and can fall behind — five commits landed on 2026-08-10 between 17:09 and + 17:27 after the entries below were written. `git log` is the reliable + source for current state; these entries are useful for intent and + reasoning. + - Codex parallel session: **STOPPED, 2026-08-10** — owner ended the session. + (Commits `9c3acd0` … `4438c5f` landed after this line was written.) Left `rag/expansion.py` and `rag/context.py` finished and tested but not wired into any live retrieval path; `rag/fusion.py`/`tests/test_fusion.py` (a third, separate ChatGPT session's work, per Codex's own note above) diff --git a/docs/adr/0002-argocd-gitops.md b/docs/adr/0002-argocd-gitops.md index 5563036..b721393 100644 --- a/docs/adr/0002-argocd-gitops.md +++ b/docs/adr/0002-argocd-gitops.md @@ -2,7 +2,27 @@ ## Status -Accepted +**Accepted — still the target, not yet implemented.** Not superseded by the +current production setup. + +Since 2026-08-10 the project has a *different*, interim deployment: a single +EC2 box running `infra/docker/docker-compose.prod.yml` behind Caddy, deployed +by `.github/workflows/deploy.yml` over SSH. That was built to get a working +demo online, not to replace this decision. Migrating to the team's Kubernetes ++ ArgoCD remains planned work, and the expensive prerequisite — containerising +both apps — is already done, so the Dockerfiles and compose services port over. + +Two things must still happen and neither has been started: + +1. **Repository move to the team's self-hosted Gitea** (company domain), which + is where the GitOps repo is meant to live. The project stays on private + GitHub until that move is deliberately made. Note the hard boundary already + in force: the team's existing `git.vinmec.tech/ai-team/gitops` repository is + **reference-only** — never push this project into it. +2. **Filling in the scaffolds this ADR assumes exist.** `infra/helm/medical-chatbot/templates/` + and `infra/k8s/**` are empty (`.gitkeep` only), the chart is version `0.0.0`, + and every `infra/argocd/applications/*/app.yaml` still carries unresolved + TODOs for project, repo URL and destination cluster. ## Context diff --git a/docs/architecture.md b/docs/architecture.md index f621173..2b4b850 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -38,10 +38,15 @@ needed now. the transactional Postgres and has a mature Helm chart for the production k8s target. See `docs/adr/0001-vector-db-qdrant.md`. - **Relational DB: PostgreSQL.** One instance, logically separated per - service (users/credentials, profiles, chat sessions+messages). + service (users/credentials, profiles, chat sessions+messages). *As built, + only `ai-service` uses it* — for conversation turns (`rag_conversation_turn`) + and retrieval traces (`rag_retrieval_trace`). The users/profiles/sessions + tables belong to services that do not exist yet. - **Redis.** Session/refresh-token cache, rate-limit counters, and reserved as the future job-queue backend (BullMQ/Celery) if async admin-triggered - re-ingestion or background jobs are added later. + re-ingestion or background jobs are added later. **Not deployed** — nothing + in the live path reads or writes Redis, so it was left out of + `docker-compose.prod.yml` rather than run idle. ## RAG ingestion pipeline (PDF-specific) @@ -150,11 +155,19 @@ methodology, cross-tool comparison, and validation numbers. always-present disclaimer. **Done** — live since 2026-08-05, see ADR 0008. 3. **auth/user/chat services + api-gateway.** Done when register → login → chat message flows end-to-end through the gateway only, persisted in - Postgres. + Postgres. **Not started** — all four directories still hold only a + `README.md` and a `package.json`. Phases 4-6 were done around this gap, + so the live system has no gateway and no auth (see below). 4. **Next.js frontend chat UI.** Done when a browser user can log in, ask a question, and see a grounded answer with citation + disclaimer banner. + **Done except the login half** — chat, citations, evidence panel and the + disclaimer banner are live; there is no login because Phase 3 does not + exist. The browser calls `apps/web`'s own route handlers, which proxy + directly to `ai-service`. 5. **Containerize + docker-compose local.** Done when `docker compose up` from a clean checkout brings up the full stack and the Phase 4 flow works. + **Done** — 2026-08-10. `infra/docker/docker-compose.prod.yml` is what + production actually runs. 6. **Kubernetes/Helm + Terraform + CI + ArgoCD (GitOps) deployment.** Done when CI builds/tests/pushes an image and bumps the target environment's Helm values file, the team's ArgoCD instance (see `infra/argocd/`, @@ -163,6 +176,44 @@ methodology, cross-tool comparison, and validation numbers. never runs `kubectl`/`helm` directly against a cluster. Cloud provider choice (AWS/GCP/Azure) only affects the Terraform module implementations, not this repo's structure. + **Still the destination — not started, not dropped.** Production was + shipped ahead of it on an interim single-box setup (see "Deployment as + actually built" below), which is a stopgap, not a replacement: ADR 0002 + remains *Accepted*. Nothing here exists yet — `infra/k8s/`, + `infra/helm/medical-chatbot/templates/` and `infra/terraform/` are empty + scaffolds (`.gitkeep` only), the chart is version `0.0.0`, and every ArgoCD + `Application` manifest still carries unresolved `TODO`s for project, repo + URL and destination cluster. -See `docs/adr/` for architecture decision records and `docs/runbooks/` for -operational runbooks (added as they're needed). + This phase also includes a **repository move to the team's self-hosted + Gitea** on the company domain, which is where the GitOps repo is intended + to live; the project stays on private GitHub until that move is made + deliberately. Hard boundary meanwhile: the team's existing + `git.vinmec.tech/ai-team/gitops` repository is **reference-only — never + push this project into it**. + +## Deployment as actually built (2026-08-10) + +Production is **not** the Phase 6 design. It is a single AWS EC2 `t3.large` +running `infra/docker/docker-compose.prod.yml` — postgres, qdrant, +ai-service, web, and Caddy terminating TLS for `realvuxbaro.me` via +automatic Let's Encrypt. Bedrock is reached through an IAM instance role, so +no long-lived AWS key exists on the box or in any env file. + +CI/CD is `.github/workflows/deploy.yml`: a push to `master` SSHes in, resets +the checkout, rebuilds only `ai-service`/`web`, runs migrations and +health-checks both. It does not touch postgres/qdrant/caddy, so the 15,100 +Qdrant points survive deploys (they live in a named volume). + +This is an **interim setup, not a decision against Phase 6.** It exists +because a working public demo was needed sooner than the Kubernetes path +could deliver one. The expensive prerequisite for that path — containerising +both apps — is exactly what this work produced, so the Dockerfiles and +compose services port over when the Gitea + team-ArgoCD migration is +actually done. Phase 6 and ADR 0002 both stand as written. + +See `docs/adr/` for architecture decision records. `docs/runbooks/` is still +**empty** — the operational knowledge that would live there (restoring a +Qdrant snapshot onto a fresh box, what a failed deploy looks like, why +`uvicorn --reload` must not be used on Windows here) currently only exists +in `docs/progress-log.md`. diff --git a/docs/current-rag-pipeline-audit.md b/docs/current-rag-pipeline-audit.md index 7e1fc88..d167952 100644 --- a/docs/current-rag-pipeline-audit.md +++ b/docs/current-rag-pipeline-audit.md @@ -4,6 +4,22 @@ > implementation thật đang có trong worktree, bao gồm các thay đổi chưa commit. > `EXISTS` không có nghĩa là đã đạt chất lượng production; nó chỉ nghĩa là đã > tìm thấy implementation live tương đương. +> +> **Cập nhật 2026-08-11 — đây là bản ghi theo ngày 2026-08-10.** Đo lại trên +> production ngày 2026-08-11 cho hai kết quả khác: +> +> - §7 ghi "Sildenafil ADR vẫn fail `ungrounded_number`". Hai lần chạy lại +> cho hai kết quả khác nhau (46,8s `abstain/unsupported_claim`; 25,1s +> `answerable/grounded`, 2 citation), không lần nào là `ungrounded_number`. +> Nhiều khả năng là nhiễu ở tầng entailment/generation hơn là một lỗi xác +> định, nên nếu xử lý thì nên tiếp cận theo hướng đó. +> - §7 ghi "Pytest: 226 passed, 5 skipped". Số hiện tại là 230 passed (bỏ +> `test_api.py`/`test_live_datastores.py` vốn cần datastore sống). +> +> Một phần §6/§7 đã được xử lý ngày 2026-08-11: `incomplete_answer` ở đường +> completeness-repair phần lớn đến từ việc cạn budget, nay tách thành +> `request_budget_exhausted`/`provider_unavailable`. Xem mục 2026-08-11 trong +> `docs/progress-log.md`. ## 1. Request path đã xác minh diff --git a/docs/progress-log.md b/docs/progress-log.md index e50d863..5db6051 100644 --- a/docs/progress-log.md +++ b/docs/progress-log.md @@ -1,5 +1,126 @@ # Progress Log +## 2026-08-11 — Five production bugs found by driving the live site, fixed, deployed and re-verified across 37 live cases + +**Starting point: two items in the docs had moved on**, found by reading the +code and driving production rather than by re-reading the docs: + +1. The structured-claims refactor that + `coordination/CLAUDE_HANDOFF_2026-08-10.md` describes as in progress + shipped the same day (`dfdbf52`, then `9c3acd0`). +2. Entailment majority-vote (2-of-3) is no longer in the code. `df55af4` + introduced it; `9c3acd0` replaced it with a single pass + (`_ENTAILMENT_MAX_ATTEMPTS = 1`). + +The previous entry here (cont. 17) also predates five commits — +`9c3acd0`/`33154d4`/`01e44ad`/`480bd1a`/`4438c5f`, 17:09-17:27 on +2026-08-10. `9c3acd0` is substantial: `QueryFrame` gained +`section_overview`/`standalone_query`/`depends_on_previous_turn`, +`population`/`route` became enum-validated, `CatalogDrugResolver.resolve()` +went from ~10k regexes per query to a token-span index, and `page.tsx` +stopped losing messages on session switch. Worth remembering generally: +entries in this log are written at a point in time, so `git log` is the +reliable check for current state. + +### Findings (all from driving `https://realvuxbaro.me`) + +- **A 25s client abort against a 40s backend budget.** `ChatPanel.tsx` + aborted every request at 25s; `config.py`'s `max_wall_clock_ms` is 40s and + can overrun by one in-flight call (`read_timeout=20`), putting the + backend's ceiling near 60s. Measured n=8 sequential: 6.2/6.4/8.4/10.9/12.4/ + 21.7/**25.1**/**40.3**s. The 25.1s case was a correct `answerable`, + grounded, 2-citation answer that never reached the user — the UI showed + "Yêu cầu vượt quá 25 giây... thử lại với câu hỏi cụ thể hơn", which points + at the question when the cause was timing. Caddy and the BFF set no timeout + of their own, so this constant was the only binding limit. +- **Availability failures reaching the user as content failures.** + `_run_entailment_check` returned a bare `None` for budget exhaustion, + provider outage and an unparseable judge reply alike, and the caller mapped + all three to `unsupported_claim`, i.e. "the answer doesn't match the + source", in cases where the judge was never consulted. The + completeness-repair path had the same shape: it fell through to + `incomplete_answer`, whose text tells the clinician the answer was + cancelled for omitting source information. Live example: Isosorbid dinitrat + dosage, 40.3s against a 40s budget, reported as `incomplete_answer`. The + failure taxonomy in `docs/current-rag-pipeline-audit.md` §4 keeps + availability and content failures separate for this reason. +- **The pediatric dosing gate asked again for what the user had given.** + `agent.py`'s fallback was the static "Bé bao nhiêu tuổi và cân nặng bao + nhiêu kg?". Reproduced 5/5: "18 ký", "18 cân", explicit "18 kg", and + "Trẻ 5 tuổi" all received it. Worth noting for anyone revisiting it: this + is not a Vietnamese colloquial-weight parsing issue — explicit "kg" + behaved identically, so the parser is not the place to change. The effect + was also more visible the better `understanding.py` did, since a frame that + parsed the weight and set `needs_clarify=false` reached the static string. +- **A retry constant with no effect.** `_verify_entailment`'s + `for _ in range(_ENTAILMENT_MAX_ATTEMPTS)` returned on its first iteration + on every path, so raising it adds no retries, and the unreachable + `return False` after it returns a `bool` where callers read `.supported`. +- **Citation chips that render identically.** Deduped by `chunkId` but + labelled only drug+section+page, so three distinct chunks appeared as three + identical "METFORMIN · Liều lượng & Cách dùng · tr. 957" chips. + +### What was changed, and what deliberately was NOT + +The pediatric gate **still requires both age and weight** — the formulary +bands paracetamol by age ("Trẻ em 4-6 tuổi: 240 mg") *and* by mg/kg ("10-50 +kg: 15 mg/kg"), so one field alone cannot pick a regimen. Only the question +changed, and it now echoes the known value back so a mis-parse is visible. +Chips are **not** collapsed by label — each opens a different evidence block +and provenance is a hard guardrail — they carry the number the evidence +panel already shows. The completeness judge was **not** relaxed: making that +symptom disappear by loosening it would ship incomplete medical answers. +Reason codes reused are ones the BFF already maps +(`request_budget_exhausted`, `provider_unavailable`, `malformed_output`); +an unmapped code silently reads as "no data in the formulary". + +### Verification (37 live cases, not one) + +`pytest`: **230 passed** (was 219; 9 added). Ruff, `tsc --noEmit` and the +Next production build all pass. One existing test changed on purpose — +`test_entailment_provider_outage_fails_closed_to_abstain` asserted the old +`unsupported_claim` label; its fail-closed assertions are untouched. + +Post-deploy, against production: a **31-case battery** (cases that must +change, cases that must NOT, plus neighbouring behaviour) and a **6-run +repeat** of one flaky query. +- Pediatric clarify verified across 7 variants: "Bé 18 ký…" → "Bé nặng 18 + kg, vậy bé bao nhiêu tuổi?"; "Trẻ 5 tuổi…" → "Bé 5 tuổi nặng bao nhiêu + kg?"; "Bé 8 tháng tuổi…" → "Bé 8 tháng tuổi nặng bao nhiêu kg?". +- Multi-turn resolves in **both** directions (clarify→age and clarify→weight + both reach `answerable` with 2 citations). +- Timeout fix proven in the browser: a Metformin adult-dosing question ran + past **33s** — dead 8s earlier under the old limit — and returned a full + grounded answer with 3 citations. Deployed bundle contains `65e3`/`15e3` + and **no** `25e3`. +- Chips render `[1] [2] [3]` matching evidence-panel cards 1/2/3, all three + preserved. +- Regression guards all held: adult dosing untouched by the pediatric gate, + quarantine `verify_pdf` intact (4 and 6 citations), fake drug → + `drug_not_in_formulary`, veterinary → `out_of_scope`, ordinary facet + lookups still answerable. + +### Known-remaining, deliberately not claimed as fixed + +- **The reason-code split is unit-tested but was NOT observed live**: nothing + in the verification run exhausted the budget (max 24.2s), so no live + `request_budget_exhausted` from the entailment/repair path was seen. +- **Zolpidem ADR is flaky**: 6 repeats gave 5 `answerable`, 1 + `ungrounded_number` (~17%). Pre-existing generation variance — + `grounding.verify` runs *before* any code changed here — not a regression. +- **The same "already told you" defect survives in the LLM-generated clarify + question**: "Bé 12 cân uống paracetamol…" is answered with "Đường dùng là + uống hay tiêm ạ?" although the user said "uống". That text comes from + `understanding.py`'s own `clarify_reason`, not the code-level fallback + fixed here. +- Latency is unchanged — the timeout fix stops discarding good answers, it + does not make anything faster. Streaming is still the real fix. +- **Sildenafil ADR is not a deterministic `ungrounded_number` failure**, as + `docs/current-rag-pipeline-audit.md` states: two runs gave 46.8s + `abstain/unsupported_claim` and 25.1s `answerable/grounded`. +- Task #4 (real BM25 via Qdrant native sparse vectors) remains **not + started**. + ## 2026-08-10 (cont. 17) — First production deployment: EC2 + Docker + CI/CD, live at realvuxbaro.me Owner and Codex agreed a work split mid-session diff --git a/ingestion/README.md b/ingestion/README.md index 4b93123..f40e7b3 100644 --- a/ingestion/README.md +++ b/ingestion/README.md @@ -4,5 +4,20 @@ Offline batch pipeline (never part of the live `ai-service` request path): extract -> segment -> chunk -> embed -> load. Parses `data/raw/duoc-thu-quoc-gia-viet-nam-2018.pdf` into per-drug, per-section chunks and upserts embeddings into Qdrant. Run via -`python -m ingestion.cli run --pdf data/raw/duoc-thu-quoc-gia-viet-nam-2018.pdf` -(once implemented). +`python -m ingestion.cli run --pdf data/raw/duoc-thu-quoc-gia-viet-nam-2018.pdf`. + +**This has already been run.** Qdrant collection `duocthu_v1` holds 15,100 +points (14,949 prose + 151 block descriptors) embedded with +`cohere.embed-v4:0`, and a `duocthu_v1__manifest` sidecar pins the corpus +sha/model/dimensions that `ai-service` checks at startup. + +Re-running the embed step costs **real Bedrock spend** on a personal AWS +account, so do not start a corpus run without the owner's explicit go for +that specific run. To move an existing corpus between machines, snapshot and +restore the Qdrant collection instead — it is free and exact. + +Scope: the corpus covers **Part 2 monographs (printed pages 99-1496) only**. +Part 1 general chapters and Part 3 appendices (BSA table, IV preparation, ATC +index) are deliberately excluded, so questions about them correctly abstain +rather than being answered from a neighbouring section. + diff --git a/packages/ui/src/ChatBubble.tsx b/packages/ui/src/ChatBubble.tsx index 7d5cff5..a2a628e 100644 --- a/packages/ui/src/ChatBubble.tsx +++ b/packages/ui/src/ChatBubble.tsx @@ -220,7 +220,18 @@ export function ChatBubble({ className="inline-flex items-center gap-1 text-[0.7rem] font-medium text-txt-muted transition-colors hover:text-accent-primary" > - Xem căn cứ · {citation.drugName} · {citationSectionLabel(citation.sectionType)} · tr. {citation.sourcePageRange[0]} + {/* Several distinct chunks routinely share one drug, + section and printed page (seen live 2026-08-11: + three chips all reading "METFORMIN · Liều lượng & + Cách dùng · tr. 957"), which reads as the same + link repeated. They are deliberately not collapsed + by label: each chip opens a different evidence + block and provenance is a hard guardrail. Instead + each carries the number the evidence panel already + shows on its cards, so a chip maps to exactly one + card. The number stays off single-source blocks, + where there is nothing to disambiguate. */} + Xem căn cứ{sources.length > 1 ? ` [${index + 1}]` : ""} · {citation.drugName} · {citationSectionLabel(citation.sectionType)} · tr. {citation.sourcePageRange[0]} ))}