Stop a bulk re-ingest resurrecting withdrawn documents
ingest_file() inserted every chunk with superseded = FALSE, so replacing a
document's chunks reset its withdrawal. One `ai-ingest --all` made every
superseded revision citable again - including the old revision of a procedure -
silently, and only after a bulk run, so a supersede survived exactly until the
next full ingest.
- ingest_file() reads the existing state with superseded_state() before the
delete and carries it through the insert. A withdrawn document is
re-ingested as withdrawn, and logs that it did so.
- --all skips withdrawn documents, so a bulk run does not spend an embeddings
call on a document that will not be cited either way. --include-superseded
overrides it; the chunks still come back withdrawn.
- --restore DOC_NUMBER REVISION is the counterpart to --supersede, refusing
while another revision of the same document is live. Without it the
conservative default would be a dead end - mark_superseded() only ever sets
TRUE, so there was no way back.
- Rule 5 in the module docstring, beside the other four.
Corrects a claim in the Phase 9 design: 16.10 said moving the withdrawn file
out of /datadisk/ai-docs "is not tidying" because otherwise --all resurrects it.
That was true when written and is not now. The guard belongs in the ingest code,
because a rule that depends on somebody remembering to move a file is not a
rule. The move stays as archival housekeeping and is documented as such in
16.10, db/005_doc_actions.sql and README.md.
Phase 3's gate gains the proof: supersede a revision, run --all again, ask the
question that used to cite it.
Not executed anywhere - no Postgres or Docker on this machine.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Claude
parent
3262f1a050
commit
0c0bd0ef6d
4 changed files with 126 additions and 13 deletions
|
|
@ -467,6 +467,7 @@ Deployed early, deliberately: from here on, every experiment is traced.
|
|||
- [ ] Step sequences intact — verify by eye on at least 3 procedures
|
||||
- [ ] "How do I lift the interlock on Pump 02" retrieves the governing procedure in the top 3, filtered to `doc_type = 'procedure'`
|
||||
- [ ] A superseded revision is never returned
|
||||
- [ ] **Supersede a revision, then run `--all` again, then ask the question that used to cite it.** It must still not be cited. This is the bulk-re-run resurrection defect; the guard is in `ingest.py` and it is cheap to prove
|
||||
- [ ] `/datadisk` usage still comfortable
|
||||
|
||||
---
|
||||
|
|
@ -586,7 +587,7 @@ Deployed early, deliberately: from here on, every experiment is traced.
|
|||
2. `/datadisk/ai-docs-inbox` and `/datadisk/ai-docs-withdrawn` — the writable staging and archive areas. **Check `df -h /datadisk` first.** Never on `/`. Create them owned by uid 10002 *before* uncommenting the `ai-api` volume block in `compose/ai-compose.yml`, and drop `profiles: [worker]` from `ai-docs-worker` in the same commit that adds `worker.py` — both are guarded so that a deploy from `main` today starts nothing that does not exist yet.
|
||||
3. `ai-api`: the `/docs/*` router — upload, list, detail, approve, reject, withdraw, restore, purge. Identity comes from Authelia's forwarded headers, never from the request body. Approval requires the publisher group and is refused without it, whatever Authelia allowed through.
|
||||
4. `ai-docs-worker` — the `ingest` image with `worker.py` as its entrypoint. Long-running, `ai-internal` only, no published port. Pre-scans uploads for a header proposal, ingests approved ones, and completes withdrawals, restores and purges. `/datadisk/ai-docs-withdrawn` is created and mounted with the other two.
|
||||
- **While you are in `ingest.py`: make `mark_superseded()` move the superseded file out of the tree as well.** Without that, `--all` re-ingests it as live — see the defect note in section 16.10.
|
||||
- The worker moves withdrawn files to `/datadisk/ai-docs-withdrawn/<date>/`. This is housekeeping, not the safety mechanism — `ingest.py` already refuses to resurrect a withdrawn document whatever folder it is in (section 16.10).
|
||||
5. `ingest.py`: extract `ingest_file(path, header=...)` so a header confirmed in the UI is passed in. `confirm_header()` stays the CLI path. Neither one gets a way to ingest an unconfirmed header. The DSN already resolves through `INGEST_DB_USER`, so the worker inherits the right role by construction.
|
||||
6. `db/006_doc_pool.sql` — `pool_enabled`, the profile tables, `pool_status` / `pool_documents`. Then `tools/retrieval.py` gains the `pool_enabled` predicate and an optional per-request profile, `contracts.py` gains `pool_scope` on `BaseAnswer`, and the UI gains the banner. **Read the HNSW note at the top of 006 before trimming the pool for a demo.**
|
||||
7. `ai-web`: a **Documents** view — upload form, review queue, review screen, the published list, and the pool screen. The nav entry is hidden without the publisher group; the hiding is cosmetic, the API check is the control.
|
||||
|
|
@ -831,9 +832,11 @@ Three operations, and the difference between them is the design:
|
|||
|
||||
**Withdraw is the answer to "remove this document" almost every time.** Retrieval already filters `superseded = FALSE`, so a withdrawal takes effect on the next question — no re-index, no restart, no worker round trip. The chunks stay in the table, which is what lets somebody answer "why did the assistant stop citing WRPS-OPS-014, and who decided that?" a month later. Deleting the rows answers the same question with silence.
|
||||
|
||||
**The file must leave the document tree too, and that is not tidying.** `ingest_file()` inserts every chunk with `superseded = FALSE`. A withdrawn document still sitting in `/datadisk/ai-docs/procedures/` comes back **live** the next time anyone runs `ai-ingest --all`, and nobody is watching for it. So a withdrawal moves the file to `/datadisk/ai-docs-withdrawn/<date>/`, outside the four `doc_type` folders that `--all` walks. The database flip is immediate and synchronous; the file move is queued to the worker, because `ai-api` has no write access to the document tree and is not getting any. Until the move completes the `doc_actions` row stays `pending`, and the UI says "withdrawn, file move pending" rather than claiming it is finished.
|
||||
**Withdrawal survives a re-ingest, and that is enforced in the ingest code rather than by where the file sits.** `ingest_file()` used to insert every chunk with `superseded = FALSE`, so replacing a document's chunks reset its withdrawal — one `ai-ingest --all` and every withdrawn revision was citable again, including the old revision of a procedure, with nobody watching for it. It now reads the existing state before replacing, carries it through, says so in the log, and `--all` skips withdrawn documents outright. Re-ingesting cannot resurrect.
|
||||
|
||||
> **Existing defect, same root cause.** This already bites without any UI: `--supersede` marks rev 3 superseded, rev 3's file stays in `procedures/`, and the next `--all` re-ingests it as live. The supersede survives only until the next bulk run. Moving superseded files out of the tree — which Phase 9 does for withdrawals — is the fix for both, and worth doing to `mark_superseded()` at the same time.
|
||||
> **This was a live defect and is now fixed** (`ingest/ingest.py`, rule 5 in its docstring). It bit without any UI: `--supersede` marked rev 3 superseded, rev 3's file stayed in `procedures/`, and the next bulk run made it live again — so a supersede survived only until the next `--all`. The fix is in the ingest code, not in where the file lives, because a rule that depends on somebody remembering to move a file is not a rule. `--restore DOC_NUMBER REVISION` is the counterpart, and refuses while another revision of the same document is live.
|
||||
|
||||
**The file still moves out of the tree on withdrawal, but as archival housekeeping** — `/datadisk/ai-docs/` should mean "the documents this plant runs on", and a withdrawn one sitting in it invites the next person to wonder. The move is queued to the worker, because `ai-api` has no write access to the document tree and is not getting any; the database flip is immediate and synchronous, and that flip alone is what stops citation. Until the move completes the `doc_actions` row stays `pending` and the UI says so. **Correcting an earlier claim in this section: the move is not the safety mechanism.** It was, in the first draft of this design, when the ingest code still reset the flag.
|
||||
|
||||
**Restore exists because withdrawing the wrong document is a thing people do.** It refuses while another revision of the same `doc_number` is live: restoring rev 3 next to rev 4 puts two revisions of one procedure in front of an operator, which is the exact failure the superseded filter was built to prevent. It goes through the worker rather than the API, so that everything which makes a document citable — publishing and restoring alike — passes through the component with no HTTP surface.
|
||||
|
||||
|
|
|
|||
18
README.md
18
README.md
|
|
@ -188,6 +188,13 @@ safety issue, not a data-quality one. When a new revision lands:
|
|||
docker compose -f ~/ai-compose.yml run --rm ai-ingest --supersede WRPS-OPS-014 4
|
||||
```
|
||||
|
||||
That sticks. Re-ingesting a superseded document brings it back superseded, and
|
||||
`--all` skips it — the flag is not reset by replacing chunks. To undo one:
|
||||
|
||||
```bash
|
||||
docker compose -f ~/ai-compose.yml run --rm ai-ingest --restore WRPS-OPS-014 3
|
||||
```
|
||||
|
||||
This is the SSH path, and it stays. Phase 9 adds the same thing as a screen, so
|
||||
that an operator who is issued a new document when the PLC logic changes does
|
||||
not have to find someone with a host login. It does not remove the header
|
||||
|
|
@ -272,10 +279,13 @@ irreversible, off unless `ALLOW_PURGE=true` and the publisher types the document
|
|||
number). All three need the publisher group and a written reason, and all three
|
||||
are recorded in `doc_actions`, which nothing can delete from.
|
||||
|
||||
Withdrawal also **moves the file out of `/datadisk/ai-docs`**. That is not
|
||||
tidying: `ingest_file()` inserts every chunk with `superseded = FALSE`, so a
|
||||
withdrawn document left in the tree comes back live on the next
|
||||
`ai-ingest --all`. The same is true of `--supersede` today — fix both together.
|
||||
Withdrawal also moves the file out of `/datadisk/ai-docs` into an archive, so
|
||||
that folder keeps meaning "the documents this plant runs on". That is
|
||||
housekeeping, not the safety mechanism: `ingest.py` reads a document's
|
||||
withdrawal state before replacing its chunks and carries it through, and `--all`
|
||||
skips withdrawn documents — so re-ingesting cannot resurrect one, whatever
|
||||
folder it is in. `--restore DOC_NUMBER REVISION` is the way back, and it refuses
|
||||
while another revision of the same document is live.
|
||||
|
||||
Then the manual steps: the `copy_headers` change on the `api.yokogawa.tech`
|
||||
Caddy block, the `^/docs/.*` Authelia rule **above** the general one, and
|
||||
|
|
|
|||
|
|
@ -83,11 +83,15 @@ CREATE INDEX IF NOT EXISTS doc_actions_pending
|
|||
-- file out of /datadisk/ai-docs needs the worker, because ai-api has no write
|
||||
-- access to the document tree and is not getting any.
|
||||
--
|
||||
-- THE FILE MOVE IS NOT OPTIONAL TIDYING. `ingest_file()` inserts every chunk
|
||||
-- with superseded = FALSE, so `ai-ingest --all` re-ingests a withdrawn document
|
||||
-- as LIVE. A withdrawn procedure left in the document tree is one bulk re-run
|
||||
-- away from being citable again, and nobody would be watching for it. Until the
|
||||
-- move completes, `doc_actions.status` stays 'pending' and the UI says so.
|
||||
-- The file move is archival housekeeping, NOT the safety mechanism. It used to
|
||||
-- be both: ingest_file() inserted every chunk with superseded = FALSE, so a
|
||||
-- withdrawn document left in the tree came back LIVE on the next
|
||||
-- `ai-ingest --all`. That is fixed in ingest.py itself - withdrawal state is
|
||||
-- read before chunks are replaced and carried through, and --all skips
|
||||
-- withdrawn documents - so citation stops on the database flip alone, whatever
|
||||
-- folder the file is in. The move keeps /datadisk/ai-docs meaning "the
|
||||
-- documents this plant runs on". Until it completes, doc_actions.status stays
|
||||
-- 'pending' and the UI says so.
|
||||
-- -----------------------------------------------------------------------------
|
||||
|
||||
-- =============================================================================
|
||||
|
|
|
|||
|
|
@ -29,6 +29,11 @@ FOUR RULES, in descending order of how badly it goes if you break them:
|
|||
|
||||
4. RE-RUNS REPLACE, NEVER DUPLICATE. Chunks for a source_file are deleted and
|
||||
reinserted in one transaction.
|
||||
|
||||
5. RE-INGESTING NEVER RESURRECTS A WITHDRAWN DOCUMENT. A superseded document
|
||||
comes back superseded, and --all skips it entirely. Replacing chunks used to
|
||||
reset the flag to FALSE, so one bulk re-run quietly made every withdrawn
|
||||
revision citable again - including the old revision of a procedure.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -238,6 +243,22 @@ def embed_all(texts: list[str], client: AzureOpenAI, model: str) -> list[list[fl
|
|||
return vectors
|
||||
|
||||
|
||||
def superseded_state(conn: psycopg.Connection, source_file: str) -> bool | None:
|
||||
"""Is this file already ingested, and was it withdrawn? None = not ingested.
|
||||
|
||||
bool_or, not bool_and: if any chunk of the file is superseded the document
|
||||
is treated as superseded. The conservative direction is the one that keeps a
|
||||
withdrawn procedure out of an answer.
|
||||
"""
|
||||
with conn.cursor() as cur:
|
||||
cur.execute(
|
||||
"SELECT bool_or(superseded) FROM doc_chunks WHERE source_file = %s",
|
||||
(source_file,),
|
||||
)
|
||||
row = cur.fetchone()
|
||||
return row[0] if row else None
|
||||
|
||||
|
||||
def ingest_file(path: Path, conn: psycopg.Connection, client: AzureOpenAI, *, assume_yes: bool) -> int:
|
||||
doc_type = doc_type_for(path)
|
||||
sections = parse_document(path)
|
||||
|
|
@ -254,12 +275,24 @@ def ingest_file(path: Path, conn: psycopg.Connection, client: AzureOpenAI, *, as
|
|||
|
||||
records: list[tuple] = []
|
||||
source_file = str(path.relative_to(DOCS_ROOT))
|
||||
|
||||
# Withdrawal survives a re-ingest. Replacing the chunks must not silently
|
||||
# give this document its citability back - if it was superseded before, it
|
||||
# is superseded after, and saying so out loud is the point.
|
||||
superseded = superseded_state(conn, source_file) or False
|
||||
if superseded:
|
||||
log.warning(
|
||||
"%s is currently SUPERSEDED - re-ingesting it as superseded. It "
|
||||
"will not be cited. Use --restore %s %s to bring it back.",
|
||||
source_file, header.doc_number, header.revision,
|
||||
)
|
||||
|
||||
for page, title, body in sections:
|
||||
for chunk in chunk_section(body, doc_type):
|
||||
records.append(
|
||||
(
|
||||
source_file, doc_type, header.doc_number, header.revision,
|
||||
header.effective_date, False,
|
||||
header.effective_date, superseded,
|
||||
link_equipment(chunk, equipment_ids), page, title, chunk,
|
||||
)
|
||||
)
|
||||
|
|
@ -364,6 +397,38 @@ def require_write_access(conn: psycopg.Connection) -> None:
|
|||
)
|
||||
|
||||
|
||||
def restore(conn: psycopg.Connection, doc_number: str, revision: str) -> int:
|
||||
"""Bring a withdrawn revision back. The counterpart of --supersede.
|
||||
|
||||
Refused while another revision of the same document is live. Restoring
|
||||
rev 3 next to rev 4 puts two revisions of one procedure in front of an
|
||||
operator, which is the failure the superseded filter exists to prevent -
|
||||
and it is a likelier mistake than it sounds, because the person restoring
|
||||
is usually looking at the old revision, not the new one.
|
||||
"""
|
||||
with conn.cursor() as cur:
|
||||
cur.execute(
|
||||
"SELECT DISTINCT revision FROM doc_chunks"
|
||||
" WHERE doc_number = %s AND revision <> %s AND superseded = FALSE",
|
||||
(doc_number, revision),
|
||||
)
|
||||
live = [row[0] for row in cur.fetchall()]
|
||||
if live:
|
||||
raise SystemExit(
|
||||
f"{doc_number} revision {', '.join(live)} is live. Restoring "
|
||||
f"revision {revision} would put two revisions of one document in "
|
||||
"front of an operator. Supersede the other one first, if that is "
|
||||
"really what you mean."
|
||||
)
|
||||
|
||||
cur.execute(
|
||||
"UPDATE doc_chunks SET superseded = FALSE"
|
||||
" WHERE doc_number = %s AND revision = %s AND superseded = TRUE",
|
||||
(doc_number, revision),
|
||||
)
|
||||
return cur.rowcount
|
||||
|
||||
|
||||
def main() -> int:
|
||||
parser = argparse.ArgumentParser(description=__doc__)
|
||||
parser.add_argument("--all", action="store_true", help="ingest every document")
|
||||
|
|
@ -379,6 +444,20 @@ def main() -> int:
|
|||
metavar=("DOC_NUMBER", "KEEP_REVISION"),
|
||||
help="mark every other revision of a document superseded",
|
||||
)
|
||||
parser.add_argument(
|
||||
"--restore",
|
||||
nargs=2,
|
||||
metavar=("DOC_NUMBER", "REVISION"),
|
||||
help="bring a withdrawn revision back; refused if another revision is live",
|
||||
)
|
||||
parser.add_argument(
|
||||
"--include-superseded",
|
||||
action="store_true",
|
||||
help=(
|
||||
"with --all, do not skip withdrawn documents. They are still "
|
||||
"re-ingested AS withdrawn - this only spends the embedding call"
|
||||
),
|
||||
)
|
||||
args = parser.parse_args()
|
||||
|
||||
client = AzureOpenAI(
|
||||
|
|
@ -395,6 +474,12 @@ def main() -> int:
|
|||
log.info("marked %d chunks superseded", count)
|
||||
return 0
|
||||
|
||||
if args.restore:
|
||||
count = restore(conn, *args.restore)
|
||||
conn.commit()
|
||||
log.info("restored %d chunks", count)
|
||||
return 0
|
||||
|
||||
if args.file:
|
||||
paths = [DOCS_ROOT / args.file]
|
||||
elif args.all:
|
||||
|
|
@ -404,6 +489,17 @@ def main() -> int:
|
|||
for p in (DOCS_ROOT / folder).glob("**/*")
|
||||
if p.is_file() and p.suffix.lower() in {".pdf", ".docx", ".md", ".txt"}
|
||||
)
|
||||
# A withdrawn document is still sitting in the tree - nothing moves
|
||||
# it. Skipping it keeps a bulk re-run from spending an embeddings
|
||||
# call on a document that will not be cited either way.
|
||||
if not args.include_superseded:
|
||||
keep = []
|
||||
for p in paths:
|
||||
if superseded_state(conn, str(p.relative_to(DOCS_ROOT))):
|
||||
log.info("skipping %s - superseded", p.relative_to(DOCS_ROOT))
|
||||
else:
|
||||
keep.append(p)
|
||||
paths = keep
|
||||
else:
|
||||
parser.error("give --all or --file")
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue