diff --git a/.env.example b/.env.example index ca68d5e..f35d586 100644 --- a/.env.example +++ b/.env.example @@ -44,6 +44,11 @@ NO_LLM_STUB=false # --- behaviour --------------------------------------------------------------- CLASSIFIER_CONFIDENCE_THRESHOLD=0.7 +# Where the CONTROLLED copy of a procedure actually lives - a site fact, the +# same for every document, so it is not in doc_chunks. Set this to the real +# DMS location. The default names who to ask, which is always true and never +# sends anybody to a place that does not exist. +CONTROLLED_COPY_LOCATION= SITE_TIMEZONE=Australia/Sydney # storage UTC; convert once, in Cube MAX_ROWS_RETURNED=5000 QUERY_TIMEOUT_SECONDS=30 diff --git a/api/config.py b/api/config.py index 5b6d777..498a807 100644 --- a/api/config.py +++ b/api/config.py @@ -40,6 +40,15 @@ class Settings(BaseModel): # --- behaviour --------------------------------------------------------- classifier_confidence_threshold: float = 0.7 site_timezone: str = "Australia/Sydney" + # Where the CONTROLLED copy of a procedure actually lives. A site fact, not + # a document fact - it is the same for every document, so it is not in + # doc_chunks. It is here rather than left to the model because sending an + # operator to a controlled copy that does not exist is worse than telling + # them nothing. The default says who to ask, which is always true. + controlled_copy_location: str = ( + "Ask the WRPS document controller - this assistant does not hold " + "controlled copies." + ) max_rows_returned: int = 5000 query_timeout_seconds: int = 30 max_output_tokens: int = 1200 @@ -91,6 +100,11 @@ def settings() -> Settings: env.get("CLASSIFIER_CONFIDENCE_THRESHOLD", "0.7") ), site_timezone=env.get("SITE_TIMEZONE", "Australia/Sydney"), + controlled_copy_location=env.get( + "CONTROLLED_COPY_LOCATION", + "Ask the WRPS document controller - this assistant does not hold " + "controlled copies.", + ), max_rows_returned=int(env.get("MAX_ROWS_RETURNED", "5000")), query_timeout_seconds=int(env.get("QUERY_TIMEOUT_SECONDS", "30")), max_output_tokens=int(env.get("MAX_OUTPUT_TOKENS", "1200")), diff --git a/db/007_doc_identity.sql b/db/007_doc_identity.sql new file mode 100644 index 0000000..6363f03 --- /dev/null +++ b/db/007_doc_identity.sql @@ -0,0 +1,45 @@ +-- ============================================================================= +-- 007 - Document title and authorising role on doc_chunks. +-- +-- WHY: ProcedureIdentity requires a title and an authorising role, and until +-- now neither was stored anywhere. The model was asked to supply them, read +-- them off whatever chunk retrieval happened to return, and returned "" when +-- the header chunk was not among them - which was most of the time, because +-- find_procedure ranked chunks by similarity to the question and a title block +-- does not resemble "how do I lift the interlock". +-- +-- These belong in the row for the same reason doc_number and revision do: they +-- are facts about the controlled document, established once when a human +-- confirms the header at ingest, not something to re-derive per question from +-- whatever text happened to be retrieved. +-- +-- Denormalised onto every chunk, exactly like doc_number/revision/ +-- effective_date already are. The alternative is a documents table and a join +-- on the retrieval hot path; the corpus is small, ingestion replaces every +-- chunk of a source_file in one transaction (ingest.py rule 4), so the columns +-- cannot drift within a document. +-- +-- NOT INCLUDED: controlled_copy_location. That is a fact about the site's +-- document management system, not about any one document - it would be the +-- same string on every row. It lives in CONTROLLED_COPY_LOCATION in +-- ~/ai/api.env instead. Letting the model invent it was how an operator could +-- be told to fetch a controlled copy from a place that does not exist. +-- +-- Existing rows get NULL and keep working: citation() falls back to +-- section_title and then to source_file, which is what it did before. Re-run +-- `ai-ingest --all` to populate them. +-- ============================================================================= + +ALTER TABLE doc_chunks ADD COLUMN IF NOT EXISTS doc_title text; +ALTER TABLE doc_chunks ADD COLUMN IF NOT EXISTS authorising_role text; + +COMMENT ON COLUMN doc_chunks.doc_title IS + 'Document title from the confirmed header. NULL for rows ingested before ' + 'migration 007; re-ingest to populate.'; +COMMENT ON COLUMN doc_chunks.authorising_role IS + 'Role that authorises work under this document, from the confirmed header. ' + 'Advisory only - it does not grant anything and is not an authorisation ' + 'check.'; + +-- ingest_rw already holds INSERT/DELETE on doc_chunks, so no grant changes. +-- agent_ro already holds SELECT on the table, which covers new columns. diff --git a/ingest/ingest.py b/ingest/ingest.py index 5851d95..67bbdf4 100644 --- a/ingest/ingest.py +++ b/ingest/ingest.py @@ -72,6 +72,18 @@ DATE_RE = re.compile( r"\d{1,2}\s+\w+\s+\d{4})\b", re.IGNORECASE, ) +# Header fields that are not safety-critical but ARE facts about the document: +# storing them stops the model being asked to read them off whatever chunk +# retrieval happened to return, which is how it ended up returning "". +TITLE_RE = re.compile(r"^\s*title[\s:]+(.+\S)\s*$", re.IGNORECASE | re.MULTILINE) +# "Authorising role: X", "Authorised by: X", "Authorising: X". The colon is +# required: without it the lazy gap swallowed the field NAME and captured +# "role: Station Maintenance Supervisor" as the value. +AUTHORISING_ROLE_RE = re.compile( + r"^[ ]*authoris(?:ing|ed)[ ]*(?:role|by)?[ ]*:[ ]*(.+\S)[ ]*$", + re.IGNORECASE | re.MULTILINE, +) + # A numbered step. Used to refuse to split, not to parse the procedure. STEP_RE = re.compile(r"^\s*(?:\d+\.|\(\d+\)|step\s+\d+)", re.IGNORECASE | re.MULTILINE) @@ -81,8 +93,17 @@ class Header: doc_number: str | None revision: str | None effective_date: date | None + title: str | None = None + authorising_role: str | None = None def complete(self) -> bool: + """The three fields a wrong value in is a SAFETY issue. + + Deliberately not title or authorising_role: a missing title makes an + answer less useful, a wrong revision sends somebody to the wrong + document. --assume-yes must keep refusing on the second and tolerate + the first. + """ return all((self.doc_number, self.revision, self.effective_date)) @@ -114,10 +135,14 @@ def extract_header(text: str) -> Header: number = DOC_NUMBER_RE.search(head) revision = REVISION_RE.search(head) effective = DATE_RE.search(head) + title = TITLE_RE.search(head) + role = AUTHORISING_ROLE_RE.search(head) return Header( doc_number=number.group(1) if number else None, revision=revision.group(1) if revision else None, effective_date=parse_date(effective.group(1)) if effective else None, + title=title.group(1).strip() if title else None, + authorising_role=role.group(1).strip() if role else None, ) @@ -127,6 +152,8 @@ def confirm_header(path: Path, header: Header, assume_yes: bool) -> Header: print(f" doc_number : {header.doc_number or '(not found)'}") print(f" revision : {header.revision or '(not found)'}") print(f" effective_date : {header.effective_date or '(not found)'}") + print(f" title : {header.title or '(not found)'}") + print(f" authorising : {header.authorising_role or '(not found)'}") if assume_yes: if not header.complete(): @@ -143,6 +170,8 @@ def confirm_header(path: Path, header: Header, assume_yes: bool) -> Header: revision=input(" revision : ").strip() or header.revision, effective_date=parse_date(input(" effective_date (YYYY-MM-DD): ").strip()) or header.effective_date, + title=input(" title : ").strip() or header.title, + authorising_role=input(" authorising : ").strip() or header.authorising_role, ) @@ -348,6 +377,7 @@ def ingest_file( source_file, doc_type, header.doc_number, header.revision, header.effective_date, superseded, link_equipment(chunk, equipment_ids), page, title, chunk, + header.title, header.authorising_role, ) ) @@ -370,8 +400,8 @@ def ingest_file( INSERT INTO doc_chunks (source_file, doc_type, doc_number, revision, effective_date, superseded, equipment_id, page, section_title, chunk_text, - embedding) - VALUES (%s,%s,%s,%s,%s,%s,%s,%s,%s,%s,%s) + doc_title, authorising_role, embedding) + VALUES (%s,%s,%s,%s,%s,%s,%s,%s,%s,%s,%s,%s,%s) """, [ record + (str(vector) if vector is not None else None,)