Files
duocthu/docs-legacy/27-technical-debt.md
T

14 KiB

27 — Technical debt

Only items with a clear technical justification are listed. Architectural choices that are deliberate and documented in-code (fail-open trace writes, one entailment pass, character-exact number comparison, quarantine over reconstruction) are not debt and are not listed here.

Priority: P0 production risk now · P1 likely to cause an incident or block work · P2 real but tolerable · P3 cleanliness.


P0

D-01 — No test gate before production deploy

Evidence .github/workflows/deploy.yml is the only workflow; it triggers on push: master and goes straight to SSH + docker compose up --build. No ruff, no pytest, no tsc, no pull_request trigger. ruff is configured in apps/ai-service/pyproject.toml and never invoked. Impact A commit that breaks all 555 passing tests deploys to a live medical reference tool. The only automated check is one smoke query after the fact. Risk High — the safety mechanisms (grounding, entailment, quarantine, scope gates) are exactly what the tests cover. Remediation Add a workflow running both suites (EMBEDDING_PROVIDER=disabled for ai-service) plus ruff check and turbo run lint build, on push and pull_request; make deploy needs: it.

D-02 — Committed default PostgreSQL credential

Evidence infra/docker/docker-compose.prod.yml sets POSTGRES_USER: duoc_thu / POSTGRES_PASSWORD: duoc_thu; infra/helm/medical-chatbot/values.yaml sets secret.postgresPassword: duoc_thu and grafanaAdminPassword: change-me. Impact A default credential in version control. Bounded today because PostgreSQL publishes no host port, but the database holds raw user queries that can contain patient context, and the Helm path would carry it into a cluster. Remediation Generate a password, inject via .env.prod / a Kubernetes Secret, and remove the literals from both files.

D-03 — No backup for either datastore

Evidence No dump job, cron, snapshot script or restore procedure anywhere in the repository. Docker named volumes on a single EC2 host. Impact Losing the host loses every trace, conversation and feedback row, and requires a full Qdrant re-load — whose embed step costs real Bedrock spend. Remediation A scheduled pg_dump and a Qdrant snapshot to S3, plus a written restore drill.


P1

D-04 — In-process agent state blocks horizontal scaling, silently

Evidence rag/agent.py keeps _last_frame and _clarify_streak as plain dicts. PostgresConversationStore replaces _history only. Impact With more than one replica, the structured prior-frame merge (which stops the model re-asking an answered clarify) and the clarify circuit breaker both become per-replica. Multi-turn quality degrades and nothing detects it. Remediation Persist both alongside the conversation turns, or document a hard single-replica constraint in the chart and enforce it.

D-05 — Test suite cannot be collected without an undocumented env var

Evidence main.py calls build_runtime() at module scope; tests/test_api.py imports main; with the repo's .env present, collection raises ResponseHandlingException against Qdrant. Impact A new contributor's first pytest run fails in a way that looks like a broken suite. Reproduced this session. Remediation Either move runtime construction behind a factory the tests can avoid (an app factory already exists — create_app), or add a conftest.py that sets EMBEDDING_PROVIDER=disabled. The latter is a two-line change.

D-06 — No .env.example

Evidence git ls-files shows no env template; the only env file is the gitignored apps/ai-service/.env. config.py is the sole record of ~22 settings. Impact Nobody can configure the service without reading the source, and production's .env.prod cannot be reviewed or reconstructed. Remediation Commit apps/ai-service/.env.example with every key, safe defaults and a comment per secret.

D-07 — No Python lockfile; the Dockerfile duplicates and diverges from pyproject.toml

Evidence apps/ai-service/Dockerfile pip-installs a hand-written list including boto3 with no version bound, and comments that this mirrors pyproject.toml plus extras. pyproject.toml itself does not declare boto3 at all, though adapters/ imports it. Impact Two builds of the same commit can install different versions; the declared dependency set is incomplete; a boto3 breaking change reaches production unannounced. Remediation Declare boto3 in pyproject.toml, generate a lockfile (uv/pip-compile), and have the Dockerfile install from it.

