diff --git a/.env.example b/.env.example index 9b38a66..03d78eb 100644 --- a/.env.example +++ b/.env.example @@ -77,6 +77,14 @@ API_DOMAIN=api.contract-intelligence.example.com # Pipeline behavior QUERY_REWRITE_STRATEGY=hyde +# Heuristic metadata-filter vocabulary (see pipeline/constants.py). A keyword in +# the question pins source_doctype / status before retrieval. Per-client config — +# override only when the client's ERPNext uses custom doctype names or status +# values (set during the Phase 5 guided review). JSON; the status form is a +# {keyword: status-value} map. Leave unset to use the shipped defaults. +# METADATA_FILTER_DOCTYPE_KEYWORDS={"Contract": ["contract"], "Terms and Conditions": ["terms and conditions"]} +# METADATA_FILTER_STATUS_KEYWORDS={"cancelled": "Cancelled", "active": "Active", "unsigned": "Unsigned"} + # Model configuration (see config.py) — change to swap the generation or # embedding model without touching code. # WARNING: EMBEDDING_MODEL must map to a known dimension in config.py's diff --git a/CLAUDE.md b/CLAUDE.md index a190f77..0e2f22e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -189,7 +189,8 @@ so ingestion embedding cost is tracked (see `docs/ARCHITECTURE.md` § Observabil query, improving recall for abstract questions; step-back rewrites the question at a higher abstraction level instead. The chat call runs on `REWRITE_MODEL` (default `gpt-4o-mini`), not `OPENAI_MODEL`. 2. Metadata filter extraction (pure keyword heuristic — doctype/status only, no LLM call, no date-range - parsing) from the original question. + parsing) from the original question. The keyword vocabulary lives in `pipeline/constants.py` + (`METADATA_FILTER_DOCTYPE_KEYWORDS` / `METADATA_FILTER_STATUS_KEYWORDS`), env-overridable per client. 3. `HybridSearch.search()` — BM25 + Qdrant vector search in parallel, fused via RRF, top-20. 4. `Reranker.rerank()` — cross-encoder scores all 20 `(query, chunk)` pairs, returns top-5. 5. GPT-4o generation with a system prompt that requires `[docname]`-style citations per claim and forbids diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 857b75a..0bd7b8a 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -48,7 +48,7 @@ the pipeline a top-5. See "Query Pipeline — Step by Step" below. ## Query Pipeline — Step by Step 1. **Query rewriting** — HyDE generates a hypothetical answer; its embedding becomes the query vector. This improves recall for abstract questions. -2. **Metadata filter extraction** — pure keyword matching (`_extract_filters()` in `query_pipeline.py`) against doctype and status keyword lists in the original question. No LLM call and no date-range parsing — only `source_doctype` and `status` are ever set this way; date/supplier filters come solely from the frontend sidebar. +2. **Metadata filter extraction** — pure keyword matching (`_extract_filters()` in `query_pipeline.py`) against the doctype and status keyword vocabulary in `pipeline/constants.py` (`METADATA_FILTER_DOCTYPE_KEYWORDS` / `METADATA_FILTER_STATUS_KEYWORDS`, env-overridable per client — #135). No LLM call and no date-range parsing — only `source_doctype` and `status` are ever set this way; date/supplier filters come solely from the frontend sidebar. 3. **Hybrid search** — BM25 (lexical) and Qdrant vector search run in parallel. Results are fused with Reciprocal Rank Fusion (`k=60`), returning 20 candidates. Metadata filters (supplier, doctype, status) are honoured by **both** legs: Qdrant filters during ANN traversal; `rank_bm25` has no filter hook, so the ranked BM25 list is filtered in Python before fusion (`_passes_filter`, mirroring `VectorStore._build_filter`). Every chunk in the fused set therefore satisfies the filter — it's a hard constraint (#98). 4. **Cross-encoder reranking** — `ms-marco-MiniLM` scores all 20 `(query, chunk)` pairs and returns the top 5. 5. **Generation** — GPT-4o receives the top-5 chunks as context with a structured prompt that requires source citations. diff --git a/docs/CLIENT_DEPLOYMENT_RUNBOOK.md b/docs/CLIENT_DEPLOYMENT_RUNBOOK.md index 56d37ac..664cbeb 100644 --- a/docs/CLIENT_DEPLOYMENT_RUNBOOK.md +++ b/docs/CLIENT_DEPLOYMENT_RUNBOOK.md @@ -299,9 +299,25 @@ docker compose exec app python3 -c \ ### Guided review with the client expert -10–15 real questions the client's team actually asks. Run them live, eyeball answers and -citations together. This is the default acceptance step — the repo's own `CLAUDE.md` notes the -LLM judge is too noisy to gate on at any dataset size. +**This is the default acceptance step** — the repo's own `CLAUDE.md` notes the LLM judge is too +noisy to gate on at any dataset size. The formal RAGAS benchmark below is an optional, separately +scoped add-on. + +- [ ] Copy `docs/templates/guided_review.md` → `evaluation/client/_guided_review.md` + (gitignored — it names real documents) and fill in the header +- [ ] The client expert brings **10–15 real questions** their team actually asks — mix single-clause + lookups, cross-document ("which of our contracts…"), and 1–2 you expect it to *refuse* +- [ ] Run them live, score each row against the worksheet rubric (answer verdict + citation verdict) +- [ ] **Metadata-filter vocabulary check** — does the client's ERPNext use doctype names or `status` + values other than the defaults (`Contract` / `Terms and Conditions`; `Cancelled` / `Active` / + `Unsigned`)? Check a real `status` value in Qdrant against their wording. If so, set + `METADATA_FILTER_DOCTYPE_KEYWORDS` / `METADATA_FILTER_STATUS_KEYWORDS` in `.env` (JSON — see the + Environment reference) and re-test the affected questions. No source edit, no re-ingest. +- [ ] When a group of questions fails the same way, nudge **one knob**, in the reach order from + `docs/PIPELINE_TUNING.md` § Per-client tuning (`RETRIEVAL_TOP_K` → prompt vocabulary → + `RERANK_TOP_N`), and re-run the failing questions +- [ ] Record the decision + any `.env` changes in the worksheet — it is the validation summary for + the client's deployment record ### Optional: client-specific RAGAS benchmark @@ -416,6 +432,8 @@ LLM judge is too noisy to gate on at any dataset size. | `FRONTEND_URL` / `PUBLIC_API_URL` | client | full `https://` URLs | | `QUERY_REWRITE_STRATEGY` | — | default `hyde` | | `OPENAI_MODEL` / `EMBEDDING_MODEL` | — | defaults `gpt-4o` / `text-embedding-3-small`. Changing embedding dim → recreate collection + full re-ingest | +| `METADATA_FILTER_DOCTYPE_KEYWORDS` | client | JSON `{"": ["kw", …]}`. Only if the client's doctype names differ. Default: `{"Contract": ["contract"], "Terms and Conditions": ["terms and conditions"]}` | +| `METADATA_FILTER_STATUS_KEYWORDS` | client | JSON `{"kw": ""}` — a *map*, so e.g. `{"expired": "Inactive"}`. Default: `{"cancelled": "Cancelled", "active": "Active", "unsigned": "Unsigned"}`. Set from Phase 5. | --- @@ -424,7 +442,8 @@ LLM judge is too noisy to gate on at any dataset size. - **Role changes lag by up to 8h.** Roles are baked into the JWT at login; a revoked role still works until the token expires. - **Metadata filters are keyword-only.** Doctype and status only — no date-range parsing, no LLM - extraction. + extraction. The keyword vocabulary is per-client config (`METADATA_FILTER_*` env vars), not the + hardcoded default, but it stays a plain substring match. - **RAGAS is not in CI and not a gate.** Quality is validated manually. - **Single box, no HA.** Host reboot recovers automatically (`restart: unless-stopped`); a host failure is a restore-from-backup event. diff --git a/docs/PIPELINE_TUNING.md b/docs/PIPELINE_TUNING.md index 4bb9e2a..fd71136 100644 --- a/docs/PIPELINE_TUNING.md +++ b/docs/PIPELINE_TUNING.md @@ -281,9 +281,11 @@ single-knob nudge only when a `case_class` slice fails. Not knobs, but genuinely per-client — and set from knowing the client's ERPNext, not from an eval sweep: -- **Metadata filter vocabulary** — `_DOCTYPE_KEYWORDS` / `_STATUS_KEYWORDS` in - `pipeline/query_pipeline.py`, if the client uses custom doctypes or status - values. Config. +- **Metadata filter vocabulary** — `METADATA_FILTER_DOCTYPE_KEYWORDS` / + `METADATA_FILTER_STATUS_KEYWORDS` env vars (`pipeline/constants.py`), if the + client uses custom doctype names or status values. Config, not a knob — set it + from knowing the client's ERPNext during the Phase 5 guided review (#135), no + source edit and no re-ingest. - **Ingestion field mapping** — whether the client's custom fields / doctypes are wired into `ingestion/`. An ingestion problem, not tuning. diff --git a/docs/templates/guided_review.md b/docs/templates/guided_review.md new file mode 100644 index 0000000..7fefbea --- /dev/null +++ b/docs/templates/guided_review.md @@ -0,0 +1,103 @@ +# Guided Review — — + +Phase 5 acceptance for a client deployment. Fill this in **live** with the client's +contract expert; it becomes the validation summary in the client's deployment +record. Copy it to `evaluation/client/_guided_review.md` (gitignored, #118) — +it names real documents and clause content. + +See `docs/CLIENT_DEPLOYMENT_RUNBOOK.md` § Phase 5 for where this sits in the sequence. + +- **Client:** +- **Corpus indexed:** Contracts · Terms and Conditions · PDF attachments +- **Build under test:** commit `` · `QUERY_REWRITE_STRATEGY=<…>` · `RETRIEVAL_TOP_K=<…>` · `RERANK_TOP_N=<…>` +- **Present:** · · + +--- + +## Rubric + +Each question gets an **answer verdict** and a **citation verdict**. A question +passes only if both are `ok`. + +| Verdict | Answer | Citations | +|---|---|---| +| `ok` | Correct and complete; nothing invented | Every claim cites a `[docname]`; the cited docs actually contain it | +| `partial` | Correct but thin, or missing a caveat that's in the source | Cited docs are right but one claim is uncited | +| `fail` | Wrong, invented a term/number, or refused when the answer *is* in the corpus | Cites the wrong document, or the cited doc doesn't support the claim | +| `n/a-refusal` | Correctly said "I could not find relevant information…" for something genuinely not in the corpus | — | + +A `fail` on retrieval grounds (right answer exists, wasn't retrieved) and a `fail` +on generation grounds (right context retrieved, answer still wrong) are different +fixes — note which in the **notes** column. + +--- + +## Questions + +10–15 real questions the client's team actually asks. Mix: single-clause lookups, +cross-document ("which of our contracts…"), and 1–2 you *expect* it to refuse +(a supplier you don't have, a clause type your contracts don't carry). + +| # | Question | Answer verdict | Citation verdict | Notes (what was wrong / which doc should have been cited / retrieval vs generation) | +|---|---|---|---|---| +| 1 | | | | | +| 2 | | | | | +| 3 | | | | | +| 4 | | | | | +| 5 | | | | | +| 6 | | | | | +| 7 | | | | | +| 8 | | | | | +| 9 | | | | | +| 10 | | | | | +| 11 | | | | | +| 12 | | | | | +| 13 | | | | | +| 14 | | | | | +| 15 | | | | | + +**Tally:** `ok` __ / `partial` __ / `fail` __ / `n/a-refusal` __ + +--- + +## Findings + +### Metadata-filter vocabulary + +Does the client's ERPNext use doctype names or status values other than the +defaults (`Contract` / `Terms and Conditions`; `Cancelled` / `Active` / `Unsigned`)? +Check against a `status` field value in Qdrant and the client's own wording in the +questions above. + +- [ ] Defaults are correct — no change +- [ ] Override needed → set in `.env` and re-test the affected questions: + - `METADATA_FILTER_DOCTYPE_KEYWORDS` = `` + - `METADATA_FILTER_STATUS_KEYWORDS` = `` + +### Knob changes + +Only when a group of questions fails the same way. Reach order (see +`docs/PIPELINE_TUNING.md` § Per-client tuning — "Reach order when a per-client slice +fails"): `RETRIEVAL_TOP_K` → system prompt vocabulary → `RERANK_TOP_N`. One knob at +a time; re-run the failing questions after each change. + +| Knob | From | To | Which questions it fixed | +|---|---|---|---| +| | | | | + +### Ingestion / data-shape issues + +(Empty chunks, unparsed PDFs, missing metadata — these are ingestion fixes, not +tuning. Cross-reference the Phase 4a smoke-ingest checklist.) + +- + +--- + +## Decision + +- [ ] **Accept** — deploy this config to the client-facing stack +- [ ] **Accept with the knob changes above** — promote the changed `.env` values +- [ ] **Blocked** — ; re-review after + +**Sign-off:** · · diff --git a/evaluation/client/README.md b/evaluation/client/README.md index 3194196..787e44e 100644 --- a/evaluation/client/README.md +++ b/evaluation/client/README.md @@ -10,10 +10,14 @@ the deployer's machine. | File | Produced by | Notes | |---|---|---| +| `_guided_review.md` | filled-in copy of `docs/templates/guided_review.md`, live with the client expert | **the default Phase 5 acceptance artifact** — names real documents, so it lives here not in the repo | | `_candidates.review.json` | `scripts/generate_eval_set.py --output evaluation/client/_candidates.review.json` | LLM draft over the client's live collection; a human-review file, not a dataset | | `_dataset.json` | hand-review of the candidates with the client expert | the client's golden set — same schema as `../test_dataset.json` | | `_results.json` | `evaluate.py --dataset evaluation/client/_dataset.json --output evaluation/client/_results.json` | the client's eval run | +The `.json` rows above are the **optional** RAGAS-benchmark add-on; the guided +review is the default. + ## The committed demo set is different `../test_dataset.json` and `../results.baseline.json` are synthetic demo diff --git a/pipeline/constants.py b/pipeline/constants.py index 798dfae..e949d37 100644 --- a/pipeline/constants.py +++ b/pipeline/constants.py @@ -12,7 +12,9 @@ from __future__ import annotations +import json import os +from typing import Any # Hybrid-search candidate pool size, then the cross-encoder rerank budget the # generator actually sees. Env-overridable so a tuning pass (#113) can sweep them @@ -20,6 +22,38 @@ RETRIEVAL_TOP_K = int(os.environ.get("RETRIEVAL_TOP_K", "20")) RERANK_TOP_N = int(os.environ.get("RERANK_TOP_N", "5")) + +def _json_env(name: str, default: str) -> Any: + """Parse a JSON-valued env var, failing fast with a pointed message. + + Used for the metadata-filter vocabulary below -- the one part of the pipeline + that is genuinely per-client (a client with a customized Contract doctype or + different status values), so it must be configurable without editing source. + """ + raw = os.environ.get(name) + if raw is None or raw == "": + return json.loads(default) + try: + return json.loads(raw) + except json.JSONDecodeError as exc: + raise ValueError(f"{name} is not valid JSON: {exc}") from exc + + +# Heuristic metadata-filter vocabulary for _extract_filters() in query_pipeline.py +# -- a keyword in the question text pins source_doctype / status before retrieval. +# Per-client config (see docs/CLIENT_DEPLOYMENT_RUNBOOK.md § Phase 5): override +# when the client uses custom doctype names or status values. The status form is +# a {keyword: status-value} map, not a list, so a client can point e.g. "expired" +# at "Inactive". Defaults reproduce the previous hardcoded behaviour exactly. +METADATA_FILTER_DOCTYPE_KEYWORDS: dict[str, list[str]] = _json_env( + "METADATA_FILTER_DOCTYPE_KEYWORDS", + '{"Contract": ["contract"], "Terms and Conditions": ["terms and conditions"]}', +) +METADATA_FILTER_STATUS_KEYWORDS: dict[str, str] = _json_env( + "METADATA_FILTER_STATUS_KEYWORDS", + '{"cancelled": "Cancelled", "active": "Active", "unsigned": "Unsigned"}', +) + # Generation call. GENERATION_MAX_TOKENS = 1024 diff --git a/pipeline/query_pipeline.py b/pipeline/query_pipeline.py index 53414d3..9cf7b11 100644 --- a/pipeline/query_pipeline.py +++ b/pipeline/query_pipeline.py @@ -28,6 +28,8 @@ ANSWER_SYSTEM_PROMPT, CONTEXT_META_FIELDS, GENERATION_MAX_TOKENS, + METADATA_FILTER_DOCTYPE_KEYWORDS, + METADATA_FILTER_STATUS_KEYWORDS, RERANK_TOP_N, RETRIEVAL_TOP_K, ) @@ -38,18 +40,6 @@ if TYPE_CHECKING: from langfuse import Langfuse -# Keywords used for heuristic doctype detection in the question text. -_DOCTYPE_KEYWORDS: dict[str, list[str]] = { - "Contract": ["contract"], - "Terms and Conditions": ["terms and conditions"], -} - -_STATUS_KEYWORDS: list[str] = [ - "cancelled", - "active", - "unsigned", -] - @dataclass class SourceDoc: @@ -191,20 +181,21 @@ def _span(trace: Any, name: str, fn, summarize=None): def _extract_filters(question: str) -> dict[str, Any]: """Heuristic metadata filter extraction from the question text. - Detects doctype and status keywords. The pipeline merges this dict with + Detects doctype and status keywords from the (per-client configurable) + vocabulary in ``pipeline.constants``. The pipeline merges this dict with any explicit sidebar filters from the frontend (sidebar wins on conflicts). """ lower = question.lower() filters: dict[str, Any] = {} - for doctype, keywords in _DOCTYPE_KEYWORDS.items(): + for doctype, keywords in METADATA_FILTER_DOCTYPE_KEYWORDS.items(): if any(kw in lower for kw in keywords): filters["source_doctype"] = doctype break - for status in _STATUS_KEYWORDS: - if status in lower: - filters["status"] = status.capitalize() + for keyword, status_value in METADATA_FILTER_STATUS_KEYWORDS.items(): + if keyword in lower: + filters["status"] = status_value break return filters diff --git a/tests/test_pipeline_constants.py b/tests/test_pipeline_constants.py new file mode 100644 index 0000000..60bd049 --- /dev/null +++ b/tests/test_pipeline_constants.py @@ -0,0 +1,49 @@ +"""Tests for pipeline.constants -- the env-overridable knob parsing (#135).""" + +from __future__ import annotations + +import pytest + +from pipeline.constants import _json_env + + +def test_json_env_returns_parsed_default_when_unset(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("METADATA_FILTER_STATUS_KEYWORDS", raising=False) + assert _json_env("METADATA_FILTER_STATUS_KEYWORDS", '{"cancelled": "Cancelled"}') == { + "cancelled": "Cancelled" + } + + +def test_json_env_returns_parsed_default_when_blank(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("METADATA_FILTER_STATUS_KEYWORDS", "") + assert _json_env("METADATA_FILTER_STATUS_KEYWORDS", "{}") == {} + + +def test_json_env_parses_the_override(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("METADATA_FILTER_DOCTYPE_KEYWORDS", '{"Agreement": ["agreement", "mou"]}') + parsed = _json_env("METADATA_FILTER_DOCTYPE_KEYWORDS", "{}") + assert parsed == {"Agreement": ["agreement", "mou"]} + + +def test_json_env_fails_fast_on_bad_json(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("METADATA_FILTER_STATUS_KEYWORDS", "{not valid") + with pytest.raises(ValueError, match="METADATA_FILTER_STATUS_KEYWORDS is not valid JSON"): + _json_env("METADATA_FILTER_STATUS_KEYWORDS", "{}") + + +def test_shipped_defaults_are_behaviour_preserving() -> None: + """The move from hardcoded constants to config must not change the shipped vocab.""" + from pipeline.constants import ( + METADATA_FILTER_DOCTYPE_KEYWORDS, + METADATA_FILTER_STATUS_KEYWORDS, + ) + + assert METADATA_FILTER_DOCTYPE_KEYWORDS == { + "Contract": ["contract"], + "Terms and Conditions": ["terms and conditions"], + } + assert METADATA_FILTER_STATUS_KEYWORDS == { + "cancelled": "Cancelled", + "active": "Active", + "unsigned": "Unsigned", + } diff --git a/tests/test_query_pipeline.py b/tests/test_query_pipeline.py index 0ff1a78..e1f4cb1 100644 --- a/tests/test_query_pipeline.py +++ b/tests/test_query_pipeline.py @@ -127,6 +127,30 @@ def test_extract_filters_is_case_insensitive() -> None: assert result.get("source_doctype") == "Contract" +def test_extract_filters_honours_per_client_status_vocabulary( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """#135: a client whose ERPNext uses different status wording overrides the + keyword -> status-value map via METADATA_FILTER_STATUS_KEYWORDS, no source edit.""" + import pipeline.query_pipeline as qp + + monkeypatch.setattr(qp, "METADATA_FILTER_STATUS_KEYWORDS", {"expired": "Inactive"}) + assert _extract_filters("which agreements are expired").get("status") == "Inactive" + # the shipped keywords are gone once overridden + assert "status" not in _extract_filters("list cancelled contracts") + + +def test_extract_filters_honours_per_client_doctype_vocabulary( + monkeypatch: pytest.MonkeyPatch, +) -> None: + import pipeline.query_pipeline as qp + + monkeypatch.setattr( + qp, "METADATA_FILTER_DOCTYPE_KEYWORDS", {"Contract": ["contract", "agreement"]} + ) + assert _extract_filters("what does the agreement say").get("source_doctype") == "Contract" + + # --------------------------------------------------------------------------- # _build_context # ---------------------------------------------------------------------------