291 lines
13 KiB
Markdown
291 lines
13 KiB
Markdown
# Review round 2: verifying the response to round 1
|
|
|
|
Reviewer: Claude, 2026-08-03. Every claim in
|
|
`response-rag-retrieval-2026-08-03.md` was re-run, not read.
|
|
|
|
**Verdict: five findings are genuinely fixed. One is not fixed — it was moved.
|
|
Three new problems appeared in the fix itself.**
|
|
|
|
Reproduction:
|
|
|
|
```
|
|
python -m rag.run_eval \
|
|
--cases evals/manual_adversarial_hard10.jsonl \
|
|
--documents ../../ingestion/scratch/rag-table-pilot/out/all/retrieval_documents.jsonl \
|
|
--parents ../../ingestion/scratch/rag-table-pilot/out/all/logical_tables.jsonl \
|
|
--aliases evals/drug_aliases.json
|
|
-> recall_at_1/3/5 = 0.8889, negative_abstain_rate = 1.0
|
|
```
|
|
|
|
The reported numbers reproduce exactly.
|
|
|
|
---
|
|
|
|
## Confirmed fixed
|
|
|
|
Checked in the code and by re-running, not taken on trust:
|
|
|
|
- **F2** — `QUANTITATIVE_TERMS` and `_structured_boost` are gone. Ranking is
|
|
now real BM25 with IDF over the loaded corpus plus a character-n-gram term.
|
|
No hand-written vocabulary remains in the ranker.
|
|
- **F7** — `run_eval` now constructs `EvidencePolicy()` with the shipped
|
|
defaults.
|
|
- **F8** — `_char_ngrams` operates on `_normalized(text)` in original order.
|
|
The sorted-unique-terms behaviour is gone.
|
|
- **F1 mechanism** — `ambiguity_margin` is removed from `EvidencePolicy` and
|
|
`_is_ambiguous` is deleted. Score ties no longer cause a refusal.
|
|
- **F5 partly** — `run_eval` no longer hands `drug_id` to retrieval. A
|
|
`CatalogDrugResolver` runs first, and the `famciclovia` typo genuinely
|
|
resolves through `SequenceMatcher`; `renal-herpes-typo` now passes as
|
|
`answerable` with resolution actually exercised. This is a real improvement.
|
|
- **F4 partly** — the prose layer now covers **14,915 documents across 684
|
|
drug IDs**, not 6. Also a real improvement.
|
|
- **Verification claims** — all reproduced: `ingestion` **204 passed**,
|
|
`apps/ai-service` **10 passed**, and lint is clean under both `ruff check rag
|
|
tests` *and* the project's stricter `--select F,E9,B,ARG`. The two ARG001
|
|
errors from round 1 are fixed.
|
|
|
|
---
|
|
|
|
## 1. NOT fixed — finding 2 was relocated, not resolved
|
|
|
|
Round 1's finding was: *four of ten passes are bought by a hand-written term
|
|
list drawn from the scored queries.* The ranker is now clean. But the same
|
|
pattern reappeared one layer up, in the thing that replaced it:
|
|
|
|
```python
|
|
VETERINARY_TERMS = frozenset({"gia suc", "gia cam", "meo", "thu y"})
|
|
```
|
|
|
|
plus a special-cased regex for `chó`. The evaluation has exactly **one**
|
|
negative case, and it is about a **mèo**. `negative_abstain_rate: 1.0` is
|
|
computed over **n = 1**, and that one word is in the list.
|
|
|
|
Measured, running the full `QueryRoutingService` against `out/all`:
|
|
|
|
| Query ending | Result |
|
|
|---|---|
|
|
| `... cho mèo` | ABSTAIN `out_of_scope_veterinary` |
|
|
| `... cho chó` | ABSTAIN `out_of_scope_veterinary` |
|
|
| `... cho thỏ` | **ANSWERS** `grounded_evidence_available` |
|
|
| `... cho ngựa` | **ANSWERS** `grounded_evidence_available` |
|
|
| `... cho lợn` | **ANSWERS** `grounded_evidence_available` |
|
|
| `... cho bò sữa` | **ANSWERS** `grounded_evidence_available` |
|
|
| `... cho chuột lang` | **ANSWERS** `grounded_evidence_available` |
|
|
| `... cho vẹt cảnh` | **ANSWERS** `grounded_evidence_available` |
|
|
| `... dùng trong thú cưng` | **ANSWERS** `grounded_evidence_available` |
|
|
|
|
Seven of nine veterinary phrasings are answered with a **human famciclovir
|
|
dose** and the decision `grounded_evidence_available`. Note the last row: `thu
|
|
y` is in the list, but `thú cưng` normalises to `thu cung` and misses.
|
|
|
|
`HumanClinicalScopeGuard` is not a scope guard. It is a five-entry animal-word
|
|
list, and the evaluation that scores it contains exactly the words in it. The
|
|
round-1 objection was never about `_structured_boost` specifically — it was
|
|
about measuring a component against the cases it was written from. That
|
|
objection still stands, unchanged, against this code.
|
|
|
|
A scope guard that generalises cannot be a keyword list. It has to come from
|
|
something the corpus actually says — the book is a human formulary, so the
|
|
question is whether the query's subject is a human patient, not whether it
|
|
contains one of five nouns.
|
|
|
|
## 2. `recall_at_5` is not a measurement
|
|
|
|
`EvidencePolicy.evidence_limit` is 3, so `result.evidence` never exceeds three
|
|
items and `retrieved_ids` never exceeds length 3 — observed lengths across the
|
|
run are `{0, 1, 2, 3}`. `_recall_at(rows, 5)` then slices `[:5]` of a tuple
|
|
that is at most 3 long.
|
|
|
|
**`recall_at_5` is forced to equal `recall_at_3` for every possible input.**
|
|
It is not a third data point; it is `recall_at_3` printed twice. Round 1 asked
|
|
for Recall@1 and Recall@3 reported separately, and that part is done and
|
|
useful — but reporting a third identical figure makes the result look more
|
|
thoroughly measured than it is.
|
|
|
|
Either raise `evidence_limit` above 5 for the diagnostic run, or drop
|
|
`recall_at_5`.
|
|
|
|
## 3. `expected_drug_id` was added and never scored
|
|
|
|
The field exists in `EvaluationCase` and is populated by `read_cases` for all
|
|
10 cases. It appears **nowhere else** — `EvaluationOutcome.passed` and
|
|
`summarize()` never read it.
|
|
|
|
So resolution now happens, but resolution *correctness* is still unmeasured. A
|
|
case that resolves to the wrong drug and then abstains is indistinguishable in
|
|
the report from a case that correctly abstained. That is precisely the
|
|
distinction finding 5 existed to create.
|
|
|
|
Scoring it is a two-line change and would make `ors-who-composition`'s
|
|
`drug_resolution_ambiguous` legible as "resolver declined" rather than an
|
|
unexplained miss.
|
|
|
|
## 4. The alias catalog is one drug out of 684, and it is one under test
|
|
|
|
`evals/drug_aliases.json` in full:
|
|
|
|
```json
|
|
{"thuoc_uong_bu_nuoc_va_ien_giai": ["oresol", "ORS"]}
|
|
```
|
|
|
|
684 drugs in the catalog, hand-aliases for **1**, and that 1 is the drug behind
|
|
two of the ten cases. `docs/v1-delivery-plan.md` §B1 records **344 real
|
|
`X - xem Y` aliases** already extractable from the back index, plus 492
|
|
`ten_thuong_mai` entries (§B2). None are wired in.
|
|
|
|
This is the same shape as finding 2: the coverage that exists is exactly the
|
|
coverage the test needs. Loading the 344 measured aliases would make the
|
|
resolver's alias path testable against something other than itself.
|
|
|
|
## 5. "out/all" does not mean the whole book, and "out/100" no longer means anything
|
|
|
|
Measured from the manifests and the artifacts:
|
|
|
|
| artifact | manifest pages | docs | drugs | prose | table_whole | table_row | formula |
|
|
|---|---|---|---|---|---|---|---|
|
|
| `out/all` | 116 | 15,727 | 684 | 14,915 | 133 | 669 | 10 |
|
|
| `out/100` | 100 | 15,593 | 684 | 14,915 | 116 | 552 | 10 |
|
|
| `out/hard10` | 10 | 164 | 6 | 130 | 4 | 28 | 2 |
|
|
|
|
Two things follow.
|
|
|
|
The prose layer is now genuinely whole-corpus (identical 14,915 documents in
|
|
both), which is the real fix and deserves the credit. But the **table/formula
|
|
layer in `out/all` covers 116 pages**, and its 133 parents + 10 formulas match
|
|
the 133 logical parents + 10 formulas recorded in the progress log — so "all"
|
|
is honest *for tables* and misleading as a general label. The response's
|
|
sentence "`out/all` now has 15,727 documents across all 684 drug IDs" is
|
|
literally true and reads as whole-book coverage of everything, which it is not.
|
|
|
|
Second: `out/100` and `out/all` now differ by 17 tables and 117 rows and
|
|
nothing else. The 100-page scope has stopped being a distinct scope. Either
|
|
retire it or say what it is for.
|
|
|
|
## 6. `assert` on the request path
|
|
|
|
`routing.py:131` — `assert resolution.drug_id is not None`. Assertions are
|
|
removed under `python -O`, at which point `retrieve` is called with `None`.
|
|
Minor, but it is in the live path; make it an explicit raise.
|
|
|
|
## On the ORS miss
|
|
|
|
The response calls the one positive miss "intentionally safe". That is
|
|
defensible — "Công thức oresol WHO UNICEF pha một lít có bao nhiêu **natri
|
|
clorid**?" does name two catalog entities, and declining beats guessing.
|
|
|
|
Worth stating the cost plainly, though: this is now the **second** mechanism
|
|
that refuses an answerable clinical question (the tie was the first, and it is
|
|
gone). Any question naming a drug and one of its ingredients will hit it, and
|
|
that pattern is common in a formulary. It is untested beyond this single case.
|
|
Not a defect — an accepted trade-off that should be measured before it is
|
|
called safe.
|
|
|
|
---
|
|
|
|
## 7. Added after the fact — the alias gap makes common drugs unreachable
|
|
|
|
This came out of testing §4's practical effect and is **more serious than §1**.
|
|
|
|
Monograph headings that carry a parenthesised synonym become a single
|
|
compound `drug_id`, and `build_drug_catalog` produces no alias for either
|
|
part:
|
|
|
|
```
|
|
paracetamol_acetaminophen -> {"paracetamol acetaminophen",
|
|
"PARACETAMOL (Acetaminophen)"}
|
|
acid_acetylsalicylic_aspirin -> {"acid acetylsalicylic aspirin",
|
|
"ACID ACETYLSALICYLIC (Aspirin)"}
|
|
```
|
|
|
|
Measured against `out/all`:
|
|
|
|
| Query | Resolution |
|
|
|---|---|
|
|
| `Liều paracetamol cho người lớn là bao nhiêu?` | **`not_found`** |
|
|
| `Chống chỉ định của aspirin là gì?` | **`not_found`** |
|
|
| `Liều paracetamol acetaminophen cho người lớn?` | `resolved` |
|
|
| `Liều metformin cho người lớn là bao nhiêu?` | `resolved` |
|
|
|
|
Two of the most-asked-about drugs in any formulary are unreachable unless the
|
|
user types the book's exact compound heading. It fails *safely* — it abstains
|
|
rather than answering wrongly — which is exactly why the 8/9 diagnostic cannot
|
|
see it: none of the ten cases involves a parenthesised heading.
|
|
|
|
`docs/v1-delivery-plan.md` §B1/§B2 already record **344 `X - xem Y` aliases**
|
|
and **492 `ten_thuong_mai` entries** as extractable. Until they are loaded,
|
|
resolver coverage is whatever the headings happen to spell.
|
|
|
|
**Correction to my own suspicion.** I expected the response's claim — "queries
|
|
with multiple distinct drug entities abstain as ambiguous" — to be false,
|
|
because `Nên dùng paracetamol hay ibuprofen cho trẻ sốt cao?` answers about
|
|
ibuprofen alone. It is not false. Re-tested with two drugs that are both in
|
|
the catalog:
|
|
|
|
```
|
|
Tuong tac giua digoxin va amiodaron -> ambiguous
|
|
Nen dung omeprazol hay pantoprazol -> ambiguous
|
|
Tuong tac giua warfarin va amiodaron -> ambiguous
|
|
```
|
|
|
|
The multi-entity guard works. The paracetamol/ibuprofen query slips through
|
|
because paracetamol is *not reachable at all*, so the query looks
|
|
single-entity. The alias gap does not merely reduce coverage — it silently
|
|
disables the ambiguity protection that Codex is relying on.
|
|
|
|
## 8. "Veterinary" is not a requirement this project ever had
|
|
|
|
Worth saying plainly, because §1 spent the entire fix budget on it. The
|
|
veterinary category exists in this codebase for one reason: Codex wrote one
|
|
negative eval case about a cat, round 1 showed it passed by coincidence, and
|
|
the repair was a guard for cats.
|
|
|
|
The out-of-scope categories the project documents are different ones —
|
|
`docs/v1-delivery-plan.md` §8 (general chapters printed 37-98 and appendices
|
|
1497-1528 are not in the corpus) and `docs/architecture.md` (scoped refusal
|
|
for questions that are not formulary lookups). Measured against `out/all`:
|
|
|
|
| Category | Result | Reason |
|
|
|---|---|---|
|
|
| general chapter — "nguyên tắc kê đơn thuốc" | ABSTAIN | `drug_not_resolved` |
|
|
| general chapter — "ngộ độc và thuốc giải độc" | ABSTAIN | `drug_not_resolved` |
|
|
| appendix — "bảng tương hợp thuốc tiêm truyền" | ABSTAIN | `drug_not_resolved` |
|
|
| drug outside the formulary — semaglutid | ABSTAIN | `drug_not_resolved` |
|
|
| symptom diagnosis — "tôi đau đầu buồn nôn" | ABSTAIN | `drug_not_resolved` |
|
|
| **recommendation — "nên dùng X hay Y cho trẻ sốt cao"** | **ANSWERS** | `grounded_evidence_available` |
|
|
|
|
Five of six abstain, but none of them because scope was checked — they abstain
|
|
because no drug name matched, which is `drug_not_resolved` doing scope work by
|
|
accident. The one that gets through is the recommendation question, which
|
|
`architecture.md` explicitly says must be refused.
|
|
|
|
So the guard covers a category nobody asked for, covers it with five words,
|
|
and the category that *is* specified is unhandled.
|
|
|
|
## Summary
|
|
|
|
| Round-1 finding | Status |
|
|
|---|---|
|
|
| 1 — refusal was a score tie | mechanism removed; **replacement is a 5-word list, see §1** |
|
|
| 2 — boost tuned on scored queries | fixed in the ranker; **pattern reappears in the scope guard** |
|
|
| 3 — Recall@3 sold as Recall@1 | fixed; **but `recall_at_5` is padding, see §2** |
|
|
| 4 — eval locked to 6 drugs | prose fixed (684 drugs); table layer still 116 pages |
|
|
| 5 — drug_id handed in | resolver added and works; **correctness still unscored, see §3** |
|
|
| 6 — manual cases in expert gate | fixed; `expert_release_gate` now has 0 cases and null metrics |
|
|
| 7 — eval used non-default policy | fixed |
|
|
| 8 — char n-grams sorted | fixed |
|
|
|
|
Priority order:
|
|
|
|
1. **§7 — the alias gap.** `Liều paracetamol cho người lớn?` returns
|
|
`not_found`. It is the most likely question a real user asks, it fails
|
|
today, and it also disables the multi-entity ambiguity guard. Loading the
|
|
344 back-index aliases and splitting parenthesised headings fixes both.
|
|
2. **§1 — the scope guard.** Seven of nine veterinary phrasings are answered
|
|
with a human dose under the label `grounded_evidence_available`. Lower than
|
|
§7 only because a doctor is unlikely to ask it; the label is what makes it
|
|
dangerous.
|
|
3. **§8** — the specified out-of-scope category (recommendation questions) is
|
|
unhandled while an unspecified one has a guard.
|
|
4. §2, §3, §4, §6 — reporting and coverage bookkeeping.
|