D-08 — search_lexical uses MatchText on an unindexed payload field

Evidence INDEXED_PAYLOAD_FIELDS in ingestion/load/models.py contains no entry for text; adapters/qdrant.py::search_lexical builds FieldCondition(key="text", match=MatchText(...)) conditions. Impact Qdrant requires an explicit full-text index for MatchText. Without one those conditions may not filter as intended, which would make the lexical route depend entirely on the Python re-scoring of whatever the scroll returned. The neighbour-pooling and patient-safety facet routes both use it. Remediation Verify the deployed collection's index list; if absent, add a text field index to INDEXED_PAYLOAD_FIELDS and create it on the existing collection.

D-09 — Rate limiting is in-memory, in the wrong tier, and does not cover every route

Evidence apps/web/middleware.ts — per-process counters, IP-keyed, rules only for /api/chat and /api/suggest. /api/pdf (37 MB per request) and /api/feedback fall through unlimited. The file documents the first two limitations itself. Impact The only guard on unauthenticated Bedrock spend does not survive a second replica, and a 37 MB endpoint is unthrottled. Remediation Add rules for the remaining routes now; move counters to Redis (already reserved for this) or to the gateway when it exists.

D-10 — No evaluation runner despite substantial evaluation material

Evidence 209 hand-labelled rows in Golden Dataset/*.csv read by no code; rag/condition_evaluation.py and rag/evaluation.py implement full metric summaries with no production caller; rag/run_eval.py measures the in-memory retriever, not Qdrant. Impact No retrieval or answer-quality number can be reproduced, so no regression is detectable. Assertions about RAG quality cannot be supported. Remediation Wire evals/condition_to_drug_v1.jsonl through the live service into summarize_condition_outcomes, store a baseline, and fail on regression.

D-11 — Zero frontend tests for code that encodes fixed production bugs

Evidence No test script, no test files, no runner in apps/web or packages/*. The untested code includes the 65 s timeout derivation, abort handling, the Strict-Mode duplicate-request guard, the ~25-entry REFUSALS map and the citation-grouping logic — each added in response to a real incident, each documented in a comment. Impact Silent regression of user-visible safety wording and behaviour. Remediation Vitest + Testing Library for route.ts mapping and middleware.ts rules first; those are pure functions and cheap to cover.


P2

D-12 — Dead code: three tested modules with no runtime caller

Evidence Verified by import-graph grep: rag/fusion.py (reciprocal_rank_fusion), rag/expansion.py (expand_siblings) and rag/calculators.py (body_surface_area_m2) are referenced only by their own tests. Impact ~140 lines plus 8 tests suggesting capabilities the system does not have. calculators.py is the notable one — it was written specifically so a BSA-based dose would be computed rather than read off a quarantined table, and that never happened. Remediation Wire calculators.py into the dosing path or record why not; delete or clearly mark fusion.py/expansion.py as unused experiments.

D-13 — Four Prometheus metrics registered but never incremented

Evidence duocthu_loop_retrieval_rounds_total, duocthu_loop_refined_total, duocthu_loop_repaired_total, duocthu_followup_inherited_total appear only in rag/metrics.py and in the registration/help tables of adapters/prometheus.py. Leftovers of the retired ADR 0007 loop. Impact Permanently-zero series that read as "this never happens" rather than "this is not measured". A dashboard panel on them would be misleading. Remediation Delete them, or re-point them at the paths that replaced the loop.

D-14 — Optional retriever capabilities discovered by getattr, not by protocol

Evidence rag/service.py probes find_by_section, find_by_indication, search_indication, search_lexical and find_by_drug with getattr(self._retriever, name, None). rag/ports.py declares only search, find_by_section (on a separate SectionRetriever) and ParentStore.get. Impact A retriever missing a method silently disables a whole route instead of failing loudly, and the real interface is undocumented. Remediation Declare the full capability set in ports.py as explicit optional protocols.

D-15 — Hand-maintained contract between Pydantic models and TypeScript DTOs

Evidence routers/rag.py response models, packages/shared-types/src/dto/ chat.ts, and the snake_case→camelCase mapping in apps/web/app/api/chat/route.ts are three independent hand-written copies of one contract. Impact A new backend field is silently dropped until three files are edited; a renamed field fails at runtime, not at build time. Remediation Generate the TS types from the OpenAPI schema FastAPI already serves.

D-16 — Container images run as root with build tooling included

Evidence Neither Dockerfile has a USER; apps/ai-service/Dockerfile installs gcc into the runtime image; apps/web's runtime stage copies the whole /repo rather than Next's standalone output. Impact Larger attack surface and image size than necessary. Remediation Multi-stage build for ai-service, non-root user in both, output: "standalone" for Next.

D-17 — No Kubernetes hardening in the Helm chart

Evidence No securityContext, runAsNonRoot, readOnlyRootFilesystem, NetworkPolicy, PodDisruptionBudget or HPA in infra/helm/medical-chatbot/templates/. A ServiceAccount is created with no RBAC bound. Impact Not a current production risk (the chart is unapplied) but it is the target state. Remediation Add them before the chart is ever applied.

D-18 — Helm chart cannot produce a working deployment as written

Evidence values.yaml defaults embeddingProvider/answerProvider to disabled, and the bundled Qdrant starts empty — against which ai-service's manifest check refuses to start. There is no corpus-load Job in the chart, and no image registry produces duocthu-ai-service:<tag>. Impact The documented target deployment path is not runnable. Remediation Add a corpus-restore Job (or document a snapshot prerequisite) and produce tagged images in CI.

D-19 — Migrations are forward-only with no version tracking

Evidence migrate.py replays every migrations/*.sql on each deploy; all are IF NOT EXISTS. No version table, no down scripts, no ordering guard beyond filenames. Impact Works today because the migrations are trivially idempotent; the first non-idempotent migration breaks it, and rollback across one is uncovered. Remediation Adopt a real migration tool, or add a schema_migrations table.

D-20 — Per-call PostgreSQL connections with no pool

Evidence adapters/postgres.py — both classes open a fresh psycopg.connect() per call. Both docstrings acknowledge it (F-09). Impact Connection setup on every trace write, history read and history append: three round trips of connection overhead per turn. Bounded by connect_timeout=5, which is what keeps it from being worse. Remediation psycopg_pool with a startup lifecycle.


P3

D-21 — Unpinned qdrant/qdrant:latest

Every other image is pinned. A rebuild can move the Qdrant version underneath a loaded collection.

D-22 — Routine timings logged at WARNING

rag/agent.py logs per-turn timing unconditionally at warning level, with a comment explaining that uvicorn does not wire handlers onto the root logger so info would go nowhere. The instrumentation is temporary (added 2026-08-07 to chase a specific bug) and now duplicates duocthu_stage_duration_seconds. It also pollutes the WARNING level, which makes real warnings hard to spot.

D-23 — Repository root and working tree noise

~25 untracked .codex-*.log / .codex-*.png files, tmp/, output/, .venv_docling_test/, .next/ and .turbo/ at the root; apps/ai-service/ holds a dozen .codex-live-80xx.*.log files. None is gitignored.

D-24 — @duoc-thu/api-client is a declared but unused dependency

apps/web depends on it and the live chat path calls fetch("/api/chat") directly. Either adopt it or drop the dependency.

D-25 — SECTION_ORDER omits ten_thuong_mai

rag/sections.py::SECTION_ORDER has 18 entries while SECTION_KEYS (and the corpus) has 19. In find_by_drug, a ten_thuong_mai chunk sorts to the end via the order.get(..., len(order)) default rather than into its book position.

D-26 — Duplicated section vocabulary across two projects

The 19 keys are defined independently in ingestion/segment/vocab.py, apps/ai-service/rag/sections.py and apps/ai-service/rag/understanding.py. The separation between the two deployables is deliberate, but the two copies inside ai-service are not.