diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1920d25..07c38db 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,11 +32,49 @@ jobs: - uses: actions/setup-python@v5 with: python-version: "3.11" - - name: Install maturin + pytest - run: pip install maturin pytest - - name: Build extension - working-directory: bindings - run: maturin develop --release + # `pip install ./bindings` builds the extension through maturin's PEP 517 + # backend. Do NOT use `maturin develop` here: it requires an active + # virtualenv, and it installs editable — which would also hide whether the + # built wheel actually ships the pure-Python `hnsw_rag` package. + - name: Build and install the extension + run: pip install ./bindings pytest - name: Pytest working-directory: bindings run: python -m pytest tests/ -q + + service: + name: RAG service + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + - uses: Swatinem/rust-cache@v2 + - uses: actions/setup-python@v5 + with: + python-version: "3.11" + - name: Install engine bindings + service deps + run: | + pip install ./bindings + pip install -r service/requirements-dev.txt + # No ANTHROPIC_API_KEY: the service runs in keyless mock mode, so the + # retrieval path is exercised end to end without a key or network. + - name: Pytest + working-directory: service + run: PYTHONPATH=. python -m pytest tests/ -q + + app: + name: Next.js app + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: "22" + cache: npm + cache-dependency-path: app/package-lock.json + - name: Install + working-directory: app + run: npm ci + - name: Build (type-checks the app) + working-directory: app + run: npm run build diff --git a/README.md b/README.md index ab49afc..7d1c436 100644 --- a/README.md +++ b/README.md @@ -118,12 +118,14 @@ This section grows as the project does; each phase documents the trade-offs it m | Layer | Command | Count | |-------|---------|-------| -| Rust engine | `cargo test` | 22 (unit + seeded recall vs. brute force + doctest) | +| Rust engine | `cargo test` | 23 (unit + seeded recall vs. brute force + doctest) | | Python bindings + helpers | `pytest` in `bindings/` | 16 (FFI surface, recall vs. brute force, chunking, embeddings, E2E retrieval) | -| RAG service | `PYTHONPATH=. pytest` in `service/` | 7 (keyless E2E via FastAPI TestClient) | +| RAG service | `PYTHONPATH=. pytest` in `service/` | 12 (keyless E2E via FastAPI TestClient + citation parsing) | | Next.js app | `npm run build` | type-checked production build | -CI (`.github/workflows/ci.yml`) runs `cargo fmt --check`, `cargo clippy -- -D warnings`, `cargo test`, and the Python bindings suite on every push. +CI (`.github/workflows/ci.yml`) runs all four on every push, in parallel jobs: `cargo fmt --check` + `cargo clippy -- -D warnings` + `cargo test`, the bindings suite, the service suite, and the app build. + +The bindings job installs with `pip install ./bindings` rather than `maturin develop`. That is deliberate: `maturin develop` installs *editable*, which would mask whether the built wheel actually ships the pure-Python `hnsw_rag` package alongside the compiled module. Installing the real wheel is what catches that. ## License diff --git a/bindings/Cargo.toml b/bindings/Cargo.toml index 842dc5e..f1babf7 100644 --- a/bindings/Cargo.toml +++ b/bindings/Cargo.toml @@ -6,7 +6,11 @@ edition.workspace = true license.workspace = true [lib] -name = "hnsw_engine" +# Must NOT be `hnsw_engine`: that is the engine crate's lib name, and two libs +# with the same name in one workspace make rustdoc fail with E0464 ("multiple +# candidates for rlib dependency"). maturin places the built artifact according +# to `module-name` in pyproject.toml, so this name is internal only. +name = "hnsw_engine_native" crate-type = ["cdylib", "rlib"] [dependencies] diff --git a/bindings/pyproject.toml b/bindings/pyproject.toml index 575a4f6..53611e1 100644 --- a/bindings/pyproject.toml +++ b/bindings/pyproject.toml @@ -22,3 +22,9 @@ features = ["pyo3/extension-module"] module-name = "hnsw_engine._native" manifest-path = "Cargo.toml" python-source = "python" +# maturin's mixed layout only packages the module named by `module-name` — the +# sibling `hnsw_rag` package would be missing from the built wheel (and only +# importable under `maturin develop`, which installs editable). Ship it +# explicitly; maturin strips the `python-source` prefix, so it lands at the +# wheel root and imports as `hnsw_rag`. +include = [{ path = "python/hnsw_rag/**/*.py", format = ["sdist", "wheel"] }] diff --git a/bindings/src/lib.rs b/bindings/src/lib.rs index ea35b32..5b76281 100644 --- a/bindings/src/lib.rs +++ b/bindings/src/lib.rs @@ -7,7 +7,7 @@ use pyo3::exceptions::{PyIndexError, PyValueError}; use pyo3::prelude::*; -use ::hnsw_engine as engine; +use hnsw_engine as engine; /// When `ef_search` is not given, use `max(4 * k, 50)` — comfortably above /// the knee of the recall curve for typical corpus sizes (see the README diff --git a/service/rag_service/generation.py b/service/rag_service/generation.py index c820925..9da5d48 100644 --- a/service/rag_service/generation.py +++ b/service/rag_service/generation.py @@ -9,6 +9,7 @@ from __future__ import annotations import os +import re from dataclasses import dataclass from typing import List, Optional @@ -42,6 +43,25 @@ def _format_sources(chunks: List[RetrievedChunk]) -> str: return "\n\n".join(blocks) +def parse_citations(text: str, chunks: List[RetrievedChunk]) -> List[int]: + """Map the `[n]` markers in an answer back to the chunk ids they refer to. + + The sources are numbered 1..len(chunks) in the prompt, so `[2]` means + `chunks[1]`. Out-of-range markers (the model inventing `[9]` for three + sources) are ignored rather than trusted. Returns ids in first-mention + order, deduplicated — so a "cited" flag actually means the answer used + that chunk, instead of just "we retrieved it". + """ + cited: List[int] = [] + for marker in re.findall(r"\[(\d+)\]", text): + idx = int(marker) - 1 + if 0 <= idx < len(chunks): + cid = chunks[idx].id + if cid not in cited: + cited.append(cid) + return cited + + def _mock_answer(question: str, chunks: List[RetrievedChunk]) -> Answer: """Deterministic, keyless fallback: return the single most relevant chunk as the answer, cited. Good enough to prove the pipeline end to end.""" @@ -109,10 +129,8 @@ def generate_answer( ) text = "".join(block.text for block in response.content if block.type == "text") - # We surface every retrieved chunk as a candidate source; the [n] markers in - # the text tell the reader which were actually used. return Answer( text=text, - cited_chunk_ids=[c.id for c in chunks], + cited_chunk_ids=parse_citations(text, chunks), model=model, ) diff --git a/service/requirements-dev.txt b/service/requirements-dev.txt new file mode 100644 index 0000000..12b8ef1 --- /dev/null +++ b/service/requirements-dev.txt @@ -0,0 +1,3 @@ +-r requirements.txt +pytest>=7 +httpx>=0.27 # required by fastapi's TestClient diff --git a/service/tests/test_citations.py b/service/tests/test_citations.py new file mode 100644 index 0000000..60ee98d --- /dev/null +++ b/service/tests/test_citations.py @@ -0,0 +1,41 @@ +"""Tests for citation-marker parsing. + +The `cited` flag in the UI is only meaningful if it reflects what the answer +actually referenced, so the mapping from `[n]` markers to chunk ids is worth +testing directly. +""" + +from rag_service.generation import parse_citations +from rag_service.store import RetrievedChunk + + +def chunk(cid: int) -> RetrievedChunk: + return RetrievedChunk( + id=cid, text="…", doc_id=0, doc_title="doc", ordinal=0, score=0.5 + ) + + +CHUNKS = [chunk(10), chunk(11), chunk(12)] + + +def test_maps_markers_to_chunk_ids(): + # [1] and [3] are 1-indexed into the sources list. + assert parse_citations("Yes [1], and also [3].", CHUNKS) == [10, 12] + + +def test_no_markers_means_nothing_cited(): + assert parse_citations("I could not find an answer.", CHUNKS) == [] + + +def test_out_of_range_markers_ignored(): + # The model inventing [9] for three sources must not crash or be trusted. + assert parse_citations("See [9] and [2].", CHUNKS) == [11] + assert parse_citations("See [0].", CHUNKS) == [] + + +def test_deduplicated_in_first_mention_order(): + assert parse_citations("[2] then [1] then [2] again.", CHUNKS) == [11, 10] + + +def test_empty_chunk_list(): + assert parse_citations("Nothing indexed [1].", []) == []