139 lines
7.5 KiB
Markdown
139 lines
7.5 KiB
Markdown
# Claude ownership claim — 2026-08-12
|
||
|
||
Scope respected per `WORK_SPLIT_2026-08-10.md`: Claude owns `infra/**`,
|
||
Dockerfiles, deployment config and `.github/**`; Codex owns
|
||
`apps/ai-service/rag/**` and the retrieval adapters. **Nothing under `rag/`,
|
||
`adapters/`, or `ingestion/` was modified.**
|
||
|
||
## Context
|
||
|
||
The owner asked for a full reverse-engineered documentation rewrite, then for
|
||
the highest-value findings to be fixed. `docs/00`–`29` + `docs/README.md` +
|
||
`docs/DOCUMENTATION_PLAN.md` + `docs/adr/README.md` + ADR 0009/0010 are new and
|
||
are now the current-state reference; the pre-existing documents in `docs/` were
|
||
deliberately kept, not deleted or edited.
|
||
|
||
Everything below was verified on local before being written up. **Nothing was
|
||
pushed or deployed.**
|
||
|
||
## Files changed
|
||
|
||
| File | Status | Why |
|
||
|---|---|---|
|
||
| `.github/workflows/ci.yml` | **new** | No test gate existed at all — `deploy.yml` went from `push: master` straight to SSH + rebuild. 555 tests had never run in CI. |
|
||
| `apps/ai-service/tests/conftest.py` | **new** | `pytest tests -q` failed at *collection*, not at a test, on any machine without Qdrant. |
|
||
| `apps/ai-service/.env.example` | **new** | No env template existed anywhere; `config.py` was the only record of ~22 settings. |
|
||
| `apps/web/middleware.ts` | edited | `/api/pdf` (37 MB per request) and `/api/feedback` had **no** rate limit — `matchRules` only had entries for `/api/chat` and `/api/suggest`, so everything else fell through to `NextResponse.next()`. |
|
||
| `.gitignore` | edited | ~25 untracked `.codex-*.log`/`.png` scratch files at the repo root and in `apps/ai-service/`. |
|
||
|
||
### `tests/conftest.py` — the detail that matters to Codex
|
||
|
||
`main.py` calls `build_runtime(get_settings())` at module scope and
|
||
`tests/test_api.py` imports `main`, so with `EMBEDDING_PROVIDER=cohere-v4` (the
|
||
code default, and what a developer `.env` sets) pytest opens a `QdrantClient`
|
||
during collection. The conftest does one thing:
|
||
|
||
```python
|
||
os.environ.setdefault("EMBEDDING_PROVIDER", "disabled")
|
||
```
|
||
|
||
`setdefault`, not assignment — an explicit
|
||
`EMBEDDING_PROVIDER=cohere-v4 pytest ...` against a local Qdrant still behaves
|
||
exactly as before. Verified both ways locally.
|
||
|
||
## Local verification (nothing pushed)
|
||
|
||
| Check | Before | After |
|
||
|---|---|---|
|
||
| `cd apps/ai-service && pytest tests -q` (no env var) | **collection ERROR** — `ResponseHandlingException`, Qdrant refused | **278 passed, 6 skipped** in 2.6 s |
|
||
| `EMBEDDING_PROVIDER=cohere-v4 pytest tests/test_api.py -q` | collection error | collection error — override still wins, as intended |
|
||
| `cd apps/ai-service && ruff check .` | passed | passed |
|
||
| `cd ingestion && pytest tests -q` | 277 passed / 12 skipped | **277 passed, 12 skipped** (unchanged) |
|
||
| `next lint` (apps/web) | clean | clean |
|
||
| `next build` (apps/web) | exit 0 | exit 0 |
|
||
| `GET /api/pdf` headers | no rate-limit header at all | `x-ratelimit-limit: 30` |
|
||
| 64 × `POST /api/feedback` | all reached the handler | 60 × 400 (reached handler), then **4 × 429 with `Retry-After: 60`** |
|
||
| `GET /api/chat`, `/api/suggest` headers | 12 / 120 | 12 / 120 — unchanged |
|
||
|
||
## Deliberately NOT done — production risk
|
||
|
||
**`infra/docker/docker-compose.prod.yml` was left alone.** The committed
|
||
default credential (`POSTGRES_USER: duoc_thu` / `POSTGRES_PASSWORD: duoc_thu`,
|
||
mirrored as `secret.postgresPassword` in the Helm values) is a real finding, but
|
||
parameterising it as `${POSTGRES_PASSWORD:-duoc_thu}` would be a half-measure
|
||
with non-zero risk: the default still sits in Git, and the deploy runs
|
||
`sudo -E docker compose`, so an unrelated `POSTGRES_PASSWORD` in the host's
|
||
environment would give the postgres container a password that `.env.prod`'s DSN
|
||
does not match. Fixing this properly is a coordinated rotation (new password →
|
||
`.env.prod` DSN → `ALTER ROLE`), which is the owner's call, not a drive-by edit.
|
||
|
||
**`ci.yml` does not gate `deploy.yml`.** They are independent workflows, so a
|
||
red CI currently does not stop a deploy. Wiring `deploy` to `needs:` the CI jobs
|
||
is the actual fix for the gap, but it changes deploy behaviour — a flaky job
|
||
would block a production deploy — so it is left for the owner to approve as a
|
||
one-line follow-up.
|
||
|
||
**`ruff` is not run over `ingestion/`.** `ingestion/pyproject.toml` declares no
|
||
`[tool.ruff]` section, so ruff applies its full default rule set and reports
|
||
~426 pre-existing findings. Adding the same lint config `apps/ai-service` uses
|
||
is follow-up work; nothing was silenced or auto-fixed.
|
||
|
||
## Handoff to Codex — findings inside `rag/**`, not touched
|
||
|
||
All verified by reading the code, several by import-graph grep. Full write-ups
|
||
with file references are in `docs/26-known-limitations.md` and
|
||
`docs/27-technical-debt.md`.
|
||
|
||
1. **`rag/fusion.py`, `rag/expansion.py`, `rag/calculators.py` have zero runtime
|
||
callers** — referenced only by their own tests. `calculators.py`
|
||
(`body_surface_area_m2`, the book's DuBois formula) was written specifically
|
||
so a BSA dose would be *computed* rather than read off a quarantined table;
|
||
that wiring never happened. (docs/27 D-12)
|
||
|
||
2. **`search_lexical` issues Qdrant `MatchText` against the `text` payload
|
||
field, which is not in `INDEXED_PAYLOAD_FIELDS`**
|
||
(`ingestion/load/models.py`). Qdrant needs an explicit full-text index for
|
||
`MatchText`. If the deployed collection has no such index, the neighbour
|
||
pooling and the patient-safety facet routes are effectively relying on the
|
||
Python re-scoring of whatever the scroll returned. **Worth checking the live
|
||
collection's index list before changing anything.** (docs/27 D-08)
|
||
|
||
3. **Four Prometheus metrics are registered but never incremented** —
|
||
`duocthu_loop_retrieval_rounds_total`, `duocthu_loop_refined_total`,
|
||
`duocthu_loop_repaired_total`, `duocthu_followup_inherited_total`. Leftovers
|
||
of the ADR 0007 loop that ADR 0008 replaced. They will always read 0, which
|
||
on a dashboard reads as "this never happens" rather than "this is not
|
||
measured". (docs/27 D-13)
|
||
|
||
4. **`SECTION_ORDER` in `rag/sections.py` has 18 entries; `ten_thuong_mai` is
|
||
missing** while `SECTION_KEYS` and the corpus both have 19. In
|
||
`find_by_drug`, that chunk sorts to the end via the
|
||
`order.get(..., len(order))` default instead of into its book position.
|
||
(docs/27 D-25)
|
||
|
||
5. **`RagAgent._last_frame` and `_clarify_streak` are still in-process dicts.**
|
||
Only `_history` got `PostgresConversationStore`. Running more than one
|
||
`ai-service` replica silently degrades multi-turn quality — the prior-frame
|
||
merge and the clarify circuit breaker both become per-replica — and nothing
|
||
detects it. (docs/27 D-04)
|
||
|
||
6. **No evaluation runner exists.** `Golden Dataset/*.csv` (209 labelled rows)
|
||
is read by no code, and `rag/condition_evaluation.py` +
|
||
`rag/evaluation.py` implement complete metric summaries with no production
|
||
caller. `rag/run_eval.py` measures the in-memory retriever, not Qdrant.
|
||
Wiring `evals/condition_to_drug_v1.jsonl` through the live service into
|
||
`summarize_condition_outcomes` is existing tested code, not new design.
|
||
(docs/19, docs/27 D-10)
|
||
|
||
7. **Optional retriever capabilities are discovered with `getattr`, not
|
||
declared in `ports.py`** — `find_by_indication`, `search_indication`,
|
||
`search_lexical`, `find_by_drug`. A retriever missing one silently disables a
|
||
whole route. (docs/27 D-14)
|
||
|
||
## Not modified
|
||
|
||
`apps/ai-service/rag/**`, `apps/ai-service/adapters/**`,
|
||
`apps/ai-service/routers/**`, `apps/ai-service/main.py`, `bootstrap.py`,
|
||
`config.py`, `ingestion/**`, `infra/**`, both Dockerfiles, `packages/**`, and
|
||
every pre-existing file in `docs/`.
|