Store the document title and authorising role at ingest
ProcedureIdentity requires a title and an authorising role, and neither was stored anywhere. The answer writer was asked for both, read them off whatever chunk retrieval happened to return, and returned "" whenever the header chunk was not among them. They 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, not something to re-derive per question from whatever text was retrieved. Denormalised onto every chunk exactly as the existing header fields are - ingest replaces every chunk of a source_file in one transaction, so they cannot drift within a document. complete() deliberately still requires only doc_number, revision and effective_date. 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. controlled_copy_location is NOT in the schema. It is a site fact, identical on every row, and the one field where an invented value sends a person to a place that does not exist. It is CONTROLLED_COPY_LOCATION in api.env, defaulting to a string that names who to ask. The authorising-role pattern requires the colon: without it the lazy gap swallowed the field name and captured "role: Station Maintenance Supervisor" as the value, which the first run caught. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
156d2a6d43
commit
dcfb411c3b
4 changed files with 96 additions and 2 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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")),
|
||||
|
|
|
|||
45
db/007_doc_identity.sql
Normal file
45
db/007_doc_identity.sql
Normal file
|
|
@ -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.
|
||||
|
|
@ -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,)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue