yau-plant-assistant/api/tests/test_documents.py
Claude 8d09c84fd0 Fix three defects the first real document exposed
None of these were reachable by the tests as they stood, and all three were
silent - the screen looked correct in every case. An 8-page control philosophy
found all of them in one upload.

1. THE WHOLE DOCUMENT BECAME ONE CHUNK. pypdf emits one line per line of the
   PDF and no blank lines at all: 416 lines, none blank. Section splitting looks
   for Markdown headings and paragraph splitting looks for blank lines, so the
   chunker was a no-op on PDF text - one 18,307-character chunk, a single
   embedding vector for eight pages, and every citation reading "(untitled),
   page 1". A longer document would have exceeded the embedding model's input
   limit and failed to publish at all.

   convert.py now recovers structure: headings from numbered and capitalised
   lines, paragraphs by reflowing on line width. Heading detection is
   deliberately narrow, because the dangerous direction is promoting a numbered
   STEP to a heading and splitting a step sequence - so a heading must be short,
   a few words, and without terminal punctuation. "1. Purpose" qualifies;
   "1. Open the isolation valve and confirm zero pressure." does not.

   chunking.py gains a ceiling no chunk may exceed whatever the input looks
   like, falling back to line and then word boundaries. The step-sequence
   refusal still holds below it and is unchanged for any realistic procedure;
   past it, splitting is the lesser harm, because an embeddings call that fails
   protects nobody. Two heuristics found only by running the real file:
   "SCADA" and "WRPS-PRO-001" were being promoted to headings, which cut real
   sections in half and re-titled the remainder with something meaningless, and
   "11 August 2026" was parsing as section 11.

   19 chunks now, largest 574 tokens, sections matching the document.

2. EVERY CHUNK CARRIED doc_title = "Revision". TITLE_RE used [\s:]+ for the gap
   after the label, and \s includes the newline. A cover page flattens to a
   label column then a value column - Title / Revision / Date - so it matched a
   bare "Title" line, consumed the line break and captured the next line. Now
   [ \t:]+, the same trap AUTHORISING_ROLE_RE was fixed for once already. The
   document's title is now null, which is the honest answer: a citation falls
   back to the section title, and a confidently wrong title falls back to
   nothing. Inherited, so fixed in ingest.py too.

3. RE-PUBLISHING A DOCUMENT DUPLICATED IT. approve deleted prior chunks by
   source_file, which carries the upload_id and is new on every upload -
   so approving the same revision twice left 38 live chunks and the same
   passage citable twice. Invisible on screen, because live_documents groups by
   (doc_number, revision) and only the count moved. Now deletes by document and
   revision as well, and logs how many chunks it replaced.

The two chunkers are now provably in step rather than asked to be. The header
of chunking.py claimed drift in ingest.py could not be detected from the test
suite; that was wrong, both files are on disk. The new test compares the source
of chunk_section, _split_on_lines, _split_on_words, extract_header and
approx_tokens character for character. Writing it found that one earlier edit to
ingest.py had silently not applied, leaving the two genuinely divergent, and
then that extract_header's docstring had drifted. Both fixed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-28 15:03:31 +10:00

330 lines
14 KiB
Python

"""The document-library rules that must hold without a database or a network.
Same principle as the contract tests: the rules that matter are in Python, so
they can be exercised without an API key, a database or a running host. What is
covered here is what would be expensive to discover on the host and impossible
to discover from `docker ps`.
"""
from __future__ import annotations
import pytest
from fastapi import HTTPException
import chunking
import convert
import identity
from config import Settings
class _Req:
"""The two attributes identity.actor() actually reads."""
def __init__(self, headers: dict[str, str] | None = None):
self.headers = headers or {}
def _use(monkeypatch, **kwargs):
"""Give identity.py a Settings built for this one test.
The real settings() is lru_cached off the process environment, which is no
use here - each test needs a different identity mode and publisher list.
monkeypatch restores the original at teardown, so nothing leaks between
tests and the cache is never touched.
"""
cfg = Settings(**kwargs)
monkeypatch.setattr(identity, "settings", lambda: cfg)
return cfg
# --- chunking ---------------------------------------------------------------
# Mirrored from ingest/ingest.py. If this drifts, the same document ingested by
# the two paths produces different chunks - see the header of api/chunking.py.
def test_a_numbered_step_sequence_in_a_procedure_is_never_split():
steps = "\n\n".join(f"{n}. Do the {n}th thing. " + "x" * 400
for n in range(1, 30))
assert chunking.approx_tokens(steps) > chunking.CHUNK_TOKEN_TARGET
assert len(chunking.chunk_section(steps, "procedure")) == 1
def test_the_same_oversized_text_IS_split_when_it_is_not_a_procedure():
# The refusal is scoped to procedures on purpose. A manual has no steps to
# break, and keeping it whole would just cost tokens.
prose = "\n\n".join("Some prose about the station. " + "x" * 400
for _ in range(30))
assert len(chunking.chunk_section(prose, "manual")) > 1
def test_sections_split_on_headings_and_keep_their_titles():
sections = chunking.split_sections(
"# Purpose\nWhy this exists.\n\n## Scope\nWhat it covers.\n")
assert [t for _, t, _ in sections] == ["Purpose", "Scope"]
def test_equipment_is_linked_only_when_it_is_unambiguous():
ids = ["PU-301", "PU-302"]
assert chunking.link_equipment("Isolate PU-301 before work.", ids) == "PU-301"
# Two units mentioned is not a tie to break - a chunk linked to the wrong
# pump is hidden from the pump it actually describes.
assert chunking.link_equipment("PU-301 and PU-302 share a header.", ids) is None
def test_header_extraction_is_a_proposal_and_may_find_nothing():
found = chunking.extract_header(
"Title: Wet Well Interlock Bypass\n"
"WRPS-OPS-014 Revision 3\n"
"Effective: 01/03/2026\n"
"Authorising role: Station Maintenance Supervisor\n")
assert found.doc_number == "WRPS-OPS-014"
assert found.revision == "3"
assert found.authorising_role == "Station Maintenance Supervisor"
empty = chunking.extract_header("A document with no header at all.")
assert empty.doc_number is None and empty.revision is None
# --- conversion -------------------------------------------------------------
def test_a_document_with_almost_no_extractable_text_is_refused():
"""A scan converts to nothing. Refusing is the point.
Storing it would put a blank document in front of a reviewer who might
approve it without noticing there is nothing in it.
"""
with pytest.raises(convert.ConversionError) as exc:
convert.convert("scan.md", b"# Title\n")
assert "OCR" in str(exc.value)
def test_an_unsupported_extension_is_refused_by_name():
with pytest.raises(convert.ConversionError) as exc:
convert.convert("drawing.dwg", b"x" * 5000)
assert ".dwg" in str(exc.value)
def test_plain_text_passes_through_and_keeps_its_headings():
body = "# Purpose\n\n" + "This station has three pumps. " * 40
result = convert.convert("notes.md", body.encode())
assert result.converter == "passthrough"
assert result.markdown.startswith("# Purpose")
# --- identity ---------------------------------------------------------------
def test_authelia_mode_refuses_a_request_with_no_remote_user(monkeypatch):
"""Reaching ai-api without passing Authelia is not a state in which to
accept a change to the document set."""
_use(monkeypatch, doc_identity_mode="authelia")
with pytest.raises(HTTPException) as exc:
identity.actor(_Req())
assert exc.value.status_code == 401
def test_authelia_mode_ignores_a_name_supplied_in_the_body(monkeypatch):
"""The form field must not be able to change the recorded actor. That is
the entire reason identity comes from the headers."""
_use(monkeypatch, doc_identity_mode="authelia")
who = identity.actor(_Req({"Remote-User": "cliu"}), declared_name="someone-else")
assert who.stored == "cliu"
assert who.verified is True
def test_demo_mode_marks_every_row_so_it_can_never_pass_as_authenticated(monkeypatch):
_use(monkeypatch, doc_identity_mode="demo")
who = identity.actor(_Req(), declared_name="admin")
assert who.stored == "demo:admin"
assert who.display == "admin"
assert who.groups == identity.DEMO_GROUPS
assert who.verified is False
def test_demo_mode_records_the_authelia_user_alongside_the_typed_name(monkeypatch):
"""A demo row whose header disagrees with the typed name is worth a look."""
_use(monkeypatch, doc_identity_mode="demo")
who = identity.actor(_Req({"Remote-User": "cliu"}), declared_name="admin")
assert "authelia=cliu" in who.groups
def test_demo_mode_still_requires_a_name(monkeypatch):
_use(monkeypatch, doc_identity_mode="demo")
with pytest.raises(HTTPException) as exc:
identity.actor(_Req(), declared_name=" ")
assert exc.value.status_code == 400
# --- authorisation ----------------------------------------------------------
def test_with_no_publishers_configured_nobody_may_change_anything(monkeypatch):
"""Fail closed. An empty list must not mean 'everyone'."""
_use(monkeypatch, doc_identity_mode="demo", doc_publishers=())
who = identity.actor(_Req(), declared_name="admin")
with pytest.raises(HTTPException) as exc:
identity.require_publisher(who)
assert exc.value.status_code == 403
def test_a_non_publisher_is_refused_even_though_authelia_let_them_in(monkeypatch):
_use(monkeypatch, doc_identity_mode="demo", doc_publishers=("admin",))
with pytest.raises(HTTPException) as exc:
identity.require_publisher(identity.actor(_Req(), declared_name="dan"))
assert exc.value.status_code == 403
def test_the_publisher_check_ignores_capitalisation(monkeypatch):
_use(monkeypatch, doc_identity_mode="demo", doc_publishers=("admin",))
assert identity.is_publisher(identity.actor(_Req(), declared_name="Admin"))
def test_the_demo_prefix_is_provenance_and_not_part_of_the_identity(monkeypatch):
"""`demo:admin` is admin. The prefix marks how we know, not who they are -
if it were matched, demo mode would authorise nobody."""
_use(monkeypatch, doc_identity_mode="demo", doc_publishers=("admin",))
who = identity.actor(_Req(), declared_name="admin")
assert who.stored == "demo:admin"
assert identity.is_publisher(who)
# --- the router itself ------------------------------------------------------
def test_the_router_can_be_mounted():
"""Import documents.py and mount it on a real FastAPI app.
This exists because it did not, and the first deployment crash-looped on
import: `withdraw` and `restore` return a UNION of two Response subclasses,
and FastAPI tried to build a Pydantic response model from the annotation.
Nothing in the rest of this file imports documents.py, so every unit test
passed against code that could not start.
A route-table assertion is not the point - reaching the assertion is. The
failure mode this catches is at import time.
"""
from fastapi import FastAPI
import documents
app = FastAPI()
app.include_router(documents.router)
paths = {r.path for r in app.routes}
for expected in ("/documents", "/documents/upload", "/documents/withdraw",
"/documents/restore", "/documents/review/{upload_id}"):
assert expected in paths, f"{expected} is not mounted"
# --- flat PDF text: the defect found on the first real document -------------
#
# WRPS-CTL-001, an 8-page control philosophy, published as ONE 18,307-character
# chunk titled "(untitled)". pypdf emits one line per PDF line and no blank
# lines at all - 416 lines, 0 blank - so section splitting found no headings
# and paragraph splitting found no paragraphs. Both halves of the fix are
# locked below.
FLAT_PDF_TEXT = "\n".join(
["1. Purpose"]
+ ["This document states how the station is to be controlled and why it"] * 40
+ ["3.2 Pump control"]
+ ["The duty pump starts on rising level and the assist pumps follow it"] * 40
)
def test_flat_pdf_text_gains_headings_and_paragraphs():
"""The conversion must produce structure, not one undifferentiated wall."""
result = convert._structure(FLAT_PDF_TEXT.splitlines())
assert "## 1. Purpose" in result
assert "## 3.2 Pump control" in result
assert "\n\n" in result, "no paragraph breaks were produced"
def test_a_numbered_step_is_not_mistaken_for_a_heading():
"""The dangerous direction. A step promoted to a heading splits a step
sequence, which is the one thing chunking must never do."""
assert convert._looks_like_heading("1. Purpose") == "1. Purpose"
assert convert._looks_like_heading(
"1. Open the isolation valve on PU-301 and confirm zero pressure.") is None
assert convert._looks_like_heading(
"2. Close the discharge valve, then wait sixty seconds before starting.") is None
def test_no_chunk_may_exceed_the_ceiling_even_with_no_paragraph_breaks():
"""The backstop. This does not depend on the heading heuristic working."""
wall = "x" * (chunking.MAX_CHUNK_TOKENS * 4 * 3) # 3x the ceiling, one line
for doc_type in ("design", "procedure", "manual", "rationalisation"):
for chunk in chunking.chunk_section(wall, doc_type):
assert chunking.approx_tokens(chunk) <= chunking.MAX_CHUNK_TOKENS, doc_type
def test_a_step_sequence_under_the_ceiling_is_still_never_split():
"""The original rule, unchanged. The ceiling must not weaken it."""
steps = "\n\n".join(f"{n}. Do the {n}th thing. " + "x" * 400
for n in range(1, 30))
assert chunking.approx_tokens(steps) > chunking.CHUNK_TOKEN_TARGET
assert chunking.approx_tokens(steps) < chunking.MAX_CHUNK_TOKENS
assert len(chunking.chunk_section(steps, "procedure")) == 1
def test_the_whole_document_no_longer_becomes_one_chunk():
"""End to end over the shape that actually failed."""
structured = convert._structure(FLAT_PDF_TEXT.splitlines())
sections = chunking.split_sections(structured)
assert len(sections) >= 2, "headings did not create sections"
assert [t for _, t, _ in sections][:1] != ["(untitled)"]
def test_the_two_chunkers_have_not_drifted():
"""api/chunking.py and ingest/ingest.py must chunk identically.
The header of api/chunking.py used to say drift in ingest.py could not be
detected from here. That was wrong - the source of both functions is right
there on disk. If they differ, the same document chunks differently
depending on whether it arrived through the UI or the CLI, and the
assistant answers or fails to answer depending on that.
Compares source text, not behaviour: behaviour can agree on the cases
somebody thought to write down and differ on the one that matters.
"""
import pathlib
here = pathlib.Path(__file__).resolve().parent.parent
def body(path: pathlib.Path, name: str) -> str:
src = path.read_text(encoding="utf-8")
start = src.index(f"def {name}(")
return src[start:src.index("\ndef ", start + 1)].strip()
api = here / "chunking.py"
cli = here.parent / "ingest" / "ingest.py"
if not cli.exists(): # api/ checked out on its own
pytest.skip("ingest/ingest.py not present")
for fn in ("chunk_section", "_split_on_lines", "_split_on_words",
"extract_header", "approx_tokens"):
assert body(api, fn) == body(cli, fn), (
f"{fn} has drifted between api/chunking.py and ingest/ingest.py")
def test_a_bare_Title_label_does_not_capture_the_next_line():
"""The cover-page table trap, found on WRPS-CTL-001.
A PDF table flattens to a label column then a value column. With `\s` in
the gap - which includes the newline - "Title" swallowed the line break and
captured "Revision" from the line below, and every chunk of an eight-page
document was stored with doc_title = "Revision".
NULL is the right answer here. A citation falls back to the section title;
a confidently wrong document title does not fall back to anything.
"""
flattened = "Document number\nTitle\nRevision\nDate\nStatus\n" \
"WRPS-CTL-001\nControl Philosophy\nA\n"
assert chunking.extract_header(flattened).title is None
def test_a_real_title_line_is_still_read():
assert chunking.extract_header(
"Title: Wet Well Interlock Bypass\n").title == "Wet Well Interlock Bypass"
assert chunking.extract_header(
"Title Wet Well Interlock Bypass\n").title == "Wet Well Interlock Bypass"