Soften the tone of the docs and comments written today
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
+56
-5
@@ -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`.
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user