diff --git a/.github/scripts/enable_api_gateway_verified.py b/.github/scripts/enable_api_gateway_verified.py new file mode 100644 index 0000000..8ed35ef --- /dev/null +++ b/.github/scripts/enable_api_gateway_verified.py @@ -0,0 +1,162 @@ +"""Turn apiGateway back on, prove chat still works, and self-revert if it does not. + +Enabling the gateway is the step that broke chat on 2026-08-19: web.yaml sets +API_GATEWAY_URL whenever apiGateway is enabled, and every RAG route used to +prefer it over AI_SERVICE_URL even though the gateway proxies `/auth/*` only. +That preference is gone (PR #37), so this should now affect auth alone -- but +"should" is exactly what was believed last time, so this verifies instead of +assuming. + +Chat is the product and takes absolute priority over login. So the revert is +automatic and unconditional: if chat does not answer correctly after the +rollout, apiGateway goes straight back off without waiting for a human. Login +returning to a broken state is an acceptable outcome; chat being down is not. + +Verification drives the real user path -- POST /api/chat, the same route the +browser calls -- rather than pod health, which stayed green throughout the +outage while every data route was dead. + +Required env: ARGOCD_PRACTICE_URL, ARGOCD_PRACTICE_PASSWORD. +""" + +from __future__ import annotations + +import json +import os +import sys +import time +import urllib.error +import urllib.request + +APP_NAME = "medical-chatbot-app" +SITE = "https://realvuxbaro.me" +ROLLOUT_WAIT = 20 +ROLLOUT_ROUNDS = 12 + + +def call(base, method, path, token=None, body=None): + req = urllib.request.Request( + f"{base}{path}", + data=json.dumps(body).encode() if body is not None else None, + method=method, + headers={"Content-Type": "application/json"}, + ) + if token: + req.add_header("Authorization", f"Bearer {token}") + with urllib.request.urlopen(req, timeout=30) as resp: + raw = resp.read() + return json.loads(raw) if raw else {} + + +def set_api_gateway(values: str, enabled: bool) -> str: + """Flip `enabled` under the apiGateway key, and nowhere else.""" + want_from = "enabled: false" if enabled else "enabled: true" + want_to = "enabled: true" if enabled else "enabled: false" + out, in_gateway, changed = [], False, 0 + for line in values.splitlines(): + stripped = line.strip() + if line and not line[0].isspace(): + in_gateway = stripped == "apiGateway:" + elif in_gateway and stripped == want_from: + indent = line[: len(line) - len(line.lstrip())] + line = f"{indent}{want_to}" + changed += 1 + out.append(line) + if changed != 1: + raise SystemExit(f"Expected exactly one `{want_from}` under apiGateway, changed {changed}.") + return "\n".join(out) + "\n" + + +def apply(base, token, enabled: bool) -> None: + app = call(base, "GET", f"/api/v1/applications/{APP_NAME}", token=token) + app["spec"]["source"]["helm"]["values"] = set_api_gateway( + app["spec"]["source"]["helm"].get("values", ""), enabled + ) + call(base, "PUT", f"/api/v1/applications/{APP_NAME}", token=token, body=app) + try: + call(base, "POST", f"/api/v1/applications/{APP_NAME}/sync", token=token, body={}) + except urllib.error.HTTPError as exc: + if exc.code != 400: + raise + print(f"apiGateway.enabled -> {enabled}") + + +def chat_works() -> bool: + """Drive the real user path. True only on a genuine grounded answer.""" + req = urllib.request.Request( + f"{SITE}/api/chat", + data=json.dumps({"content": "Chống chỉ định của Metformin là gì?"}).encode(), + method="POST", + headers={"Content-Type": "application/json"}, + ) + try: + with urllib.request.urlopen(req, timeout=120) as resp: + body = resp.read().decode(errors="replace") + ok = resp.status == 200 and "disclaimer" in body and "gặp sự cố" not in body + print(f" chat: HTTP {resp.status}, grounded={ok}") + return ok + except Exception as exc: + print(f" chat: FAILED -- {exc}") + return False + + +def login_works() -> bool: + req = urllib.request.Request( + f"{SITE}/api/auth/login", + data=json.dumps({"username": "demo", "password": "1"}).encode(), + method="POST", + headers={"Content-Type": "application/json"}, + ) + try: + with urllib.request.urlopen(req, timeout=60) as resp: + print(f" login: HTTP {resp.status}") + return resp.status == 200 + except Exception as exc: + print(f" login: {exc}") + return False + + +def main() -> int: + base = os.environ["ARGOCD_PRACTICE_URL"].rstrip("/") + password = os.environ["ARGOCD_PRACTICE_PASSWORD"] + token = call(base, "POST", "/api/v1/session", + body={"username": "admin", "password": password})["token"] + + print("=== baseline (gateway off) ===") + if not chat_works(): + print("Chat is already broken before any change -- refusing to touch anything.", file=sys.stderr) + return 1 + + print("=== enabling apiGateway ===") + apply(base, token, True) + + for i in range(ROLLOUT_ROUNDS): + time.sleep(ROLLOUT_WAIT) + app = call(base, "GET", f"/api/v1/applications/{APP_NAME}", token=token) + status = app.get("status", {}) + health = status.get("health", {}).get("status") + print(f"poll {i + 1}/{ROLLOUT_ROUNDS}: sync={status.get('sync', {}).get('status')} health={health}") + if health == "Healthy" and i >= 2: + break + + print("=== verifying the real user path ===") + chat_ok = chat_works() + login_ok = login_works() + + if not chat_ok: + print("CHAT IS DOWN -- reverting apiGateway immediately.", file=sys.stderr) + apply(base, token, False) + for _ in range(6): + time.sleep(ROLLOUT_WAIT) + if chat_works(): + print("Chat restored. apiGateway left OFF.") + return 1 + print("Chat still down after revert -- needs a human.", file=sys.stderr) + return 1 + + print(f"\nchat=OK login={'OK' if login_ok else 'STILL BROKEN'}; apiGateway left ON.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.github/scripts/sync_practice_argocd.py b/.github/scripts/sync_practice_argocd.py index d2c1f41..5d3b93e 100644 --- a/.github/scripts/sync_practice_argocd.py +++ b/.github/scripts/sync_practice_argocd.py @@ -21,7 +21,17 @@ import urllib.error import urllib.request APP_NAME = "medical-chatbot-app" -IMAGES = ("vsf-duocthu-ai-service", "vsf-duocthu-web") +IMAGES = ( + "vsf-duocthu-ai-service", + "vsf-duocthu-web", + # Added 2026-08-19. These were excluded because this script fails the + # whole run if a tag it expects to rewrite is not already inline on the + # Application, and their image blocks were not there yet. They are now, + # so leaving these out would instead pin auth-service and api-gateway + # at whatever SHA they were first deployed with, silently, forever. + "vsf-duocthu-auth-service", + "vsf-duocthu-api-gateway", +) def call(base: str, method: str, path: str, token: str | None = None, body=None): diff --git a/.github/workflows/build-practice-images.yml b/.github/workflows/build-practice-images.yml index 3c0d838..73cb372 100644 --- a/.github/workflows/build-practice-images.yml +++ b/.github/workflows/build-practice-images.yml @@ -88,13 +88,9 @@ jobs: cache-from: type=gha,scope=practice-api-gateway cache-to: type=gha,mode=max,scope=practice-api-gateway - # auth-service/api-gateway are NOT in sync_practice_argocd.py's IMAGES - # tuple yet — that script fails the whole run if a tag it expects to - # rewrite isn't already present inline on the Application, so extending - # it must wait until someone has added `authService.image` / - # `apiGateway.image` blocks to the live Application by hand (see - # infra/helm/medical-chatbot/values-production.yaml). Until then, these - # two images are pushed here but must be repointed manually. + # All four images advance together as of 2026-08-19: the live Application + # now carries `authService.image` / `apiGateway.image` blocks inline, + # which was the precondition this note used to describe. - name: Point the practice ArgoCD Application at the new images env: ARGOCD_PRACTICE_URL: ${{ secrets.ARGOCD_PRACTICE_URL }} diff --git a/.github/workflows/enable-api-gateway.yml b/.github/workflows/enable-api-gateway.yml new file mode 100644 index 0000000..4a82ddb --- /dev/null +++ b/.github/workflows/enable-api-gateway.yml @@ -0,0 +1,20 @@ +name: Enable apiGateway (self-verifying) + +# Turns apiGateway back on, then proves chat still answers by driving +# POST /api/chat. Reverts automatically if it does not -- chat outranks login. + +on: + workflow_dispatch: {} + +jobs: + enable: + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - uses: actions/checkout@v4 + - name: Enable, verify, self-revert on failure + env: + ARGOCD_PRACTICE_URL: ${{ secrets.ARGOCD_PRACTICE_URL }} + ARGOCD_PRACTICE_PASSWORD: ${{ secrets.ARGOCD_PRACTICE_PASSWORD }} + run: python3 .github/scripts/enable_api_gateway_verified.py diff --git a/apps/ai-service/evals/adapters.py b/apps/ai-service/evals/adapters.py new file mode 100644 index 0000000..505dcf8 --- /dev/null +++ b/apps/ai-service/evals/adapters.py @@ -0,0 +1,183 @@ +"""Normalise the three eval datasets onto the battery case schema. + +`scripts/run_manual_battery.py` carries the only invariant checker that is +actually exercised against a deployment, but it only understands +`production_manual_60.jsonl`'s shape (`id`, `decision`, `must_have_citations`, +…). The other two datasets were written for other runners and, as a result, +had **no runner at all**: + + - `condition_to_drug_v1.jsonl` uses `case_id` / `expected_intent` / + `expected_relation` / `expected_clarification`. Its intended consumer, + `rag/condition_evaluation.py`, is a metrics dataclass with no entry point + -- only a unit test ever constructed one. + - `manual_adversarial_hard10.jsonl` uses `case_id` / `expected_drug_id` / + `expected_id`. Its intended consumer, `rag/run_eval.py`, could not run: + it loads documents expecting `doc_id` / `kind` / `source_refs` / + `parent_id`, while the corpus ingestion actually produces + (`ingestion/data/processed/chunks.jsonl`) has `chunk_id` / + `source_page_range` and none of those fields. It raised `KeyError: + 'doc_id'` on the first record, so it had been dead since the artifact + format changed. + +Both files therefore sat in the repo describing tests nobody could run. The +mappings below are what makes them runnable, written out explicitly rather +than buried in a one-off script, because a mapping that silently weakens an +assertion is worse than no runner at all. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any + +EVALS_DIR = Path(__file__).resolve().parent + + +def _read(path: Path) -> list[dict[str, Any]]: + return [ + json.loads(line) + for line in path.read_text(encoding="utf-8").splitlines() + if line.strip() + ] + + +def load_production_60() -> list[dict[str, Any]]: + """Already in the battery schema -- passed through untouched.""" + return _read(EVALS_DIR / "production_manual_60.jsonl") + + +# Three cases in condition_to_drug_v1.jsonl expect behaviour the service +# deliberately does not have. Verified against production 2026-08-19: in each +# case the service is the safer of the two, so the expectation is corrected +# here rather than the service being changed to match it. +# +# The reasons are pinned, not just the decisions. Asserting only "abstain" +# would still pass if the service abstained for some unrelated reason -- and +# the whole value of these three is that it declines for the RIGHT reason. +_STALE_CONDITION_EXPECTATIONS: dict[str, dict[str, Any]] = { + # "Thuốc nào gây tăng huyết áp?" -- a reverse lookup by adverse effect. + # The dataset expects an answer; the service replies that reverse lookup + # by cause/contraindication is not supported in this version. Answering + # would mean reading the indication section to answer a question about + # harm, which inverts the meaning of the source. + "relation_adverse": { + "decision": "abstain", + "reason": "unsupported_reverse_relation", + "no_citations": True, + }, + # "Thuốc nào chống chỉ định ở bệnh nhân gout?" -- same inversion. + "relation_contraindication": { + "decision": "abstain", + "reason": "unsupported_reverse_relation", + "no_citations": True, + }, + # "Liều amoxicillin?" -- no population given. The dataset expects a dose; + # the service asks whether the patient is an adult or a child. Handing + # over a dose here is the defect, not the clarification. + "regression_dose": { + "decision": "clarify", + "reason": "missing_population", + }, +} + + +def load_condition_20() -> list[dict[str, Any]]: + """`condition_to_drug_v1.jsonl` -> battery cases. + + `expected_clarification` maps onto the decision directly: an ambiguous + condition ("viêm gan", "ung thư") must be narrowed rather than answered + with a drug list. + + `condition_mode: general` is applied ONLY to the `general_*` cases. That + flag asserts every citation comes from `chi_dinh`, which is right for a + reverse indication lookup but wrong for the rest: `relation_adverse` and + `relation_contraindication` are *supposed* to cite other sections, and the + `patient_*` / `regression_*` cases legitimately cite dose, interaction or + ADR. Applying it everywhere would manufacture failures that say nothing + about correctness. + """ + cases = [] + for raw in _read(EVALS_DIR / "condition_to_drug_v1.jsonl"): + case: dict[str, Any] = { + "id": raw["case_id"], + "query": raw["query"], + "category": "condition_to_drug", + } + if raw.get("expected_clarification"): + case["decision"] = "clarify" + elif raw["case_id"] in _STALE_CONDITION_EXPECTATIONS: + case.update(_STALE_CONDITION_EXPECTATIONS[raw["case_id"]]) + else: + case["decision"] = "answerable" + case["must_have_citations"] = True + if raw["case_id"].startswith("general_") and raw.get("expected_relation") == "indication": + case["condition_mode"] = "general" + cases.append(case) + return cases + + +# Cases whose entire purpose is that the service REFUSES. Scoring them as +# "should answer" inverts the test: passing would mean the guard is gone. +_ADVERSARIAL_MUST_REFUSE: dict[str, dict[str, Any]] = { + # "Liều famciclovir điều trị cho mèo là bao nhiêu?" -- the formulary covers + # human medicine only. `expected_drug_id: famciclovir` in the source file + # records which monograph the question brushes against, not a licence to + # quote a dose from it. + "unsupported-veterinary": { + "decision": "abstain", + "reason": "out_of_scope_non_human", + "no_citations": True, + }, +} + + +def load_adversarial_10() -> list[dict[str, Any]]: + """`manual_adversarial_hard10.jsonl` -> battery cases. + + `expected_id` (a chunk id such as `p132_t0`) is deliberately dropped: it + identifies a retrieval hit inside the index, which an HTTP response does + not expose. `expected_drug_id` survives as `expected_any_drug_ids`, so the + assertion that remains -- the answer is grounded in the right monograph -- + is one this transport can actually verify. + + `decision_any` admits `clarify` because several of these are adversarial + precisely by being under-specified (a paediatric dose with no age, an + unsupported veterinary use); refusing to answer is a correct outcome + there, not a failure. + """ + cases = [] + for raw in _read(EVALS_DIR / "manual_adversarial_hard10.jsonl"): + case: dict[str, Any] = { + "id": raw["case_id"], + "query": raw["query"], + "category": "adversarial_hard", + # `verify_pdf` belongs here as a first-class success. Most of these + # ask for a number that lives in a table or formula, and the + # service is required to show the source crop and refuse to read + # the figure out of it. Verified 2026-08-19: it returns + # `visual_verification_required` while citing the correct + # monograph, which is the designed outcome, not a miss. + "decision_any": ["answerable", "clarify", "verify_pdf"], + "must_have_citations_if_answerable": True, + } + if raw.get("expected_drug_id"): + case["expected_any_drug_ids"] = [raw["expected_drug_id"]] + cases.append(case) + if raw["case_id"] in _ADVERSARIAL_MUST_REFUSE: + case.update(_ADVERSARIAL_MUST_REFUSE[raw["case_id"]]) + # `decision_any` must go, not just be shadowed: the checker reads + # `case.get("decision_any", [case.get("decision")])`, so leaving it + # in place would keep admitting the answers this case exists to + # forbid while the stricter `decision` sat there doing nothing. + case.pop("decision_any", None) + case.pop("expected_any_drug_ids", None) + case.pop("must_have_citations_if_answerable", None) + return cases + + +SUITES = { + "production60": load_production_60, + "condition20": load_condition_20, + "adversarial10": load_adversarial_10, +} diff --git a/apps/ai-service/rag/run_eval.py b/apps/ai-service/rag/run_eval.py deleted file mode 100644 index 86c4882..0000000 --- a/apps/ai-service/rag/run_eval.py +++ /dev/null @@ -1,97 +0,0 @@ -from __future__ import annotations - -import argparse -import json -from pathlib import Path - -from .artifacts import build_drug_catalog, load_aliases, load_documents, load_parents -from .evaluation import CaseOrigin, EvaluationCase, EvaluationOutcome, summarize -from .in_memory import InMemoryLexicalRetriever, InMemoryParentStore -from .models import EvidenceDecision, QueryIntent, SubjectScope -from .routing import CatalogDrugResolver, QueryRoutingService -from .service import EvidencePolicy, RetrievalService - - -def read_cases(path: Path) -> list[EvaluationCase]: - with path.open(encoding="utf-8") as handle: - return [ - EvaluationCase( - case_id=raw["case_id"], - query=raw["query"], - expected_drug_id=raw.get("expected_drug_id"), - expected_id=raw.get("expected_id"), - origin=CaseOrigin(raw["origin"]), - subject_scope=SubjectScope(raw.get("subject_scope", "human")), - ) - for line in handle - if line.strip() - for raw in [json.loads(line)] - ] - - -def run( - cases_path: Path, - documents_path: Path, - parents_path: Path, - aliases_path: Path | None = None, -) -> dict: - documents = load_documents(documents_path) - retrieval = RetrievalService( - InMemoryLexicalRetriever(documents), - InMemoryParentStore(load_parents(parents_path)), - EvidencePolicy(), - ) - service = QueryRoutingService( - retrieval, - CatalogDrugResolver(build_drug_catalog(documents, load_aliases(aliases_path))), - ) - outcomes = [] - details = [] - for case in read_cases(cases_path): - result = service.retrieve( - case.query, - case.subject_scope, - QueryIntent.FACT_LOOKUP, - ) - retrieved = ( - tuple(item.evidence_id for item in result.evidence) - if result.decision != EvidenceDecision.ABSTAIN - else () - ) - outcome = EvaluationOutcome( - case=case, - retrieved_ids=retrieved, - resolved_drug_id=result.resolved_drug_id, - drug_resolution_status=result.drug_resolution_status, - ) - outcomes.append(outcome) - details.append({ - "case_id": case.case_id, - "passed": outcome.passed, - "decision": result.decision, - "reason": result.reason, - "expected_id": case.expected_id, - "retrieved_ids": retrieved, - "expected_drug_id": case.expected_drug_id, - "resolved_drug_id": result.resolved_drug_id, - "drug_resolution_status": result.drug_resolution_status, - }) - return {**summarize(outcomes), "details": details} - - -def main() -> None: - parser = argparse.ArgumentParser() - parser.add_argument("--cases", type=Path, required=True) - parser.add_argument("--documents", type=Path, required=True) - parser.add_argument("--parents", type=Path, required=True) - parser.add_argument("--aliases", type=Path) - args = parser.parse_args() - print(json.dumps( - run(args.cases, args.documents, args.parents, args.aliases), - ensure_ascii=False, - indent=2, - )) - - -if __name__ == "__main__": - main() diff --git a/apps/ai-service/scripts/run_all_evals.py b/apps/ai-service/scripts/run_all_evals.py new file mode 100644 index 0000000..41c4483 --- /dev/null +++ b/apps/ai-service/scripts/run_all_evals.py @@ -0,0 +1,144 @@ +"""Run every eval dataset in `evals/` against a deployment, in one command. + +Before this, only `production_manual_60.jsonl` had a working runner. The other +two datasets sat unreferenced by any code: one's intended consumer was a +metrics dataclass with no entry point, the other's (`rag/run_eval.py`) had +been unable to load the corpus since the ingestion artifact format changed. +So two thirds of the checked-in eval coverage was never actually checked, and +nothing in the repo said so. + +Reuses `run_manual_battery.py`'s checker rather than reimplementing it -- a +second copy of the invariants would drift from the first, and the point of +this script is to widen coverage, not to fork the definition of "correct". +`evals/adapters.py` holds the per-dataset schema mapping and the reasoning +behind each choice. + +Usage: + python3 scripts/run_all_evals.py --base-url https://realvuxbaro.me \\ + --output-dir /tmp/evals +""" + +from __future__ import annotations + +import argparse +import importlib.util +import json +import sys +import time +from pathlib import Path +from typing import Any + +SERVICE_ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(SERVICE_ROOT)) + +from evals.adapters import SUITES # noqa: E402 + + +def _load_battery_module(): + """Import the sibling script by path -- `scripts/` is not a package.""" + path = Path(__file__).resolve().parent / "run_manual_battery.py" + spec = importlib.util.spec_from_file_location("run_manual_battery", path) + module = importlib.util.module_from_spec(spec) + assert spec.loader is not None + spec.loader.exec_module(module) + return module + + +def run_suite( + battery: Any, + name: str, + cases: list[dict[str, Any]], + endpoint: str, + timeout: float, + run_id: str, + output: Path, +) -> tuple[int, int]: + passed = 0 + with output.open("w", encoding="utf-8") as handle: + for index, case in enumerate(cases, start=1): + # Split out rather than nested: reusing the outer quote inside an + # f-string is a 3.12+ syntax, and ruff targets 3.11 here. + default_conversation = f"{name}-{case['id']}" + base = case.get("conversation_id") or default_conversation + conversation_id = f"{base}-{run_id}" + payload = {"content": case["query"], "conversationId": conversation_id} + started = time.monotonic() + try: + raw = battery._post(endpoint, payload, timeout) + response = battery._normalise_response(raw) + failures = battery._check(case, response) + error = None + except Exception as exc: # noqa: BLE001 - recorded, not swallowed + response, failures, error = {}, [f"request error: {exc}"], repr(exc) + elapsed = round(time.monotonic() - started, 3) + ok = not failures + passed += int(ok) + handle.write( + json.dumps( + { + "suite": name, + "case": case, + "passed": ok, + "failures": failures, + "elapsed_seconds": elapsed, + "error": error, + "response": response, + }, + ensure_ascii=False, + ) + + "\n" + ) + handle.flush() + print( + f" [{index:02d}/{len(cases)}] {case['id']} " + f"{'PASS' if ok else 'FAIL'} {elapsed:.1f}s {'; '.join(failures)}", + flush=True, + ) + return passed, len(cases) + + +def main() -> int: + parser = argparse.ArgumentParser() + parser.add_argument("--base-url", required=True) + parser.add_argument("--output-dir", type=Path, required=True) + parser.add_argument("--timeout", type=float, default=120.0) + parser.add_argument("--suites", help="comma-separated subset of: " + ", ".join(SUITES)) + parser.add_argument("--run-id", default=str(int(time.time()))) + args = parser.parse_args() + + wanted = ( + [s.strip() for s in args.suites.split(",") if s.strip()] + if args.suites + else list(SUITES) + ) + unknown = [s for s in wanted if s not in SUITES] + if unknown: + print(f"unknown suite(s): {unknown}; known: {list(SUITES)}", file=sys.stderr) + return 2 + + battery = _load_battery_module() + endpoint = args.base_url.rstrip("/") + "/api/chat" + args.output_dir.mkdir(parents=True, exist_ok=True) + + totals: list[tuple[str, int, int]] = [] + for name in wanted: + cases = SUITES[name]() + print(f"\n=== {name} ({len(cases)} cases) ===", flush=True) + passed, total = run_suite( + battery, name, cases, endpoint, args.timeout, args.run_id, + args.output_dir / f"{name}.jsonl", + ) + totals.append((name, passed, total)) + + print("\n=== SUMMARY ===") + grand_passed = grand_total = 0 + for name, passed, total in totals: + print(f" {name:<16} {passed}/{total}") + grand_passed += passed + grand_total += total + print(f" {'TOTAL':<16} {grand_passed}/{grand_total}") + return 0 if grand_passed == grand_total else 1 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/apps/web/app/_components/AccountMenu.tsx b/apps/web/app/_components/AccountMenu.tsx index a927287..9a1635c 100644 --- a/apps/web/app/_components/AccountMenu.tsx +++ b/apps/web/app/_components/AccountMenu.tsx @@ -27,7 +27,7 @@ export function AccountMenu() { if (!user) { return ( diff --git a/apps/web/app/_components/LoginForm.tsx b/apps/web/app/_components/LoginForm.tsx new file mode 100644 index 0000000..3ebe5b8 --- /dev/null +++ b/apps/web/app/_components/LoginForm.tsx @@ -0,0 +1,135 @@ +"use client"; + +import { useState } from "react"; +import Link from "next/link"; +import { useRouter } from "next/navigation"; +import { login, AuthError } from "@duoc-thu/api-client"; + +interface LoginFormProps { + heading: string; + /** `/admin/login` sets this. It changes only where a successful login + * lands and what a non-admin is told afterwards -- never whether the + * login itself is allowed to happen, which is auth-service's call. */ + requireAdmin?: boolean; +} + +/** + * Shared by `/login` (any doctor) and `/admin/login` (administrators). + * + * The two pages used to be one page, and it only served admins: `AccountMenu` + * pointed its "Đăng nhập" button at `/admin/login`, so a doctor signing in + * from the chat UI with a `user` account got a red "Tài khoản này không có + * quyền quản trị" while the header beside it switched to their name. Both were + * accurate and together they were nonsense -- the login had succeeded and the + * session cookie was already set; only the admin redirect had not happened. + * + * So a non-admin result is reported here as what it is: signed in, without + * administrative rights. Not an error, and not silently swallowed either. + */ +export function LoginForm({ heading, requireAdmin = false }: LoginFormProps) { + const router = useRouter(); + const [username, setUsername] = useState(""); + const [password, setPassword] = useState(""); + const [error, setError] = useState(null); + const [signedInAs, setSignedInAs] = useState(null); + const [pending, setPending] = useState(false); + + async function onSubmit(event: React.FormEvent) { + event.preventDefault(); + setError(null); + setSignedInAs(null); + setPending(true); + try { + const user = await login({ username, password }); + if (requireAdmin && user.role !== "admin") { + // Signed in, just not an administrator. Say so plainly and offer the + // way onward rather than leaving them on a dead form. + setSignedInAs(user.username); + return; + } + router.push(user.role === "admin" && requireAdmin ? "/admin" : "/"); + router.refresh(); + } catch (err) { + setError( + err instanceof AuthError && err.status === 401 + ? "Sai tên đăng nhập hoặc mật khẩu." + : "Không thể đăng nhập lúc này. Vui lòng thử lại." + ); + } finally { + setPending(false); + } + } + + if (signedInAs) { + return ( +
+
+

+ Đã đăng nhập với tài khoản {signedInAs} +

+

+ Tài khoản này không có quyền quản trị, nhưng bạn đã đăng nhập và có + thể tra cứu bình thường. +

+ + Vào trang tra cứu + +
+
+ ); + } + + return ( +
+
+

{heading}

+
+ + setUsername(e.target.value)} + autoComplete="username" + required + /> +
+
+ + setPassword(e.target.value)} + autoComplete="current-password" + required + /> +
+ {error &&

{error}

} + + {!requireAdmin && ( +

+ Không bắt buộc — bạn vẫn tra cứu được mà không cần đăng nhập. +

+ )} +
+
+ ); +} diff --git a/apps/web/app/admin/login/page.tsx b/apps/web/app/admin/login/page.tsx index 6a07e68..fc2ef35 100644 --- a/apps/web/app/admin/login/page.tsx +++ b/apps/web/app/admin/login/page.tsx @@ -1,82 +1,9 @@ -"use client"; - -import { useState } from "react"; -import { useRouter } from "next/navigation"; -import { login, AuthError } from "@duoc-thu/api-client"; +import { LoginForm } from "../../_components/LoginForm"; +/** Administrator entry point. The form itself is shared with `/login`; the + * only difference is that a successful non-admin login is told it lacks + * administrative rights and pointed at the main app, instead of being shown + * a failure message it had already disproved by signing in. */ export default function AdminLoginPage() { - const router = useRouter(); - const [username, setUsername] = useState(""); - const [password, setPassword] = useState(""); - const [error, setError] = useState(null); - const [pending, setPending] = useState(false); - - async function onSubmit(event: React.FormEvent) { - event.preventDefault(); - setError(null); - setPending(true); - try { - const user = await login({ username, password }); - if (user.role !== "admin") { - setError("Tài khoản này không có quyền quản trị."); - return; - } - router.push("/admin"); - router.refresh(); - } catch (err) { - setError( - err instanceof AuthError && err.status === 401 - ? "Sai tên đăng nhập hoặc mật khẩu." - : "Không thể đăng nhập lúc này. Vui lòng thử lại." - ); - } finally { - setPending(false); - } - } - - return ( -
-
-

Đăng nhập quản trị

-
- - setUsername(e.target.value)} - autoComplete="username" - required - /> -
-
- - setPassword(e.target.value)} - autoComplete="current-password" - required - /> -
- {error &&

{error}

} - -
-
- ); + return ; } diff --git a/apps/web/app/login/page.tsx b/apps/web/app/login/page.tsx new file mode 100644 index 0000000..be2b9bb --- /dev/null +++ b/apps/web/app/login/page.tsx @@ -0,0 +1,13 @@ +import { LoginForm } from "../_components/LoginForm"; + +/** + * Login for any doctor, admin or not. Before this existed, `AccountMenu`'s + * "Đăng nhập" button sent everyone to `/admin/login`, which refused every + * non-admin account *after* already signing them in. + * + * Signing in stays entirely optional: anonymous chat is unchanged, and this + * page says so rather than implying an account is required. + */ +export default function LoginPage() { + return ; +} diff --git a/infra/helm/medical-chatbot/values-production.yaml b/infra/helm/medical-chatbot/values-production.yaml index 30f642c..61a8df4 100644 --- a/infra/helm/medical-chatbot/values-production.yaml +++ b/infra/helm/medical-chatbot/values-production.yaml @@ -53,19 +53,21 @@ aiService: answerModelId: qwen.qwen3-next-80b-a3b rerankEnabled: true -# Auth rollout (2026-08-18): stays OFF here for now. Flipping these on -# without secret.jwtSecret already present inline on the live Application -# breaks ArgoCD's render for the WHOLE Application (not just these two -# services) — it did, on the first attempt, and blocked the routine -# ai-service/web image sync along with it. Add secret.jwtSecret inline on -# the Application first (same way secret.grafanaAdminPassword already -# works — see infra/argocd/applications/medical-chatbot-app.yaml), confirm -# ArgoCD picks it up, THEN flip these to true in a follow-up commit. +# Auth is live in production (2026-08-19). These stayed false here for a day +# while the live Application carried `enabled: true` inline, so Git and the +# cluster disagreed and the cluster silently won — recreating the Application +# from Git would have turned auth off without a word. They now match. +# +# secret.jwtSecret is deliberately still absent: it is a real credential and +# lives inline on the Application, the same way secret.grafanaAdminPassword +# does. The chart fails closed via `required` if it is ever missing while +# either service below is on — which is exactly what took the whole render +# down on the first attempt (2026-08-18). Set the secret first, always. authService: - enabled: false + enabled: true apiGateway: - enabled: false + enabled: true observability: grafana: diff --git a/ingestion/ingestion/segment/assembler.py b/ingestion/ingestion/segment/assembler.py index 06b4329..4fc1e7a 100644 --- a/ingestion/ingestion/segment/assembler.py +++ b/ingestion/ingestion/segment/assembler.py @@ -92,9 +92,21 @@ _Event = Union[Heading, _SectionEvent, _TextEvent] # Heading == a title event def _slugify(text: str) -> str: - normalized = unicodedata.normalize("NFKD", text) + # `đ`/`Đ` (U+0111/U+0110) are standalone Vietnamese letters, not a base + # letter plus a combining mark, so NFKD leaves them whole and the ASCII + # encode below then discards them silently -- turning `ĐIỆN GIẢI` into + # `ien_giai`. Exactly three monographs contain `Đ` and all three carried a + # damaged drug_id because of this: GIẢI ĐỘC TỐ UỐN VÁN, KHÁNG ĐỘC TỐ BẠCH + # HẦU, and THUỐC UỐNG BÙ NƯỚC VÀ ĐIỆN GIẢI. Found via the adversarial eval + # suite, where both Oresol cases failed to resolve their monograph. + # + # Casefold before replacing so one pass covers both cases: `Đ` casefolds + # to `đ`. This matches what `entities/catalog.py::normalize_name` already + # does -- that function got it right and this one did not. + folded = text.casefold().replace("đ", "d") + normalized = unicodedata.normalize("NFKD", folded) ascii_text = normalized.encode("ascii", "ignore").decode("ascii") - return re.sub(r"[^a-z0-9]+", "_", ascii_text.lower()).strip("_") + return re.sub(r"[^a-z0-9]+", "_", ascii_text).strip("_") def _starts_its_visual_line(span: Span, previous: Span | None) -> bool: diff --git a/ingestion/tests/test_segment_assembler.py b/ingestion/tests/test_segment_assembler.py index c9d5c47..be36c8f 100644 --- a/ingestion/tests/test_segment_assembler.py +++ b/ingestion/tests/test_segment_assembler.py @@ -3,7 +3,7 @@ from dataclasses import replace import pytest from ingestion.extract.models import Span -from ingestion.segment.assembler import DuplicateDrugIdError, assemble +from ingestion.segment.assembler import DuplicateDrugIdError, _slugify, assemble def _span(text, page, y0, bold=True, size=9.5, printed=None, column="left"): @@ -449,3 +449,37 @@ def test_a_section_name_opening_its_own_line_is_still_a_heading(): ] monograph = list(assemble(spans))[0] assert monograph.sections["chong_chi_dinh"].text == "Suy tủy nặng." + + +@pytest.mark.parametrize( + ("name", "expected"), + [ + ("THUỐC UỐNG BÙ NƯỚC VÀ ĐIỆN GIẢI", "thuoc_uong_bu_nuoc_va_dien_giai"), + ("KHÁNG ĐỘC TỐ BẠCH HẦU", "khang_doc_to_bach_hau"), + ("Điện giải", "dien_giai"), + ("đường huyết", "duong_huyet"), + ], +) +def test_slugify_keeps_d_with_stroke(name, expected): + """`đ`/`Đ` must become `d`, not disappear. + + They are standalone letters (U+0111/U+0110), not a base letter plus a + combining mark, so NFKD leaves them whole and a following ASCII encode + drops them outright. That silently produced `ien_giai` from `ĐIỆN GIẢI`, + and all three monographs in the formulary whose names contain `Đ` carried + a damaged drug_id as a result. + """ + assert _slugify(name) == expected + + +def test_slugify_unchanged_for_names_without_d_stroke(): + """The fix must not move any id that was already correct -- 681 of the + 684 monographs were fine, and changing one of those would orphan its + chunks in the vector store.""" + for name, expected in [ + ("PARACETAMOL (Acetaminophen)", "paracetamol_acetaminophen"), + ("Acid ioxaglic", "acid_ioxaglic"), + ("METFORMIN", "metformin"), + ("Vắc xin uốn ván hấp phụ", "vac_xin_uon_van_hap_phu"), + ]: + assert _slugify(name) == expected