From 5217d8bf6a2e726be54fb5155d3890e9c128849c Mon Sep 17 00:00:00 2001 From: shruti0025 Date: Mon, 21 Sep 2026 07:22:26 +0530 Subject: [PATCH 01/11] feat: add an approver-set validity window to the network catalog [#111] The catalog announced to the Discovery Service carried no lifetime, so an announcement stood indefinitely under updateMode: MERGE. The prod approver now names the window at the approve-prod gate, where they already decide a document is fit for PROD. Validated and stored before anything is promoted, so a rejected window is a 400 that leaves the document parked at the gate. --- AGENTS.md | 1 + CONTEXT.md | 4 + ...ts-the-network-catalogs-validity-window.md | 21 ++++ docs/SYSTEM_DESIGN.md | 9 +- docs/architecture-pipeline-rbac.md | 5 +- pipeline/activities.py | 19 +-- pipeline/api.py | 38 +++++- pipeline/catalog_builder.py | 26 +++- pipeline/db.py | 33 +++++ pipeline/discovery_publish_service.py | 12 +- pipeline/document_repository.py | 15 +++ pipeline/models.py | 17 +++ pipeline/network_validity.py | 118 ++++++++++++++++++ tests/test_activities.py | 104 ++++++++++++++- tests/test_api.py | 114 +++++++++++++++++ tests/test_catalog_builder.py | 38 ++++++ tests/test_discovery_publish_service.py | 59 +++++++++ tests/test_document_repository.py | 41 ++++++ tests/test_network_validity.py | 118 ++++++++++++++++++ ui/src/views/DocumentOpsView.jsx | 104 ++++++++++++++- 20 files changed, 866 insertions(+), 30 deletions(-) create mode 100644 docs/ADR/0005-approver-sets-the-network-catalogs-validity-window.md create mode 100644 pipeline/network_validity.py create mode 100644 tests/test_network_validity.py diff --git a/AGENTS.md b/AGENTS.md index 66b9e9b..8698c2c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,6 +62,7 @@ Core modules (all under `pipeline/`): - `document_repository.py` — `DocumentRepository`, a domain layer over `db.py` for document reads; `db.py` stays one-function-per-query with no domain knowledge - `network_constants.py` — fixed values sent on the network (catalog/resource ids, topics, languages, Beckn version). The file v2 edits when the AI layer starts deriving them - `catalog_builder.py` — pure builder mapping a document's knowledge kind (`advisory`/`scheme`) to the single `OnDemand` Beckn catalog announced for that kind; returns `None` for any other kind. No env, no I/O +- `network_validity.py` — pure module owning the Announcement Lifetime: what a legal `validity` window is, the today/today default an approver starts from, and how it renders on the wire. Shared by `api.py` (validating approver input) and `catalog_builder.py` (placing it on the catalog) - `discovery_publish_service.py` — `DiscoveryPublishService`: owns the Publish to Network env vars and the HTTP call. Deliberately Temporal-free - `models.py` — Pydantic models, including `DocumentStage` enum and `PIPELINE_STAGES` (the stepper-UI stage list) - `config.py` — `Config` dataclass reading env vars, with defaults diff --git a/CONTEXT.md b/CONTEXT.md index 0df94d7..f667d68 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -28,5 +28,9 @@ _Avoid_: "catalog" alone. What a document *is* to the network — `advisory`, `scheme`, `video`, or an operator-entered slug — held in `documents.document_kind`. Asserted by a reviewer during the pipeline, never inferred from the file, and absent until then (every document starts as the default `document`). Only `advisory` and `scheme` map to a Network Catalog Envelope; the rest publish nothing. _Avoid_: "document type" — collides with `source_type`/`canonical_input_type`, which describe the input *format* (pdf, spreadsheet). Also avoid saying an advisory is "uploaded": what is uploaded is a file, which only becomes an advisory when someone classifies it. +**Announcement Lifetime**: +The `validity` window (`startDate`/`endDate`) carried on the Network Catalog Envelope, named by the prod approver at `approve-prod` and stored as `documents.network_valid_from` / `network_valid_to`. Start is the approval date; end is prepopulated with the same day and is the approver's to move. Because the envelope is kind-level, the most recently published window is the live one for that whole Knowledge Kind — see `docs/ADR/0005-approver-sets-the-network-catalogs-validity-window.md`. +_Avoid_: "document lifetime" — the document itself is not what expires, the announcement is. Also avoid "expiry": there is a start as well as an end. + **network_visible**: An operator-controlled flag on a document/scheme (not a pipeline stage) that gates whether it's exposed to other BAPs through the pull-based Scheme Catalog snapshot. Independent of Publish to Network. diff --git a/docs/ADR/0005-approver-sets-the-network-catalogs-validity-window.md b/docs/ADR/0005-approver-sets-the-network-catalogs-validity-window.md new file mode 100644 index 0000000..31a0bc6 --- /dev/null +++ b/docs/ADR/0005-approver-sets-the-network-catalogs-validity-window.md @@ -0,0 +1,21 @@ +# The prod approver sets the published catalog's validity window + +Context: `publishing_to_network` announced a catalog with no lifetime at all. Under `updateMode: MERGE` on a permanent catalog id (ADR 0003), that announcement stands indefinitely — a provider that stops serving a knowledge kind has no way to say so on the wire, and no operator ever states how long the claim should hold. The `network-specs` pack answers this at two levels: `resourceAttributes.validity` on a resource (a `TimePeriod` of `startsAt`/`endsAt`), and a **catalog-level** `validity` of `startDate`/`endDate`, which every provider-catalog example in `schema/examples/` carries. + +Decision: +- The **prod approver** names the window, at the same gate where they already decide a document is fit for PROD. `POST /documents/{id}/approve-prod` takes an optional `{network_valid_from, network_valid_to}` body. +- **Start date is the approval date**, defaulted server-side and shown read-only in the UI: the announcement begins when it is published, and nothing is gained by letting an approver backdate a claim the network only learns about now. +- **End date is prepopulated with the same day and is the one field the approver edits.** Today/today is a deliberate floor rather than a guess — an approver who wants the catalog to stand longer has to say so, instead of inheriting a default nobody chose. +- **Validated before anything is promoted.** The window is parsed and stored at the top of `approve-prod`, before any signal is sent or `PromoteToProdWorkflow` is started, so a rejected window is a plain 400 that leaves the document parked at the gate. `publishing_to_network` therefore only ever reads a window that passed `network_validity.parse_window`. Validation is deliberately thin: a real `YYYY-MM-DD` at each end, and an end not before the start. +- The window lands on the **catalog**, not on `resourceAttributes`. ADR 0003 pins the resource to `informationMode: OnDemand`, whose profile omits `validity`; catalog-level `validity` is where the specs' own examples put a published announcement's lifetime. Dates are widened to the instants those examples use, with the end date inclusive to `23:59:59Z` of that day. +- Stored on the document (`documents.network_valid_from` / `network_valid_to`) and read back by the activity, not threaded through as a workflow argument — that keeps every existing call site and every in-flight Temporal execution untouched, the same reasoning that already applies to `document_kind` in `publish_catalog_to_network`. +- A document with **no stored window publishes without a `validity`**, exactly as before. That covers everything promoted before this change. It is not defaulted to today on read: doing so would expire an established catalog the first time an old document was re-ingested. + +Known limitation, accepted: the envelope is **kind-level** (ADR 0003), so the window an approver sets for one document becomes the window for its whole knowledge kind — the last publish wins under MERGE. A per-document lifetime is not expressible until v2 introduces per-document, AI-derived catalogs; until then "lifetime of the document" means "lifetime of the announcement this document triggered". Operators approving several documents of one kind should expect the most recent window to be the live one. + +Alternatives considered: +- **Collect the window at `ready_for_ingestion` (Publish to Dev) instead.** Rejected: that gate is about DEV content quality and is open to `state_contributor`; the lifetime of a network-facing claim belongs with the super admin who makes that claim (ADR 0004). +- **Put it on `resourceAttributes.validity`.** Rejected: contradicts ADR 0003's OnDemand shape, and the existing `test_catalog_builder` guard against exactly this field. +- **Make both dates required with no default.** Rejected: it breaks every body-less `approve-prod` caller (scripts, the bulk pattern) for no safety gain, since the endpoint validates whatever it ends up with either way. +- **Let the approver backdate the start.** Rejected as the default, but the API still accepts and validates a supplied `network_valid_from` rather than silently ignoring it — an operator scripting a deliberate window gets what they asked for or a 400, never a quiet override. +- **Auto-expire the catalog by re-publishing `isActive: false` at `endDate`.** Out of scope: it needs a scheduler this pipeline does not have, and the window on the wire is what the Discovery Service is meant to enforce. diff --git a/docs/SYSTEM_DESIGN.md b/docs/SYSTEM_DESIGN.md index 7fbaac3..451142f 100644 --- a/docs/SYSTEM_DESIGN.md +++ b/docs/SYSTEM_DESIGN.md @@ -195,10 +195,11 @@ read and write paths stay consistent. 1. Super Admin sees all states’ documents in `approval_for_prod` 2. Reviews pages/chunks (optional quality check) -3. `POST /documents/{id}/approve-prod` (`RequireAdmin`) -4. Signal `approve_prod` **or** `PromoteToProdWorkflow` -5. `promote_document_to_prod_qdrant` → **PROD Qdrant** -6. Stage `completed`; audit `promote_to_prod` +3. Sets the **Announcement Lifetime** — start date is the approval date, end date prepopulated with the same day and editable +4. `POST /documents/{id}/approve-prod` (`RequireAdmin`) with `{network_valid_from, network_valid_to}`; the window is validated and stored before anything is promoted, so a bad window is a 400 that promotes nothing +5. Signal `approve_prod` **or** `PromoteToProdWorkflow` +6. `promote_document_to_prod_qdrant` → **PROD Qdrant** +7. Stage `completed`; audit `promote_to_prod`. The stored window rides along on the `publishing_to_network` catalog as `validity` ### 5.3 Search diff --git a/docs/architecture-pipeline-rbac.md b/docs/architecture-pipeline-rbac.md index 91e4358..b1649c1 100644 --- a/docs/architecture-pipeline-rbac.md +++ b/docs/architecture-pipeline-rbac.md @@ -225,7 +225,7 @@ Signals (workflow): `approve_ocr`, `approve_translation`, `approve_chunks`, `app API (examples): - `POST /documents/{id}/approve-ocr` … `approve-ingestion` → state-capable roles with `review` -- `POST /documents/{id}/approve-prod` → **`RequireAdmin`** (Super Admin) +- `POST /documents/{id}/approve-prod` → **`RequireAdmin`** (Super Admin). Also carries the Announcement Lifetime (`network_valid_from` / `network_valid_to`), validated before any promotion is triggered — see `docs/ADR/0005-approver-sets-the-network-catalogs-validity-window.md` UI gates: `DocumentOpsView` maps `approve_prod` → permission `admin`; other approvals → `review`. @@ -298,7 +298,8 @@ sequenceDiagram UI->>API: GET /auth/me - unrestricted + admin SA->>UI: Open Approval for Prod queue SA->>API: Review document detail - SA->>API: POST /documents/id/approve-prod + SA->>UI: Set announcement lifetime - start today, end editable + SA->>API: POST /documents/id/approve-prod with validity window API->>T: Signal approve_prod or PromoteToProdWorkflow T->>PROD: promote_document_to_prod_qdrant T->>API: Set stage completed diff --git a/pipeline/activities.py b/pipeline/activities.py index ec8304a..16412c8 100644 --- a/pipeline/activities.py +++ b/pipeline/activities.py @@ -1419,19 +1419,22 @@ async def ingest_document_from_db( async def publish_catalog_to_network(workflow_id: str, transaction_id: str) -> dict: """POST a catalog/publish envelope to the Discovery Service and record the exchange. - The catalog depends on the document's knowledge kind, which is read here - rather than passed in as an activity argument - that keeps both workflow - call sites, and any workflow already in flight, untouched. + The catalog depends on the document's knowledge kind and on the lifetime the + prod approver set for it. Both are read here rather than passed in as + activity arguments - that keeps every workflow call site, and any workflow + already in flight, untouched. """ from . import db - document_kind = normalize_document_kind( - DocumentRepository().get_document_kind(workflow_id) - ) + repository = DocumentRepository() + document_kind = normalize_document_kind(repository.get_document_kind(workflow_id)) + # Set at the prod approval gate and already validated there; None only for a + # document promoted before approvers named a lifetime. + validity = repository.get_network_validity(workflow_id) service = DiscoveryPublishService() result = await asyncio.to_thread( - service.publish, transaction_id, document_kind, workflow_id + service.publish, transaction_id, document_kind, workflow_id, validity ) if result["skipped"]: @@ -1490,6 +1493,8 @@ async def publish_catalog_to_network(workflow_id: str, transaction_id: str) -> d return { "status": "skipped" if result["skipped"] else "published", "document_kind": document_kind, + "valid_from": validity.start_date if validity else None, + "valid_to": validity.end_date if validity else None, "transaction_id": transaction_id, "message_id": None if result["skipped"] else result["envelope"]["context"]["messageId"], "result_status": result["result_status"], diff --git a/pipeline/api.py b/pipeline/api.py index b7d04a6..3c61a7c 100644 --- a/pipeline/api.py +++ b/pipeline/api.py @@ -95,6 +95,7 @@ OperationQueueEntry, OperationQueueResponse, PageUpdate, + ProdApprovalRequest, RegisterFolderRequest, RegisterRequest, ReindexStateRequest, @@ -103,6 +104,7 @@ SearchSettingsUpdate, SettingsAuditResponse, ) +from .network_validity import ValidityWindowError, parse_window from .workflows import ( ChunkingOnlyWorkflow, DocumentPipelineWorkflow, @@ -454,6 +456,8 @@ def _document_summary_from_row(doc: dict, current_job: Optional[dict] = None) -> network_visible=bool(int(doc["network_visible"])) if doc.get("network_visible") is not None else True, prod_ready_requested_at=doc.get("prod_ready_requested_at"), prod_ready_requested_by_username=doc.get("prod_ready_requested_by_username"), + network_valid_from=doc.get("network_valid_from"), + network_valid_to=doc.get("network_valid_to"), ) @@ -1533,6 +1537,8 @@ def _build_document_detail(doc: dict) -> DocumentDetail: network_visible=bool(int(doc["network_visible"])) if doc.get("network_visible") is not None else True, prod_ready_requested_at=doc.get("prod_ready_requested_at"), prod_ready_requested_by_username=doc.get("prod_ready_requested_by_username"), + network_valid_from=doc.get("network_valid_from"), + network_valid_to=doc.get("network_valid_to"), ) @@ -2992,8 +2998,19 @@ async def request_prod_ready(workflow_id: str, user: RequireReview): @app.post("/documents/{workflow_id}/approve-prod") -async def approve_prod(workflow_id: str, user: RequireAdmin): - """Superadmin-only: promote DEV-ingested vectors into PROD Qdrant.""" +async def approve_prod( + workflow_id: str, + user: RequireAdmin, + approval: Optional[ProdApprovalRequest] = None, +): + """Superadmin-only: promote DEV-ingested vectors into PROD Qdrant. + + The approver also names the lifetime of the network announcement this + promotion leads to - start date today, end date theirs to set. The window is + validated and stored here, before anything is signaled or started, so a + rejected window costs nothing: `publishing_to_network` only ever reads a + window that passed through `parse_window`. + """ doc = _require_document_for_user(workflow_id, user) stage = doc.get("stage") # Promoting an already-completed document is a fresh PROD write, so it is @@ -3013,6 +3030,19 @@ async def approve_prod(workflow_id: str, user: RequireAdmin): "(expected 'approval_for_prod' or 'completed').", ) + # Validated before any promotion is triggered: the publish that follows + # reads these dates off the document, and a 400 here must leave the + # document exactly where it was. + try: + validity = parse_window( + approval.network_valid_from if approval else None, + approval.network_valid_to if approval else None, + ) + except ValidityWindowError as exc: + raise HTTPException(400, f"Cannot approve for prod: {exc}") from None + + db.set_network_validity(workflow_id, validity.start_date, validity.end_date) + # Prefer signaling the running main workflow when it is waiting at approval_for_prod. signaled = False if stage == "approval_for_prod": @@ -3070,6 +3100,8 @@ async def approve_prod(workflow_id: str, user: RequireAdmin): "next_stage": "ingesting_prod", "signaled": signaled, "promote_workflow_id": promote_workflow_id, + "network_valid_from": validity.start_date, + "network_valid_to": validity.end_date, }, user=user, ) @@ -3078,6 +3110,8 @@ async def approve_prod(workflow_id: str, user: RequireAdmin): "workflow_id": workflow_id, "signaled": signaled, "promote_workflow_id": promote_workflow_id, + "network_valid_from": validity.start_date, + "network_valid_to": validity.end_date, "next_stage": "ingesting_prod", } diff --git a/pipeline/catalog_builder.py b/pipeline/catalog_builder.py index f6a3a84..f4c25f7 100644 --- a/pipeline/catalog_builder.py +++ b/pipeline/catalog_builder.py @@ -5,9 +5,10 @@ document of that kind - a seeker that matches the resource comes back to ask the actual question, so there is nothing per-document to put on the wire. -Pure: no env, no I/O. Values live in network_constants.py; the transport lives -in discovery_publish_service.py. See -docs/ADR/0003-static-ondemand-network-catalog-per-knowledge-kind.md. +Pure: no env, no I/O. Values live in network_constants.py, the validity window +in network_validity.py, and the transport in discovery_publish_service.py. See +docs/ADR/0003-static-ondemand-network-catalog-per-knowledge-kind.md and +docs/ADR/0005-approver-set-validity-window-on-the-network-catalog.md. """ import copy @@ -32,6 +33,7 @@ SCHEMES_RESOURCE_NAME, SERVED_LANGUAGES, ) +from .network_validity import ValidityWindow, to_catalog_validity _ADVISORY_RESOURCE_ATTRIBUTES = { "@context": f"{SCHEMA_CONTEXT_BASE}/KnowledgeAdvisory/v0.1/context.jsonld", @@ -80,18 +82,29 @@ def normalize_document_kind(document_kind: Optional[str]) -> str: return (document_kind or DEFAULT_DOCUMENT_KIND).strip().lower() -def build_catalog(document_kind: Optional[str]) -> Optional[dict]: +def build_catalog( + document_kind: Optional[str], + validity: Optional[ValidityWindow] = None, +) -> Optional[dict]: """Return the catalog to announce for `document_kind`, or None. None means this kind has nothing to announce - `document`, `video` and any operator-entered custom slug. Callers must skip the publish rather than send an empty `catalogs` array, which the spec rejects (`minItems: 1`). + + `validity` is the lifetime an approver set for the document being published. + It lands on the **catalog**, not on `resourceAttributes`: the OnDemand + KnowledgeAdvisory profile omits a resource-level `validity` (ADR 0003), and + catalog-level `validity` is where the network specs' own provider-catalog + examples put a published announcement's lifetime. Omitted entirely when + None, which leaves a pre-existing announcement's lifetime untouched under + `updateMode: MERGE`. """ spec = _KNOWLEDGE_KIND_CATALOGS.get(normalize_document_kind(document_kind)) if spec is None: return None - return { + catalog = { "id": spec["catalog_id"], "descriptor": {"name": spec["catalog_name"]}, "isActive": True, @@ -104,3 +117,6 @@ def build_catalog(document_kind: Optional[str]) -> Optional[dict]: } ], } + if validity is not None: + catalog["validity"] = to_catalog_validity(validity) + return catalog diff --git a/pipeline/db.py b/pipeline/db.py index ed914fd..f838815 100644 --- a/pipeline/db.py +++ b/pipeline/db.py @@ -118,6 +118,10 @@ def init_db(): _add_column_if_missing(conn, "documents", "prod_ready_requested_at", "TEXT") _add_column_if_missing(conn, "documents", "prod_ready_requested_by_user_id", "TEXT") _add_column_if_missing(conn, "documents", "prod_ready_requested_by_username", "TEXT") + # Lifetime the prod approver set for the network announcement. + # NULL on documents promoted before approvers named one. + _add_column_if_missing(conn, "documents", "network_valid_from", "TEXT") + _add_column_if_missing(conn, "documents", "network_valid_to", "TEXT") # Stamp NULL/empty rows with the configured default so list filters # (which coalesce to DEFAULT_INSTANCE) match migrated data. default_instance = ( @@ -1696,6 +1700,35 @@ def mark_prod_ready_requested( return get_document(workflow_id) +def set_network_validity( + workflow_id: str, + valid_from: str, + valid_to: str, +) -> Optional[dict]: + """Record the lifetime the prod approver set for this document's network + announcement. Both ends are `YYYY-MM-DD`; validation belongs to the caller + (`pipeline/network_validity.parse_window`), not to this SQL layer.""" + with _db_lock: + with get_connection() as conn: + conn.execute( + """ + UPDATE documents + SET network_valid_from = ?, + network_valid_to = ?, + updated_at = ? + WHERE workflow_id = ? + """, + ( + valid_from, + valid_to, + datetime.utcnow().isoformat(), + workflow_id, + ), + ) + conn.commit() + return get_document(workflow_id) + + def clear_prod_ready_requested(workflow_id: str) -> Optional[dict]: """Reset the request flag once prod approval actually happens (or the document leaves the approval_for_prod gate for any other reason).""" diff --git a/pipeline/discovery_publish_service.py b/pipeline/discovery_publish_service.py index d34f2b8..a7ac9cf 100644 --- a/pipeline/discovery_publish_service.py +++ b/pipeline/discovery_publish_service.py @@ -18,6 +18,7 @@ from .catalog_builder import build_catalog from .network_constants import BECKN_VERSION, RESULT_ACCEPTED +from .network_validity import ValidityWindow logger = logging.getLogger(__name__) @@ -47,6 +48,7 @@ def publish( transaction_id: str, document_kind: Optional[str], workflow_id: Optional[str] = None, + validity: Optional[ValidityWindow] = None, ) -> dict: """ POST a catalog/publish envelope for `document_kind`. @@ -55,11 +57,16 @@ def publish( reused across retries) - only `messageId` is generated fresh here, per Beckn convention. + `validity` is the lifetime the approver set for the document being + published; None announces the catalog without one. The window is + already validated by the time it reaches here - see + `network_validity.parse_window`. + A kind with no catalog mapped to it makes no HTTP call and comes back with `skipped=True`: the spec declares `message.catalogs` as `minItems: 1`, so there is no valid "publish nothing" request to send. """ - catalog = build_catalog(document_kind) + catalog = build_catalog(document_kind, validity) if catalog is None: logger.info( "workflow_id=%s document_kind=%s network_publish_skipped=True", @@ -102,12 +109,13 @@ def publish( url = f"{self.endpoint.rstrip('/')}/publish" logger.info( "workflow_id=%s catalog_id=%s transaction_id=%s message_id=%s " - "network_publish_url=%s", + "network_publish_url=%s network_validity=%s", workflow_id, catalog["id"], transaction_id, envelope["context"]["messageId"], url, + catalog.get("validity"), ) logger.debug("Request to %s with \n body %s", url, envelope) try: diff --git a/pipeline/document_repository.py b/pipeline/document_repository.py index 9f2e2c3..ab27aee 100644 --- a/pipeline/document_repository.py +++ b/pipeline/document_repository.py @@ -9,6 +9,7 @@ from typing import Optional from . import db +from .network_validity import ValidityWindow, window_from_row class DocumentRepository: @@ -28,3 +29,17 @@ def get_document_kind(self, workflow_id: str) -> Optional[str]: if not doc: return None return doc.get("document_kind") + + def get_network_validity(self, workflow_id: str) -> Optional[ValidityWindow]: + """The lifetime the prod approver set for this document's network + announcement, or None when it has none. + + None is not an error: documents promoted before approvers named a + window have no stored dates, and those publish without a validity. + """ + doc = self._db.get_document(workflow_id) + if not doc: + return None + return window_from_row( + doc.get("network_valid_from"), doc.get("network_valid_to") + ) diff --git a/pipeline/models.py b/pipeline/models.py index 43bbc8c..77d93e1 100644 --- a/pipeline/models.py +++ b/pipeline/models.py @@ -213,6 +213,23 @@ class DocumentSummary(BaseModel): network_visible: bool = True prod_ready_requested_at: Optional[str] = None prod_ready_requested_by_username: Optional[str] = None + # Lifetime of the network announcement, set by the prod approver. + network_valid_from: Optional[str] = None + network_valid_to: Optional[str] = None + + +class ProdApprovalRequest(BaseModel): + """POST /documents/{id}/approve-prod body. + + Both ends are `YYYY-MM-DD` and both are optional: an omitted end defaults to + today, so a body-less approval still publishes a well-formed window. The + dates are validated in the endpoint (see `network_validity.parse_window`) + rather than by a Pydantic validator, so a bad window comes back as the same + 400 an approver gets for a bad stage, not a 422 with a Pydantic trace. + """ + + network_valid_from: Optional[str] = None + network_valid_to: Optional[str] = None class SchemeMetadataUpdate(BaseModel): diff --git a/pipeline/network_validity.py b/pipeline/network_validity.py new file mode 100644 index 0000000..6347c91 --- /dev/null +++ b/pipeline/network_validity.py @@ -0,0 +1,118 @@ +""" +The validity window a published catalog is announced with. + +An operator names how long this provider's announcement should stand; this +module owns what a legal window is, what the default one is, and how it renders +on the wire. Pure: no env, no I/O, no db - so the API can validate an operator's +input and the activity can rebuild the same window from a stored row without +either of them re-deriving the rules. + +`pipeline/catalog_builder.py` places the rendered window on the catalog; +`pipeline/api.py` validates operator input against `parse_window`. +""" + +from dataclasses import dataclass +from datetime import date, datetime +from typing import Optional + +# Operators think in dates, not instants - the window is stored and exchanged +# as a plain calendar date and only widened to an instant on the wire. +DATE_FORMAT = "%Y-%m-%d" + + +class ValidityWindowError(ValueError): + """An operator-supplied window that cannot be published.""" + + +@dataclass(frozen=True) +class ValidityWindow: + """Inclusive calendar span, both ends `YYYY-MM-DD`.""" + + start_date: str + end_date: str + + +def today() -> str: + """Today's date in the stored form.""" + return date.today().strftime(DATE_FORMAT) + + +def default_window() -> ValidityWindow: + """The window an approver is offered before editing anything. + + Both ends are today: the announcement starts the day it is approved, and + the end is a deliberate floor rather than a guess - an approver who wants + the catalog to stand longer has to say so. + """ + stamp = today() + return ValidityWindow(start_date=stamp, end_date=stamp) + + +def _parse_date(value: str, field: str) -> date: + try: + return datetime.strptime(value.strip(), DATE_FORMAT).date() + except (AttributeError, ValueError): + raise ValidityWindowError( + f"{field} must be a calendar date formatted as YYYY-MM-DD, got {value!r}." + ) from None + + +def parse_window( + start_date: Optional[str] = None, + end_date: Optional[str] = None, +) -> ValidityWindow: + """Validate an operator-supplied window, defaulting either end to today. + + Raises `ValidityWindowError` - never returns a half-valid window, so a + caller that gets a window back can publish it unchecked. + """ + default = default_window() + raw_start = start_date if (start_date or "").strip() else default.start_date + raw_end = end_date if (end_date or "").strip() else default.end_date + + start = _parse_date(raw_start, "start date") + end = _parse_date(raw_end, "end date") + + if end < start: + raise ValidityWindowError( + f"end date ({end.strftime(DATE_FORMAT)}) cannot be before " + f"start date ({start.strftime(DATE_FORMAT)})." + ) + + return ValidityWindow( + start_date=start.strftime(DATE_FORMAT), + end_date=end.strftime(DATE_FORMAT), + ) + + +def window_from_row( + start_date: Optional[str], + end_date: Optional[str], +) -> Optional[ValidityWindow]: + """Rebuild a stored window, or None when the document has none. + + None is the normal case for a document promoted before windows existed, and + means "announce without a validity" - not an error, and not a silent + fallback to today, which would expire an established catalog. + """ + if not (start_date or "").strip() or not (end_date or "").strip(): + return None + try: + return parse_window(start_date, end_date) + except ValidityWindowError: + # A row this module never wrote. Announcing no window beats announcing + # a malformed one the Discovery Service would reject the catalog for. + return None + + +def to_catalog_validity(window: ValidityWindow) -> dict: + """The `validity` object a catalog carries on the wire. + + Widened from dates to instants because the spec's catalog examples are + date-times; the end date is inclusive, so it runs to the last second of + that day rather than its midnight boundary. + """ + return { + "startDate": f"{window.start_date}T00:00:00Z", + "endDate": f"{window.end_date}T23:59:59Z", + } diff --git a/tests/test_activities.py b/tests/test_activities.py index 8a14b97..00f3890 100644 --- a/tests/test_activities.py +++ b/tests/test_activities.py @@ -402,7 +402,7 @@ class FakeService: def __init__(self): pass - def publish(self, transaction_id, document_kind, workflow_id=None): + def publish(self, transaction_id, document_kind, workflow_id=None, validity=None): assert transaction_id == "txn-1" assert document_kind == "advisory" assert workflow_id == "wf-1" @@ -430,6 +430,10 @@ def fake_add_artifact(**kwargs): assert result == { "status": "published", "document_kind": "advisory", + # No approver window stored for this document - see + # TestPublishedLifetime for the case where one is. + "valid_from": None, + "valid_to": None, "transaction_id": "txn-1", "message_id": "msg-1", "result_status": "ACCEPTED", @@ -443,6 +447,94 @@ def fake_add_artifact(**kwargs): assert recorded["metadata"]["result_status"] == "ACCEPTED" assert recorded["metadata"]["errors"] == [] + @pytest.mark.unit + @pytest.mark.asyncio + async def test_hands_the_stored_window_to_the_service(self, monkeypatch): + """The window the approver set at the prod gate reaches the publish. + + Read here rather than passed as an activity argument, so workflows + already in flight keep working - that is exactly what makes this worth + a test of its own. + """ + import pipeline.activities as activities + import pipeline.db as db + from pipeline.network_validity import ValidityWindow + + seen = {} + + class FakeService: + def publish(self, transaction_id, document_kind, workflow_id=None, validity=None): + seen["validity"] = validity + return { + "skipped": True, + "envelope": None, + "status_code": None, + "response_body": None, + "result_status": None, + "errors": [], + } + + monkeypatch.setattr(activities, "DiscoveryPublishService", FakeService) + monkeypatch.setattr( + activities, + "_upload_file_to_minio", + lambda *a, **k: ("minio://documents/network_publish_skipped.json", 45, "application/json"), + ) + monkeypatch.setattr( + db, + "get_document", + lambda workflow_id: { + "document_kind": "advisory", + "network_valid_from": "2026-09-16", + "network_valid_to": "2026-12-31", + }, + ) + monkeypatch.setattr(db, "get_latest_document_job", lambda workflow_id: None) + monkeypatch.setattr(db, "add_document_artifact", lambda **kwargs: None) + + result = await activities.publish_catalog_to_network("wf-v", "txn-v") + + assert seen["validity"] == ValidityWindow( + start_date="2026-09-16", end_date="2026-12-31" + ) + assert result["valid_from"] == "2026-09-16" + assert result["valid_to"] == "2026-12-31" + + @pytest.mark.unit + @pytest.mark.asyncio + async def test_a_document_with_no_window_publishes_without_one(self, monkeypatch): + # Promoted before approvers named a lifetime: still publishes. + import pipeline.activities as activities + import pipeline.db as db + + seen = {} + + class FakeService: + def publish(self, transaction_id, document_kind, workflow_id=None, validity=None): + seen["validity"] = validity + return { + "skipped": True, + "envelope": None, + "status_code": None, + "response_body": None, + "result_status": None, + "errors": [], + } + + monkeypatch.setattr(activities, "DiscoveryPublishService", FakeService) + monkeypatch.setattr( + activities, + "_upload_file_to_minio", + lambda *a, **k: ("minio://documents/network_publish_skipped.json", 45, "application/json"), + ) + monkeypatch.setattr(db, "get_document", lambda workflow_id: {"document_kind": "advisory"}) + monkeypatch.setattr(db, "get_latest_document_job", lambda workflow_id: None) + monkeypatch.setattr(db, "add_document_artifact", lambda **kwargs: None) + + await activities.publish_catalog_to_network("wf-none", "txn-none") + + assert seen["validity"] is None + @pytest.mark.unit @pytest.mark.asyncio async def test_publish_failure_propagates_and_records_nothing(self, monkeypatch): @@ -450,7 +542,7 @@ async def test_publish_failure_propagates_and_records_nothing(self, monkeypatch) import pipeline.db as db class FakeService: - def publish(self, transaction_id, document_kind, workflow_id=None): + def publish(self, transaction_id, document_kind, workflow_id=None, validity=None): raise RuntimeError("discovery service unreachable") monkeypatch.setattr(activities, "DiscoveryPublishService", FakeService) @@ -472,7 +564,7 @@ async def test_unmapped_kind_records_a_skipped_artifact(self, monkeypatch): import pipeline.db as db class FakeService: - def publish(self, transaction_id, document_kind, workflow_id=None): + def publish(self, transaction_id, document_kind, workflow_id=None, validity=None): return { "skipped": True, "envelope": None, @@ -499,6 +591,8 @@ def publish(self, transaction_id, document_kind, workflow_id=None): assert result == { "status": "skipped", "document_kind": "document", + "valid_from": None, + "valid_to": None, "transaction_id": "txn-2", "message_id": None, "result_status": None, @@ -517,7 +611,7 @@ async def test_non_rejected_results_complete_the_stage(self, monkeypatch, result import pipeline.db as db class FakeService: - def publish(self, transaction_id, document_kind, workflow_id=None): + def publish(self, transaction_id, document_kind, workflow_id=None, validity=None): return { "skipped": False, "envelope": {"context": {"messageId": "msg-3"}}, @@ -551,7 +645,7 @@ async def test_rejected_result_records_the_errors_then_fails(self, monkeypatch): errors = [{"code": "SCH_SCHEMA_VALIDATION_FAILED", "message": "topics is required"}] class FakeService: - def publish(self, transaction_id, document_kind, workflow_id=None): + def publish(self, transaction_id, document_kind, workflow_id=None, validity=None): return { "skipped": False, "envelope": {"context": {"messageId": "msg-4"}}, diff --git a/tests/test_api.py b/tests/test_api.py index eb39239..c275db2 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -109,6 +109,120 @@ def test_get_document_not_found(self, test_client): assert response.status_code == 404 +class TestProdApprovalLifetime: + """The prod approver names the lifetime of the network announcement. + + The window is validated and stored before anything is promoted, so a + rejected window must leave the document exactly where it was. + """ + + def _document_at_the_gate(self, db_connection, workflow_id): + db_connection.upsert_document( + workflow_id=workflow_id, + document_id=f"doc-{workflow_id}", + filename="test.pdf", + filepath="/app/books/test.pdf", + stage="approval_for_prod", + ) + + @pytest.mark.api + @pytest.mark.unit + def test_stores_the_approvers_window(self, test_client, db_connection): + from pipeline.network_validity import today + + workflow_id = "prod-lifetime-001" + self._document_at_the_gate(db_connection, workflow_id) + + response = test_client.post( + f"/documents/{workflow_id}/approve-prod", + json={"network_valid_to": "2099-12-31"}, + ) + + assert response.status_code == 200 + assert response.json()["network_valid_from"] == today() + assert response.json()["network_valid_to"] == "2099-12-31" + stored = db_connection.get_document(workflow_id) + assert stored["network_valid_from"] == today() + assert stored["network_valid_to"] == "2099-12-31" + + @pytest.mark.api + @pytest.mark.unit + def test_a_body_less_approval_defaults_both_ends_to_today(self, test_client, db_connection): + from pipeline.network_validity import today + + workflow_id = "prod-lifetime-002" + self._document_at_the_gate(db_connection, workflow_id) + + response = test_client.post(f"/documents/{workflow_id}/approve-prod") + + assert response.status_code == 200 + stored = db_connection.get_document(workflow_id) + assert stored["network_valid_from"] == today() + assert stored["network_valid_to"] == today() + + @pytest.mark.api + @pytest.mark.unit + def test_the_window_is_surfaced_on_the_document(self, test_client, db_connection): + workflow_id = "prod-lifetime-003" + self._document_at_the_gate(db_connection, workflow_id) + test_client.post( + f"/documents/{workflow_id}/approve-prod", + json={"network_valid_to": "2099-12-31"}, + ) + + doc = test_client.get(f"/documents/{workflow_id}").json() + + assert doc["network_valid_to"] == "2099-12-31" + + @pytest.mark.api + @pytest.mark.unit + def test_rejects_an_end_before_the_start(self, test_client, db_connection): + workflow_id = "prod-lifetime-004" + self._document_at_the_gate(db_connection, workflow_id) + + response = test_client.post( + f"/documents/{workflow_id}/approve-prod", + json={"network_valid_from": "2026-09-16", "network_valid_to": "2026-09-15"}, + ) + + assert response.status_code == 400 + assert "cannot be before" in response.json()["detail"] + + @pytest.mark.api + @pytest.mark.unit + def test_rejects_a_malformed_date(self, test_client, db_connection): + workflow_id = "prod-lifetime-005" + self._document_at_the_gate(db_connection, workflow_id) + + response = test_client.post( + f"/documents/{workflow_id}/approve-prod", + json={"network_valid_to": "31-12-2099"}, + ) + + assert response.status_code == 400 + assert "YYYY-MM-DD" in response.json()["detail"] + + @pytest.mark.api + @pytest.mark.unit + def test_a_rejected_window_stores_nothing_and_promotes_nothing( + self, test_client, db_connection, mock_temporal_client + ): + workflow_id = "prod-lifetime-006" + self._document_at_the_gate(db_connection, workflow_id) + mock_temporal_client.start_workflow.reset_mock() + + test_client.post( + f"/documents/{workflow_id}/approve-prod", + json={"network_valid_to": "not-a-date"}, + ) + + stored = db_connection.get_document(workflow_id) + assert stored["network_valid_from"] is None + assert stored["network_valid_to"] is None + assert stored["stage"] == "approval_for_prod" + mock_temporal_client.start_workflow.assert_not_called() + + class TestPageEndpoints: """Tests for page operations.""" diff --git a/tests/test_catalog_builder.py b/tests/test_catalog_builder.py index 24fbfa4..577fab4 100644 --- a/tests/test_catalog_builder.py +++ b/tests/test_catalog_builder.py @@ -13,6 +13,7 @@ SCHEMES_CATALOG_ID, SCHEMES_RESOURCE_ID, ) +from pipeline.network_validity import ValidityWindow # Forbidden by the KnowledgeAdvisory schema under informationMode OnDemand. # Both advisory and scheme resources use this schema. @@ -131,6 +132,43 @@ def test_returns_none_so_the_caller_skips_publishing(self, kind): assert build_catalog(kind) is None +class TestValidityWindow: + @pytest.mark.unit + @pytest.mark.parametrize("kind", ["advisory", "scheme"]) + def test_omitted_when_the_document_has_no_window(self, kind): + # Documents promoted before approvers named a window: MERGE leaves an + # existing announcement's lifetime untouched rather than clearing it. + assert "validity" not in build_catalog(kind) + + @pytest.mark.unit + @pytest.mark.parametrize("kind", ["advisory", "scheme"]) + def test_lands_on_the_catalog_when_the_approver_set_one(self, kind): + catalog = build_catalog( + kind, ValidityWindow(start_date="2026-09-16", end_date="2026-12-31") + ) + + assert catalog["validity"] == { + "startDate": "2026-09-16T00:00:00Z", + "endDate": "2026-12-31T23:59:59Z", + } + + @pytest.mark.unit + def test_never_lands_on_resource_attributes(self): + # `validity` is absent from the OnDemand KnowledgeAdvisory profile + # (ADR 0003) — the catalog is the only legal home for it. + catalog = build_catalog( + "advisory", ValidityWindow(start_date="2026-09-16", end_date="2026-12-31") + ) + + assert "validity" not in catalog["resources"][0]["resourceAttributes"] + + @pytest.mark.unit + def test_an_unmapped_kind_still_publishes_nothing(self): + assert build_catalog( + "video", ValidityWindow(start_date="2026-09-16", end_date="2026-12-31") + ) is None + + class TestBuilderIsolation: @pytest.mark.unit def test_mutating_a_built_catalog_does_not_leak_into_the_next(self): diff --git a/tests/test_discovery_publish_service.py b/tests/test_discovery_publish_service.py index 30052a9..bd685cd 100644 --- a/tests/test_discovery_publish_service.py +++ b/tests/test_discovery_publish_service.py @@ -439,3 +439,62 @@ def test_skipped_publish_logs_no_request(self, monkeypatch, caplog): messages = [r.getMessage() for r in caplog.records] assert any("network_publish_skipped=True" in m for m in messages) assert not any("network_publish_url=" in m for m in messages) + + +class TestPublishedValidityWindow: + """The lifetime the prod approver set travels on the catalog it publishes.""" + + @pytest.mark.unit + def test_envelope_carries_the_approvers_window(self, monkeypatch): + from pipeline.network_validity import ValidityWindow + + service = _service(monkeypatch) + client = _client_returning( + _on_publish_response("cat-oan-knowledge-provider-advisories", "ACCEPTED") + ) + + with patch("pipeline.discovery_publish_service.httpx.Client", return_value=client): + result = service.publish( + transaction_id="txn-validity", + document_kind="advisory", + validity=ValidityWindow(start_date="2026-09-16", end_date="2026-12-31"), + ) + + catalog = client.post.call_args.kwargs["json"]["message"]["catalogs"][0] + assert catalog["validity"] == { + "startDate": "2026-09-16T00:00:00Z", + "endDate": "2026-12-31T23:59:59Z", + } + # The recorded envelope is what an operator inspects after the fact. + assert result["envelope"]["message"]["catalogs"][0]["validity"] == catalog["validity"] + + @pytest.mark.unit + def test_omits_validity_when_the_document_has_none(self, monkeypatch): + # Pre-existing behaviour for documents promoted before windows existed. + service = _service(monkeypatch) + client = _client_returning( + _on_publish_response("cat-oan-knowledge-provider-advisories", "ACCEPTED") + ) + + with patch("pipeline.discovery_publish_service.httpx.Client", return_value=client): + service.publish(transaction_id="txn-none", document_kind="advisory") + + catalog = client.post.call_args.kwargs["json"]["message"]["catalogs"][0] + assert "validity" not in catalog + + @pytest.mark.unit + def test_a_window_does_not_make_an_unmapped_kind_publishable(self, monkeypatch): + from pipeline.network_validity import ValidityWindow + + service = _service(monkeypatch) + client = _client_returning(MagicMock()) + + with patch("pipeline.discovery_publish_service.httpx.Client", return_value=client): + result = service.publish( + transaction_id="txn-skip", + document_kind="video", + validity=ValidityWindow(start_date="2026-09-16", end_date="2026-12-31"), + ) + + assert result["skipped"] is True + client.post.assert_not_called() diff --git a/tests/test_document_repository.py b/tests/test_document_repository.py index dbf6c4f..1752af8 100644 --- a/tests/test_document_repository.py +++ b/tests/test_document_repository.py @@ -8,6 +8,7 @@ import pytest from pipeline.document_repository import DocumentRepository +from pipeline.network_validity import ValidityWindow class FakeDb: @@ -56,6 +57,31 @@ def test_passes_the_workflow_id_through(self): assert fake.get_document_called_with == "wf-42" +class TestGetNetworkValidity: + @pytest.mark.unit + def test_rebuilds_the_stored_window(self): + repo = DocumentRepository( + FakeDb(document={"network_valid_from": "2026-09-16", "network_valid_to": "2026-12-31"}) + ) + + assert repo.get_network_validity("wf-1") == ValidityWindow( + start_date="2026-09-16", end_date="2026-12-31" + ) + + @pytest.mark.unit + def test_returns_none_for_a_missing_document(self): + assert DocumentRepository(FakeDb(document=None)).get_network_validity("wf-nope") is None + + @pytest.mark.unit + def test_returns_none_when_no_approver_set_a_window(self): + # Documents promoted before the approval gate collected one. + repo = DocumentRepository( + FakeDb(document={"network_valid_from": None, "network_valid_to": None}) + ) + + assert repo.get_network_validity("wf-1") is None + + class TestAgainstRealSqlite: """Guards against db.py drifting out from under the repository.""" @@ -76,3 +102,18 @@ def test_unclassified_document_reads_as_the_column_default(self, db_connection, @pytest.mark.db def test_missing_document_reads_as_none(self, db_connection): assert DocumentRepository().get_document_kind("no-such-workflow") is None + + @pytest.mark.unit + @pytest.mark.db + def test_reads_a_window_written_through_the_db_layer(self, db_connection, sample_document): + workflow_id = sample_document["workflow_id"] + db_connection.set_network_validity(workflow_id, "2026-09-16", "2026-12-31") + + assert DocumentRepository().get_network_validity(workflow_id) == ValidityWindow( + start_date="2026-09-16", end_date="2026-12-31" + ) + + @pytest.mark.unit + @pytest.mark.db + def test_an_unapproved_document_has_no_window(self, db_connection, sample_document): + assert DocumentRepository().get_network_validity(sample_document["workflow_id"]) is None diff --git a/tests/test_network_validity.py b/tests/test_network_validity.py new file mode 100644 index 0000000..b1f9cf4 --- /dev/null +++ b/tests/test_network_validity.py @@ -0,0 +1,118 @@ +"""Unit tests for the published catalog's validity window.""" + +from datetime import date, timedelta + +import pytest + +from pipeline.network_validity import ( + ValidityWindow, + ValidityWindowError, + default_window, + parse_window, + to_catalog_validity, + today, + window_from_row, +) + +TODAY = date.today().isoformat() +TOMORROW = (date.today() + timedelta(days=1)).isoformat() +YESTERDAY = (date.today() - timedelta(days=1)).isoformat() + + +class TestDefaultWindow: + @pytest.mark.unit + def test_both_ends_are_today(self): + # The approver is offered today/today and edits the end from there. + assert default_window() == ValidityWindow(start_date=TODAY, end_date=TODAY) + + @pytest.mark.unit + def test_today_is_the_stored_form(self): + assert today() == TODAY + + +class TestParseWindow: + @pytest.mark.unit + def test_defaults_both_ends_to_today(self): + assert parse_window() == ValidityWindow(start_date=TODAY, end_date=TODAY) + + @pytest.mark.unit + @pytest.mark.parametrize("blank", [None, "", " "]) + def test_defaults_a_blank_end_to_today(self, blank): + assert parse_window(TODAY, blank).end_date == TODAY + + @pytest.mark.unit + def test_keeps_an_approver_widened_end(self): + window = parse_window(TODAY, TOMORROW) + + assert window == ValidityWindow(start_date=TODAY, end_date=TOMORROW) + + @pytest.mark.unit + def test_same_day_window_is_legal(self): + # The default itself — an end equal to the start must not be rejected. + assert parse_window(TODAY, TODAY).end_date == TODAY + + @pytest.mark.unit + def test_trims_surrounding_whitespace(self): + assert parse_window(f" {TODAY} ", f" {TOMORROW} ").start_date == TODAY + + @pytest.mark.unit + def test_rejects_an_end_before_the_start(self): + with pytest.raises(ValidityWindowError, match="cannot be before"): + parse_window(TODAY, YESTERDAY) + + @pytest.mark.unit + @pytest.mark.parametrize( + "bad", + ["15-09-2026", "2026/09/15", "2026-13-01", "2026-02-30", "next tuesday", "2026-09-15T00:00:00Z"], + ) + def test_rejects_anything_that_is_not_a_calendar_date(self, bad): + with pytest.raises(ValidityWindowError, match="YYYY-MM-DD"): + parse_window(TODAY, bad) + + @pytest.mark.unit + def test_names_which_end_was_wrong(self): + with pytest.raises(ValidityWindowError, match="start date"): + parse_window("not-a-date", TODAY) + + +class TestWindowFromRow: + @pytest.mark.unit + def test_rebuilds_a_stored_window(self): + assert window_from_row(TODAY, TOMORROW) == ValidityWindow( + start_date=TODAY, end_date=TOMORROW + ) + + @pytest.mark.unit + @pytest.mark.parametrize( + "start,end", + [(None, None), (TODAY, None), (None, TOMORROW), ("", ""), (" ", TOMORROW)], + ) + def test_a_document_with_no_window_announces_without_one(self, start, end): + # Documents promoted before approvers named a window: not an error, and + # deliberately not defaulted to today, which would expire the catalog. + assert window_from_row(start, end) is None + + @pytest.mark.unit + def test_a_malformed_row_announces_without_a_window(self): + # Better an announcement with no validity than one the Discovery + # Service rejects the whole catalog over. + assert window_from_row("15-09-2026", TOMORROW) is None + assert window_from_row(TOMORROW, TODAY) is None + + +class TestToCatalogValidity: + @pytest.mark.unit + def test_widens_dates_to_the_instants_the_spec_examples_use(self): + validity = to_catalog_validity(ValidityWindow(start_date="2026-09-16", end_date="2026-12-31")) + + assert validity == { + "startDate": "2026-09-16T00:00:00Z", + "endDate": "2026-12-31T23:59:59Z", + } + + @pytest.mark.unit + def test_end_date_is_inclusive_to_the_last_second_of_that_day(self): + # A same-day window must still be a non-empty span. + validity = to_catalog_validity(ValidityWindow(start_date="2026-09-16", end_date="2026-09-16")) + + assert validity["startDate"] < validity["endDate"] diff --git a/ui/src/views/DocumentOpsView.jsx b/ui/src/views/DocumentOpsView.jsx index eec153d..4f8be6f 100644 --- a/ui/src/views/DocumentOpsView.jsx +++ b/ui/src/views/DocumentOpsView.jsx @@ -216,6 +216,62 @@ function DocumentClassificationPanel({ doc, workflowId, canClassify, onSaved }) ) } +/** Today in the `YYYY-MM-DD` form the API stores and uses. */ +function todayISODate() { + const now = new Date() + const month = String(now.getMonth() + 1).padStart(2, '0') + const day = String(now.getDate()).padStart(2, '0') + return `${now.getFullYear()}-${month}-${day}` +} + +/** + * Why a bad window is blocked rather than silently corrected: the approver is + * the only person who knows how long the announcement should stand, so a bad + * window is a question for them, not something to round into shape. Mirrors the + * server-side check in pipeline/network_validity.parse_window. + */ +function validateLifetime(startDate, endDate) { + if (!startDate || !endDate) return 'Set both a start and an end date.' + if (endDate < startDate) return 'End date cannot be before the start date.' + return '' +} + +/** + * Lifetime the super admin gives the network announcement this promotion leads + * to. Start date is the approval date and not editable - the announcement + * begins when it is published. End date opens at the same day and is the one + * thing the approver sets. + */ +function NetworkLifetimePanel({ startDate, endDate, onEndDateChange, error, disabled }) { + return ( +
+

+ Announcement lifetime - published to the discovery service +

+
+
+ Start date (approval date) + + {startDate} + +
+
+ End date + onEndDateChange(e.target.value)} + /> +
+
+ {error ?

{error}

: null} +
+ ) +} + // Which permission each mutating action requires. Approvals / edits are // review; anything that re-runs pipeline stages or touches the index is pipeline. const ACTION_PERMISSION = { @@ -328,6 +384,10 @@ export default function DocumentOpsView() { const [highlightedChunk, setHighlightedChunk] = useState(null) const [panelLoading, setPanelLoading] = useState({}) const [actionPending, setActionPending] = useState(null) + // Start date is fixed at the approval date; only the end date is the + // approver's to move. Both re-seed per document (see the reset effect below). + const [lifetimeStart, setLifetimeStart] = useState(todayISODate) + const [lifetimeEnd, setLifetimeEnd] = useState(todayISODate) const requestIdRef = useRef(0) const attemptedPanelsRef = useRef({}) const activeTabRef = useRef(activeTab) @@ -586,6 +646,10 @@ export default function DocumentOpsView() { setPanelErrors({}) setPanelLoading({}) setMessage('') + // Re-approving carries no memory of the previous document's window: the + // start date is always today, and the end date opens there again. + setLifetimeStart(todayISODate()) + setLifetimeEnd(todayISODate()) load({ soft: false }) }, [workflowId, load]) @@ -703,6 +767,18 @@ export default function DocumentOpsView() { return } else if (action === 'restore_document') { await fetchJson(`/documents/${workflowId}/restore`, { method: 'POST' }) + } else if (action === 'approve_prod') { + // The window travels with the approval itself: the API validates and + // stores it before it signals anything, so a rejected window leaves the + // document parked at the gate rather than promoted-but-unannounced. + await fetchJson(`/documents/${workflowId}/approve-prod`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + network_valid_from: lifetimeStart, + network_valid_to: lifetimeEnd, + }), + }) } else { await fetchJson(`/documents/${workflowId}/${action.replace(/_/g, '-')}`, { method: 'POST' }) } @@ -768,6 +844,9 @@ export default function DocumentOpsView() { (doc?.document_kind && doc.document_kind !== 'document') || doc?.scheme_code || doc?.scheme_name ) const ingestBlockedByClassification = doc?.stage === 'ready_for_ingestion' && !isDocClassified + const showsLifetimePanel = visibleActions.includes('approve_prod') + const lifetimeError = showsLifetimePanel ? validateLifetime(lifetimeStart, lifetimeEnd) : '' + const prodBlockedByLifetime = showsLifetimePanel && Boolean(lifetimeError) const sortedPages = useMemo(() => [...pages].sort((a, b) => a.page_number - b.page_number), [pages]) const reviewedPages = useMemo(() => pages.filter(p => p.is_reviewed).length, [pages]) const reviewedChunks = useMemo(() => chunks.filter(c => c.is_reviewed).length, [chunks]) @@ -927,21 +1006,30 @@ export default function DocumentOpsView() {
{visibleActions.slice(0, 4).map(action => { const blockedByClassification = action === 'approve_ingestion' && ingestBlockedByClassification + const blockedByLifetime = action === 'approve_prod' && prodBlockedByLifetime return ( ) })} @@ -969,6 +1057,16 @@ export default function DocumentOpsView() { /> )} + {showsLifetimePanel && ( + + )} + {message ? ( ) : null} From e2dcff1a1b4104b6e76da620e763ae7dcc139624 Mon Sep 17 00:00:00 2001 From: shruti0025 Date: Tue, 22 Sep 2026 09:50:53 +0530 Subject: [PATCH 02/11] feat: filter search results by document validity period [#111] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A chunk, once ingested, answered searches forever — a rabi sowing advisory is wrong advice in June. Every chunk now carries start_date/end_date in its Qdrant payload, defaulted on upload to the upload day plus one year and editable by the reviewer beside the document type. Search returns only chunks whose period covers today. Chunks ingested before validity existed carry neither date and stay searchable, so adding this does not empty the live index. The clock is injected, so an operator can ask what search would return on another day. --- AGENTS.md | 1 + CONTEXT.md | 4 + README.md | 2 +- .../0006-document-validity-filters-search.md | 25 ++ docs/openapi-search.yaml | 28 +++ pipeline/activities.py | 39 ++- pipeline/api.py | 26 +- pipeline/db.py | 55 +++- pipeline/document_repository.py | 15 ++ pipeline/document_validity.py | 162 ++++++++++++ pipeline/models.py | 17 +- pipeline/scheme_catalog.py | 24 +- pipeline/vector_store/base.py | 4 + pipeline/vector_store/qdrant_store.py | 153 +++++++++-- scripts/verify_document_validity_e2e.py | 205 +++++++++++++++ tests/test_activities.py | 104 ++++++++ tests/test_api.py | 237 ++++++++++++++++++ tests/test_document_repository.py | 58 +++++ tests/test_document_validity.py | 189 ++++++++++++++ tests/test_qdrant_validity.py | 229 +++++++++++++++++ ui/src/views/DocumentOpsView.jsx | 82 +++++- 21 files changed, 1624 insertions(+), 35 deletions(-) create mode 100644 docs/ADR/0006-document-validity-filters-search.md create mode 100644 pipeline/document_validity.py create mode 100644 scripts/verify_document_validity_e2e.py create mode 100644 tests/test_document_validity.py create mode 100644 tests/test_qdrant_validity.py diff --git a/AGENTS.md b/AGENTS.md index 8698c2c..e65ff2e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,6 +62,7 @@ Core modules (all under `pipeline/`): - `document_repository.py` — `DocumentRepository`, a domain layer over `db.py` for document reads; `db.py` stays one-function-per-query with no domain knowledge - `network_constants.py` — fixed values sent on the network (catalog/resource ids, topics, languages, Beckn version). The file v2 edits when the AI layer starts deriving them - `catalog_builder.py` — pure builder mapping a document's knowledge kind (`advisory`/`scheme`) to the single `OnDemand` Beckn catalog announced for that kind; returns `None` for any other kind. No env, no I/O +- `document_validity.py` — pure module owning Document Validity: the period a document's chunks answer searches in, the upload-day/+1-year default, and the clock injection point. Shared by `db.py` (stamping on upload), `scheme_catalog.py` (validating a reviewer's edit), `activities.py` (stamping chunk payloads) and `vector_store/qdrant_store.py` (filtering at search time). Not the same window as `network_validity.py` — see CONTEXT.md - `network_validity.py` — pure module owning the Announcement Lifetime: what a legal `validity` window is, the today/today default an approver starts from, and how it renders on the wire. Shared by `api.py` (validating approver input) and `catalog_builder.py` (placing it on the catalog) - `discovery_publish_service.py` — `DiscoveryPublishService`: owns the Publish to Network env vars and the HTTP call. Deliberately Temporal-free - `models.py` — Pydantic models, including `DocumentStage` enum and `PIPELINE_STAGES` (the stepper-UI stage list) diff --git a/CONTEXT.md b/CONTEXT.md index f667d68..597a2b8 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -32,5 +32,9 @@ _Avoid_: "document type" — collides with `source_type`/`canonical_input_type`, The `validity` window (`startDate`/`endDate`) carried on the Network Catalog Envelope, named by the prod approver at `approve-prod` and stored as `documents.network_valid_from` / `network_valid_to`. Start is the approval date; end is prepopulated with the same day and is the approver's to move. Because the envelope is kind-level, the most recently published window is the live one for that whole Knowledge Kind — see `docs/ADR/0005-approver-sets-the-network-catalogs-validity-window.md`. _Avoid_: "document lifetime" — the document itself is not what expires, the announcement is. Also avoid "expiry": there is a start as well as an end. +**Document Validity**: +The period a document's chunks answer searches in, held as `documents.valid_from` / `documents.valid_to` and stamped onto every chunk's vector payload as `start_date` / `end_date`. Defaulted on upload to the upload day plus one year, then confirmed or moved by the reviewer in the same form that sets the Knowledge Kind. Both ends are inclusive, and search filters on it so an expired or not-yet-started document is never answered from. Chunks ingested before it existed carry neither date and stay searchable — see `docs/ADR/0006-document-validity-filters-search.md`. +_Avoid_: "validity" alone — ambiguous with the Announcement Lifetime, which is a different window (kind-level, `startDate`/`endDate` on the wire, named by the prod approver). This one is document-level, never leaves the vector store, and expires a document's answers rather than an announcement. + **network_visible**: An operator-controlled flag on a document/scheme (not a pipeline stage) that gates whether it's exposed to other BAPs through the pull-based Scheme Catalog snapshot. Independent of Publish to Network. diff --git a/README.md b/README.md index 9a3beb7..daa705f 100644 --- a/README.md +++ b/README.md @@ -516,7 +516,7 @@ Translation data is surfaced through the page model and review endpoints. - `GET /documents/{workflow_id}/qdrant` - `GET /documents/{workflow_id}/qdrant/chunks` - `POST /documents/{workflow_id}/reingest` -- `POST /search` +- `POST /search` — only answers from documents whose validity period covers today; `valid_on` asks about another day and `include_expired` drops the filter (see `docs/openapi-search.yaml`) - `GET /indexes/summary` - `GET /indexes/{index_name}/settings` - `GET /indexes/{index_name}/stats` diff --git a/docs/ADR/0006-document-validity-filters-search.md b/docs/ADR/0006-document-validity-filters-search.md new file mode 100644 index 0000000..54604b3 --- /dev/null +++ b/docs/ADR/0006-document-validity-filters-search.md @@ -0,0 +1,25 @@ +# Document validity filters search + +Context: a chunk, once ingested, answered searches forever. Agricultural content does not work that way — a rabi sowing advisory is wrong advice in June, and a scheme circular superseded last year is worse than no answer. Nothing in the pipeline could express "this stopped being true", so the only way to retire content was to disable the whole document, losing it from the operator console too. ADR 0005 added a window at the prod gate, but that one is the *Announcement Lifetime*: kind-level, on the wire, about whether this provider still claims to serve a knowledge kind. It says nothing about whether any given document's content is current. + +Decision: +- **A separate, document-level concept.** `documents.valid_from` / `documents.valid_to`, stamped onto every chunk's Qdrant payload as `start_date` / `end_date`, owned by a new pure module `pipeline/document_validity.py`. Deliberately not folded into `network_validity.py`: the two windows have different defaults, different authors, different moments, and different destinations, and CONTEXT.md already warns against conflating them. They share a date format and nothing else. +- **Defaulted on upload, not asked for.** `db.upsert_document` stamps the upload day and one year out on INSERT. An uploader is not asked a question they cannot answer, and no document can reach the index without a period. A year is the business's review cadence, not a technical limit. +- **Confirmed or moved by the reviewer, in the form that already exists.** The dates sit beside the Knowledge Kind selector and travel on the same `PATCH /documents/{id}/scheme-metadata`. Choosing the period is part of classifying a document, not a separate errand, and the reviewer sees real prefilled dates rather than an empty field. Either end may be sent alone; the other is taken from what is stored, so moving the end date does not require restating the start. +- **Validated like a bad kind, not like a bad type.** `document_validity.parse_period` raises, `apply_scheme_metadata` lets it through as a `ValueError`, and the endpoint answers 400 — the same shape a bad `document_kind` gets, not a Pydantic 422. A rejected period stores nothing, kind included. +- **Both ends inclusive.** A document uploaded this morning starts today and must answer this afternoon; an end date names the last day it answers rather than the first day it does not. The requirement was written as "today greater than start and less than end", but exclusive bounds would hide a document on its own first day, which is the default every document gets. +- **Search filters on it by default**, as `(start_date missing OR start_date <= today) AND (end_date missing OR end_date >= today)`. Per-end rather than one window clause, so a partially-dated point behaves sensibly instead of vanishing. +- **Undated chunks stay searchable.** The missing-field branches are the whole reason the rule is shaped this way: every point already in Qdrant carries no dates, and a change that silently emptied the live index would rightly be called a regression. Such a document picks up a period the next time it is ingested — anchored on its own upload day, not on today, so reingesting a two-year-old document does not quietly extend its life by another year. +- **The clock is injected.** `QdrantVectorStore(clock=...)` decides what "today" means, defaulting to the real clock. Tests pin a day instead of writing fixtures relative to whenever they run, and `POST /search` exposes `valid_on` (answer as of another day) and `include_expired` (drop the filter) so an operator can inspect what an expired document still holds. Both default to the safe behaviour: a caller that asks for nothing gets today's valid documents only. +- **Document-scoped reads ignore validity.** `list_by_doc_id`, `delete_by_doc_id` and `delete_chunk` pass no date: an expired chunk is still that document's chunk, and a purge that skipped expired chunks would orphan them in the index. +- **Indexed as Qdrant `DATETIME`**, created idempotently on every `ensure_collection` including one that already exists — a collection created before the field existed has no index for it, and ingest is the one path that reliably runs against every live collection. The filter was verified to work unindexed too, so a collection still being backfilled filters correctly, just more slowly. + +Known limitation, accepted: expiry is enforced at **read** time, not by a sweeper. An expired document's vectors stay in Qdrant, still costing storage and still visible to anything that queries Qdrant directly without this filter (including `include_expired`). That is the right trade for now — deleting on expiry would make un-expiring a document a reingest — but a caller bypassing `/search` is not protected. + +Alternatives considered: +- **Reuse `network_valid_from`/`network_valid_to`.** Rejected: ADR 0005 gives those a today/today default and a kind-level meaning on the wire. Reusing them would either expire every document the day it is approved or corrupt the announcement window, and would collapse two concepts CONTEXT.md keeps apart. +- **Collect the dates at the prod gate with the Announcement Lifetime.** Rejected: DEV search is filtered too, and the prod gate is reached long after ingestion — a document would be searchable with no period for most of its life. +- **Store epoch integers instead of `YYYY-MM-DD` strings.** Rejected: Qdrant's `DatetimeRange` accepts date-only strings (verified against a live instance), and an operator reading a chunk payload can see `2027-03-31` rather than decoding a number. +- **Filter after retrieval, in the API.** Rejected: it silently shrinks the candidate set below `top_k`, so an index full of expired documents would return a short page of results rather than the valid ones further down. +- **Ask the uploader for the dates.** Rejected: the uploader is often not the person who knows how long content holds, and blocking upload on it would slow the common case for a value the reviewer sets anyway. +- **Default to no end date (never expires).** Rejected: it makes expiry opt-in, so content would go stale by default — the same failure this ADR exists to fix. diff --git a/docs/openapi-search.yaml b/docs/openapi-search.yaml index cb8a8b5..39e4408 100644 --- a/docs/openapi-search.yaml +++ b/docs/openapi-search.yaml @@ -237,6 +237,22 @@ components: type: boolean description: If true, also return the pre-cap/pre-rerank candidate list as raw_hits. default: false + include_expired: + type: boolean + description: | + Drop the document-validity filter, returning chunks whose period + has ended or not yet started. An operator tool for inspecting what + an expired document still holds — not for a caller serving an end + user, who should never be answered from an expired document. + default: false + valid_on: + type: string + format: date + description: | + Answer as of this calendar day (`YYYY-MM-DD`) instead of today — + what search would have returned, or will return, on that date. A + malformed value returns 400. Ignored when include_expired is true. + default: "(today)" SearchHit: type: object @@ -296,6 +312,18 @@ components: chunk_number: type: integer description: Alias of chunk_num, set when chunk_num is present. + start_date: + type: string + format: date + description: | + First day this chunk's document answers searches. Absent on chunks + ingested before validity existed, which are treated as current. + end_date: + type: string + format: date + description: | + Last day this chunk's document answers searches (inclusive). + Absent on chunks ingested before validity existed. section: type: string token_count: diff --git a/pipeline/activities.py b/pipeline/activities.py index 16412c8..ea282dd 100644 --- a/pipeline/activities.py +++ b/pipeline/activities.py @@ -23,7 +23,7 @@ from temporalio import activity from temporalio.exceptions import ApplicationError -from . import scheme_catalog +from . import document_validity, scheme_catalog from .catalog_builder import normalize_document_kind from .chunking import chunk_pages, load_chunking_config from .discovery_publish_service import DiscoveryPublishService @@ -641,6 +641,8 @@ def _prepare_records( scheme_code: str | None = None, scheme_name: str | None = None, scheme_aliases: list[str] | None = None, + valid_from: str | None = None, + valid_to: str | None = None, ) -> list[dict]: metadata = _get_doc_metadata(filename) resolved_instance = _normalize_instance(instance) @@ -672,6 +674,10 @@ def _prepare_records( else [] ) instance_name = instance_display_name(resolved_instance) + # One period for the whole document, resolved before the loop: every chunk + # of a document expires together, and re-deriving it per chunk would let a + # midnight boundary split one document across two periods. + validity = document_validity.period_from_row(valid_from, valid_to) records = [] for chunk in chunks: @@ -716,6 +722,9 @@ def _prepare_records( "quality_score": float(quality_score) if str(quality_score).strip().replace(".", "", 1).isdigit() else 0.0, "priority_rank": float(priority_rank) if str(priority_rank).strip().replace(".", "", 1).isdigit() else 0.0, } + if validity is not None: + record["start_date"] = validity.start_date + record["end_date"] = validity.end_date if is_scheme: record["scheme_code"] = (scheme_code or "").strip().lower() record["scheme_name"] = resolved_scheme_name @@ -751,6 +760,27 @@ def _scheme_fields_from_doc(doc: dict | None) -> dict: } +def _validity_fields_from_doc(doc: dict | None) -> dict: + """Extract the validity kwargs for `_prepare_records` from a documents row. + + A row with no stored period is stamped with the default anchored on its + upload day, so every chunk written from here on carries validity. Old + points already in the index keep none until their document is reingested, + which is what keeps them searchable in the meantime. + """ + doc = doc or {} + period = document_validity.period_from_row( + doc.get("valid_from"), doc.get("valid_to") + ) + if period is None: + upload_day = str(doc.get("created_at") or "")[:10] + try: + period = document_validity.period_from_upload_date(upload_day) + except document_validity.DocumentValidityError: + period = document_validity.default_period() + return {"valid_from": period.start_date, "valid_to": period.end_date} + + def prepare_ingestion_records( document_id: str, filename: str, @@ -810,6 +840,10 @@ def _passage_schema_definition(use_tensor_prefix_field: bool = True) -> dict: {"name": "priority_rank", "type": "float", "features": ["filter"]}, {"name": "text", "type": "text", "features": ["lexical_search"]}, {"name": "priority", "type": "float", "features": ["score_modifier", "filter"]}, + # Document validity - filtered on at search time, so the day a chunk + # starts and stops answering travels with the chunk itself. + {"name": "start_date", "type": "date", "features": ["filter"]}, + {"name": "end_date", "type": "date", "features": ["filter"]}, ] if use_tensor_prefix_field: all_fields.append({"name": "text_for_embedding", "type": "text"}) @@ -1196,6 +1230,7 @@ async def prepare_for_ingestion( name_en=name_en, description=description, instance=(doc or {}).get("instance"), + **_validity_fields_from_doc(doc), ) activity.logger.info(f"Prepared {len(records)} records") return records @@ -1305,6 +1340,7 @@ async def promote_document_to_prod_qdrant( workflow_id=workflow_id, instance=doc.get("instance"), **scheme_kwargs, + **_validity_fields_from_doc(doc), ) activity.logger.info( "Promoting %s records to PROD Qdrant collection %s (kind=%s)", @@ -1373,6 +1409,7 @@ async def ingest_document_from_db( workflow_id=workflow_id, instance=doc.get("instance"), **_scheme_fields_from_doc(doc), + **_validity_fields_from_doc(doc), ) payload_path = _write_json_temp(records) try: diff --git a/pipeline/api.py b/pipeline/api.py index 3c61a7c..b1ed6f3 100644 --- a/pipeline/api.py +++ b/pipeline/api.py @@ -42,7 +42,7 @@ from slowapi.util import get_remote_address from temporalio.client import Client, WorkflowFailureError -from . import db, scheme_catalog +from . import db, document_validity, scheme_catalog from .auth.config import load_auth_config, validate_auth_config from .auth.deps import ( CurrentUser, @@ -458,6 +458,8 @@ def _document_summary_from_row(doc: dict, current_job: Optional[dict] = None) -> prod_ready_requested_by_username=doc.get("prod_ready_requested_by_username"), network_valid_from=doc.get("network_valid_from"), network_valid_to=doc.get("network_valid_to"), + valid_from=doc.get("valid_from"), + valid_to=doc.get("valid_to"), ) @@ -1539,6 +1541,8 @@ def _build_document_detail(doc: dict) -> DocumentDetail: prod_ready_requested_by_username=doc.get("prod_ready_requested_by_username"), network_valid_from=doc.get("network_valid_from"), network_valid_to=doc.get("network_valid_to"), + valid_from=doc.get("valid_from"), + valid_to=doc.get("valid_to"), ) @@ -2147,6 +2151,8 @@ async def patch_scheme_metadata( tool_routing=body.tool_routing, catalog_visible=body.catalog_visible, network_visible=body.network_visible, + valid_from=body.valid_from, + valid_to=body.valid_to, ) except LookupError as exc: raise HTTPException(404, str(exc)) from exc @@ -2174,6 +2180,8 @@ async def patch_scheme_metadata( "tool_routing": doc.get("tool_routing"), "catalog_visible": bool(int(doc["catalog_visible"])) if doc.get("catalog_visible") is not None else True, "network_visible": bool(int(doc["network_visible"])) if doc.get("network_visible") is not None else True, + "valid_from": doc.get("valid_from"), + "valid_to": doc.get("valid_to"), "catalog_version": result.get("catalog_version"), "requires_reindex": result.get("requires_reindex"), "pending_reindex": result.get("pending_reindex"), @@ -3941,6 +3949,18 @@ async def run_search(payload: dict, user: RequireSearch): query_expansion_profile = payload.get("query_expansion_profile") or settings.get("queryExpansionProfile") or "gu-v1" rerank_mode = payload.get("rerank_mode") or settings.get("rerankMode") or "none" hybrid_rrf_k = int(payload.get("hybrid_rrf_k") or settings.get("hybridRrfK") or 60) + # Validity is on by default: a search serving a farmer must not answer from + # a document that has expired or has not started. Both overrides are + # operator tools - `include_expired` to inspect what an expired document + # still holds, `valid_on` to ask what search would have returned on another + # day - and the response echoes which day was actually applied. + include_expired = bool(payload.get("include_expired", False)) + valid_on = (payload.get("valid_on") or "").strip() or None + if valid_on: + try: + document_validity.parse_period(valid_on, valid_on) + except document_validity.DocumentValidityError as exc: + raise HTTPException(400, f"Invalid valid_on: {exc}") from None expanded_query = _expand_query(query, query_expansion_profile) # Qdrant embeddings apply E5 prefixes internally; don't pre-prefix here. search_query = expanded_query @@ -3960,6 +3980,8 @@ async def run_search(payload: dict, user: RequireSearch): use_e5_prefix=use_e5_prefix, hybrid_alpha=alpha, ef_search=ef_search, + apply_validity=not include_expired, + valid_on=valid_on, ) except Exception as error: raise HTTPException(400, f"Vector search failed ({backend}): {error}") from error @@ -4006,6 +4028,8 @@ async def run_search(payload: dict, user: RequireSearch): "query_expansion_profile": query_expansion_profile, "query_expansion_applied": expanded_query != query, "rerank_mode": rerank_mode, + "apply_validity": not include_expired, + "valid_on": result.get("valid_on"), "filter_string": None, }, "candidate_count": len(hits), diff --git a/pipeline/db.py b/pipeline/db.py index f838815..7385479 100644 --- a/pipeline/db.py +++ b/pipeline/db.py @@ -15,6 +15,8 @@ from threading import Lock from typing import Optional +from . import document_validity + # Database path - can be configured via environment DB_PATH = os.environ.get("DOCUMENT_DB_PATH", "/data/documents.db") @@ -122,6 +124,12 @@ def init_db(): # NULL on documents promoted before approvers named one. _add_column_if_missing(conn, "documents", "network_valid_from", "TEXT") _add_column_if_missing(conn, "documents", "network_valid_to", "TEXT") + # Period this document's chunks are searchable in, stamped on + # upload and editable by the approver. NULL on documents uploaded + # before validity existed - search reads that as always current + # (see pipeline/document_validity.period_from_row). + _add_column_if_missing(conn, "documents", "valid_from", "TEXT") + _add_column_if_missing(conn, "documents", "valid_to", "TEXT") # Stamp NULL/empty rows with the configured default so list filters # (which coalesce to DEFAULT_INSTANCE) match migrated data. default_instance = ( @@ -673,15 +681,25 @@ def upsert_document( uploaded_by_username: Optional[str] = None, uploaded_by_email: Optional[str] = None, uploaded_by_roles: Optional[str] = None, + valid_from: Optional[str] = None, + valid_to: Optional[str] = None, ): """Insert or update a document record. ``instance`` is applied on INSERT only. Updates leave the existing tenant stamp alone so a re-upload / restart cannot silently reassign tenants. Uploader identity is set on INSERT, and filled on UPDATE only when currently null. + + ``valid_from``/``valid_to`` are the document's validity period, likewise + INSERT-only: an uploader does not choose it (it defaults to the upload day + and a year out) and an approver's later edit must survive a restart that + re-registers the same workflow. """ now = datetime.utcnow().isoformat() instance_value = (instance or os.environ.get("DEFAULT_INSTANCE") or "default").strip().lower() or "default" + default_period = document_validity.default_period() + valid_from_value = (valid_from or "").strip() or default_period.start_date + valid_to_value = (valid_to or "").strip() or default_period.end_date with _db_lock: with get_connection() as conn: @@ -748,8 +766,9 @@ def upsert_document( reindex_required, reindex_reason, original_artifact_id, normalized_artifact_id, latest_job_id, instance, - uploaded_by_user_id, uploaded_by_username, uploaded_by_email, uploaded_by_roles - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + uploaded_by_user_id, uploaded_by_username, uploaded_by_email, uploaded_by_roles, + valid_from, valid_to + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) """, ( workflow_id, document_id, filename, filepath, canonical_document_id, display_name, source_filename, source_manifest_name, @@ -762,6 +781,7 @@ def upsert_document( original_artifact_id, normalized_artifact_id, latest_job_id, instance_value, uploaded_by_user_id, uploaded_by_username, uploaded_by_email, uploaded_by_roles, + valid_from_value, valid_to_value, )) conn.commit() @@ -1370,6 +1390,8 @@ def update_document_fields(workflow_id: str, **updates: object) -> Optional[dict "document_kind", "scheme_code", "scheme_name", "scheme_aliases_json", "tool_routing", "catalog_visible", "network_visible", "prod_ready_requested_at", "prod_ready_requested_by_user_id", "prod_ready_requested_by_username", + # Document validity period (searchable-from / searchable-to) + "valid_from", "valid_to", } set_clauses = [] values: list[object] = [] @@ -1729,6 +1751,35 @@ def set_network_validity( return get_document(workflow_id) +def set_document_validity( + workflow_id: str, + valid_from: str, + valid_to: str, +) -> Optional[dict]: + """Record the period this document's chunks are searchable in. Both ends + are `YYYY-MM-DD`; validation belongs to the caller + (`pipeline/document_validity.parse_period`), not to this SQL layer.""" + with _db_lock: + with get_connection() as conn: + conn.execute( + """ + UPDATE documents + SET valid_from = ?, + valid_to = ?, + updated_at = ? + WHERE workflow_id = ? + """, + ( + valid_from, + valid_to, + datetime.utcnow().isoformat(), + workflow_id, + ), + ) + conn.commit() + return get_document(workflow_id) + + def clear_prod_ready_requested(workflow_id: str) -> Optional[dict]: """Reset the request flag once prod approval actually happens (or the document leaves the approval_for_prod gate for any other reason).""" diff --git a/pipeline/document_repository.py b/pipeline/document_repository.py index ab27aee..1acbd39 100644 --- a/pipeline/document_repository.py +++ b/pipeline/document_repository.py @@ -9,6 +9,8 @@ from typing import Optional from . import db +from .document_validity import ValidityPeriod +from .document_validity import period_from_row as validity_period_from_row from .network_validity import ValidityWindow, window_from_row @@ -43,3 +45,16 @@ def get_network_validity(self, workflow_id: str) -> Optional[ValidityWindow]: return window_from_row( doc.get("network_valid_from"), doc.get("network_valid_to") ) + + def get_validity(self, workflow_id: str) -> Optional[ValidityPeriod]: + """The period this document's chunks are searchable in, or None when + it has none. + + None is not an error: documents uploaded before validity existed have + no stored dates, and their chunks are searchable without restriction + (see `document_validity.period_from_row`). + """ + doc = self._db.get_document(workflow_id) + if not doc: + return None + return validity_period_from_row(doc.get("valid_from"), doc.get("valid_to")) diff --git a/pipeline/document_validity.py b/pipeline/document_validity.py new file mode 100644 index 0000000..e555fcd --- /dev/null +++ b/pipeline/document_validity.py @@ -0,0 +1,162 @@ +""" +The validity period a document's chunks are searchable in. + +A document is only worth answering from for as long as its content still +holds. This module owns that rule: what a legal period is, the period an +uploader's document starts life with, and whether a period is live on a given +day. Pure - no env, no I/O, no db - so the API can validate an approver's edit, +the ingest activity can stamp the same period onto every chunk, and the vector +store can build a search filter from it without any of them re-deriving it. + +Deliberately separate from `pipeline/network_validity.py`, which owns the +Announcement Lifetime: a different window, with different defaults (today / +today, not today / a year out), named by a different person at a different +moment, and carried on the wire rather than in a chunk payload. They share a +date format and nothing else; see `CONTEXT.md` on not conflating the two. + +`pipeline/activities.py` stamps the period onto chunk payloads as +`start_date` / `end_date`; `pipeline/vector_store/qdrant_store.py` filters on +those fields at search time; `pipeline/api.py` validates approver input here. +""" + +from dataclasses import dataclass +from datetime import date, datetime +from typing import Callable, Optional + +# Operators think in calendar dates, not instants - a period is stored, +# exchanged and filtered as a plain `YYYY-MM-DD` day. +DATE_FORMAT = "%Y-%m-%d" + +# How long a freshly uploaded document is presumed to stay current. A year is +# the business's default review cadence, not a technical limit: the approver is +# shown it prefilled and is free to move either end. +DEFAULT_VALIDITY_YEARS = 1 + +# Returns "now". Injected wherever the answer depends on the current day, so a +# test can pin the day instead of building fixtures relative to the real one. +Clock = Callable[[], datetime] + + +class DocumentValidityError(ValueError): + """An operator-supplied period that cannot be stored.""" + + +def system_clock() -> datetime: + """The real clock. Local, matching `network_validity.today()`'s + `date.today()`, so both windows agree on what day it is.""" + return datetime.now() + + +@dataclass(frozen=True) +class ValidityPeriod: + """Inclusive calendar span, both ends `YYYY-MM-DD`. + + Inclusive on purpose: a document uploaded today defaults to a period + starting today and must be searchable the same day, and an end date names + the last day it answers rather than the first day it does not. + """ + + start_date: str + end_date: str + + def is_active_on(self, on_date: str) -> bool: + """Whether this period covers `on_date` (`YYYY-MM-DD`).""" + return self.start_date <= on_date <= self.end_date + + +def today(clock: Optional[Clock] = None) -> str: + """Today's date in the stored form.""" + return (clock or system_clock)().date().strftime(DATE_FORMAT) + + +def add_years(stamp: str, years: int = DEFAULT_VALIDITY_YEARS) -> str: + """`stamp` moved forward by whole calendar years. + + 29 February lands on 28 February in a non-leap year - the alternative + (1 March) would push the period into the wrong month for no gain. + """ + anchor = _parse_date(stamp, "date") + try: + moved = anchor.replace(year=anchor.year + years) + except ValueError: + moved = anchor.replace(year=anchor.year + years, day=28) + return moved.strftime(DATE_FORMAT) + + +def default_period(clock: Optional[Clock] = None) -> ValidityPeriod: + """The period a document gets on upload, before anyone edits it. + + Starts the day it is uploaded and runs a year out, so a document is live + the moment it is ingested and expires without anyone having to remember to + expire it. + """ + return period_from_upload_date(today(clock)) + + +def period_from_upload_date(upload_date: str) -> ValidityPeriod: + """The default period anchored on the day a document was uploaded. + + Used when stamping a document that predates the stored columns: its own + upload day is a truer start than today, which would silently extend a + two-year-old document's life by another year. + """ + stamp = _parse_date(upload_date, "upload date").strftime(DATE_FORMAT) + return ValidityPeriod(start_date=stamp, end_date=add_years(stamp)) + + +def _parse_date(value: Optional[str], field: str) -> date: + try: + return datetime.strptime((value or "").strip(), DATE_FORMAT).date() + except (AttributeError, ValueError): + raise DocumentValidityError( + f"{field} must be a calendar date formatted as YYYY-MM-DD, got {value!r}." + ) from None + + +def parse_period( + start_date: Optional[str] = None, + end_date: Optional[str] = None, + clock: Optional[Clock] = None, +) -> ValidityPeriod: + """Validate an operator-supplied period, defaulting either end. + + An omitted start defaults to today and an omitted end to a year past the + resolved start, so a caller that sends only one end still gets a complete + period. Raises `DocumentValidityError` - never returns a half-valid + period, so a caller holding one can store it unchecked. + """ + raw_start = start_date if (start_date or "").strip() else today(clock) + start = _parse_date(raw_start, "start date") + stamp = start.strftime(DATE_FORMAT) + + raw_end = end_date if (end_date or "").strip() else add_years(stamp) + end = _parse_date(raw_end, "end date") + + if end < start: + raise DocumentValidityError( + f"end date ({end.strftime(DATE_FORMAT)}) cannot be before " + f"start date ({stamp})." + ) + + return ValidityPeriod(start_date=stamp, end_date=end.strftime(DATE_FORMAT)) + + +def period_from_row( + start_date: Optional[str], + end_date: Optional[str], +) -> Optional[ValidityPeriod]: + """Rebuild a stored period, or None when the document has none. + + None is the normal case for a document uploaded before validity existed. + It means "this document has no stated period", which search reads as + always current - not an error, and not a silent fallback to today, which + would retire every legacy document at once. + """ + if not (start_date or "").strip() or not (end_date or "").strip(): + return None + try: + return parse_period(start_date, end_date) + except DocumentValidityError: + # A row this module never wrote. Treating it as no period beats + # filtering on a date the store cannot compare. + return None diff --git a/pipeline/models.py b/pipeline/models.py index 77d93e1..b84aff4 100644 --- a/pipeline/models.py +++ b/pipeline/models.py @@ -216,6 +216,11 @@ class DocumentSummary(BaseModel): # Lifetime of the network announcement, set by the prod approver. network_valid_from: Optional[str] = None network_valid_to: Optional[str] = None + # Period this document's chunks are searchable in. Stamped on upload and + # editable alongside the document type; NULL on documents uploaded before + # validity existed. + valid_from: Optional[str] = None + valid_to: Optional[str] = None class ProdApprovalRequest(BaseModel): @@ -233,7 +238,15 @@ class ProdApprovalRequest(BaseModel): class SchemeMetadataUpdate(BaseModel): - """PATCH /documents/{id}/scheme-metadata body.""" + """PATCH /documents/{id}/scheme-metadata body. + + `valid_from`/`valid_to` are `YYYY-MM-DD` and travel with the document type + because that is the one form a reviewer fills in before ingestion - the + period is part of classifying a document, not a separate errand. Omitting + both leaves the stored period alone; the dates are validated in + `document_validity.parse_period` so a bad period comes back as the same + 400 a bad kind does, not a Pydantic 422. + """ document_kind: Optional[str] = None # document | scheme scheme_code: Optional[str] = None scheme_name: Optional[str] = None @@ -241,6 +254,8 @@ class SchemeMetadataUpdate(BaseModel): tool_routing: Optional[str] = None # qdrant | legacy | both catalog_visible: Optional[bool] = None network_visible: Optional[bool] = None + valid_from: Optional[str] = None + valid_to: Optional[str] = None class DocumentArtifact(BaseModel): diff --git a/pipeline/scheme_catalog.py b/pipeline/scheme_catalog.py index 86fab29..f99e6db 100644 --- a/pipeline/scheme_catalog.py +++ b/pipeline/scheme_catalog.py @@ -14,7 +14,7 @@ from datetime import datetime, timezone from typing import Any, Optional -from . import db +from . import db, document_validity SCHEME_CODE_RE = re.compile(r"^[a-z0-9]+(?:-[a-z0-9]+)*$") @@ -754,8 +754,17 @@ def apply_scheme_metadata( tool_routing: Optional[str] = None, catalog_visible: Optional[bool] = None, network_visible: Optional[bool] = None, + valid_from: Optional[str] = None, + valid_to: Optional[str] = None, ) -> dict[str, Any]: - """Update document scheme fields and rebuild catalog when needed.""" + """Update document scheme fields and rebuild catalog when needed. + + `valid_from`/`valid_to` set the document's validity period (see + `pipeline/document_validity.py`). Either end alone is enough - the other is + taken from what is stored, or from the period's own default when the + document has none - so a reviewer who only moves the end date does not have + to restate the start. + """ ensure_catalog_schema() doc = db.get_document(workflow_id) if not doc: @@ -817,6 +826,17 @@ def apply_scheme_metadata( elif kind == "scheme" and doc.get("network_visible") is None: updates["network_visible"] = 1 if default_network_visible(doc.get("instance")) else 0 + if valid_from is not None or valid_to is not None: + # Validated here rather than at the SQL layer, and against the stored + # period rather than today, so a reviewer editing one end keeps the + # other exactly as it was. + period = document_validity.parse_period( + valid_from if valid_from is not None else doc.get("valid_from"), + valid_to if valid_to is not None else doc.get("valid_to"), + ) + updates["valid_from"] = period.start_date + updates["valid_to"] = period.end_date + if kind == "scheme" and not (new_code or updates.get("scheme_code") or old_code): # Allow setting kind first without code only if not completing as scheme pass diff --git a/pipeline/vector_store/base.py b/pipeline/vector_store/base.py index 419b0a9..9aaac03 100644 --- a/pipeline/vector_store/base.py +++ b/pipeline/vector_store/base.py @@ -53,5 +53,9 @@ def search( hybrid_alpha: float = 0.6, ef_search: int = 256, attributes_to_retrieve: Optional[list[str]] = None, + apply_validity: bool = True, + valid_on: Optional[str] = None, ) -> dict[str, Any]: + """`apply_validity` keeps only chunks whose validity period covers + `valid_on` (default: the store's today).""" ... diff --git a/pipeline/vector_store/qdrant_store.py b/pipeline/vector_store/qdrant_store.py index df86772..844154c 100644 --- a/pipeline/vector_store/qdrant_store.py +++ b/pipeline/vector_store/qdrant_store.py @@ -11,6 +11,7 @@ from qdrant_client.http import models as qmodels from qdrant_client.http.exceptions import UnexpectedResponse +from ..document_validity import Clock, system_clock, today from .embeddings import embed_passages, embed_query, get_vector_size logger = logging.getLogger(__name__) @@ -52,6 +53,10 @@ "scheme_aliases", "chunk_id", "chunk_index", + # Document validity - the period this chunk is searchable in. Absent on + # points written before validity existed, which search treats as current. + "start_date", + "end_date", ) @@ -168,12 +173,58 @@ def _hit_from_point(point: Any) -> dict[str, Any]: return hit +def _validity_conditions(on_date: str) -> list[qmodels.Filter]: + """Conditions keeping only chunks whose validity period covers `on_date`. + + One clause per end, each satisfied either by a missing field or by a date + on the right side of `on_date`. Read together they say: valid unless it + has not started yet, or has already ended. + + The missing-field branches are what keep the pre-validity corpus + searchable - points ingested before `start_date`/`end_date` existed carry + neither, and an operator would rightly call it a regression if adding + validity silently hid every document already in the index. + + Both ends are inclusive: a document uploaded today starts today and must + answer today, and its end date names the last day it answers. + """ + return [ + qmodels.Filter( + should=[ + qmodels.IsEmptyCondition( + is_empty=qmodels.PayloadField(key="start_date") + ), + qmodels.FieldCondition( + key="start_date", + range=qmodels.DatetimeRange(lte=on_date), + ), + ] + ), + qmodels.Filter( + should=[ + qmodels.IsEmptyCondition( + is_empty=qmodels.PayloadField(key="end_date") + ), + qmodels.FieldCondition( + key="end_date", + range=qmodels.DatetimeRange(gte=on_date), + ), + ] + ), + ] + + def _build_filter( doc_id: Optional[str] = None, chunk_num: Optional[int] = None, exclude_reference: bool = False, + valid_on: Optional[str] = None, ) -> Optional[qmodels.Filter]: - must: list[qmodels.FieldCondition] = [] + """Assemble a Qdrant filter. `valid_on` (`YYYY-MM-DD`) restricts the result + to chunks valid on that day; None leaves validity out entirely, which is + what the delete/list paths want - an expired chunk is still that + document's chunk.""" + must: list[qmodels.Condition] = [] if doc_id is not None: must.append( qmodels.FieldCondition( @@ -195,6 +246,8 @@ def _build_filter( match=qmodels.MatchValue(value=False), ) ) + if valid_on: + must.extend(_validity_conditions(valid_on)) if not must: return None return qmodels.Filter(must=must) @@ -203,8 +256,55 @@ def _build_filter( class QdrantVectorStore: backend = "qdrant" - def __init__(self, client: Optional[QdrantClient] = None): + def __init__( + self, + client: Optional[QdrantClient] = None, + clock: Optional[Clock] = None, + ): self.client = client or get_qdrant_client() + # What "today" means when filtering on document validity. Injected so a + # test can pin the day rather than write fixtures relative to the real + # one, and so an operator can ask what search returned on some other + # date without changing the machine's clock. + self.clock: Clock = clock or system_clock + + def today(self) -> str: + """Today per the injected clock, in the `YYYY-MM-DD` payload form.""" + return today(self.clock) + + # Indexed for filtering. `start_date`/`end_date` are DATETIME so Qdrant + # compares them as dates rather than strings; the validity filter also + # works unindexed, so an older collection still filters correctly while + # this is being backfilled - just more slowly. + PAYLOAD_INDEXES = { + "doc_id": qmodels.PayloadSchemaType.KEYWORD, + "workflow_id": qmodels.PayloadSchemaType.KEYWORD, + "filename": qmodels.PayloadSchemaType.KEYWORD, + "instance": qmodels.PayloadSchemaType.KEYWORD, + "chunk_num": qmodels.PayloadSchemaType.INTEGER, + "is_reference": qmodels.PayloadSchemaType.BOOL, + "type": qmodels.PayloadSchemaType.KEYWORD, + "source": qmodels.PayloadSchemaType.KEYWORD, + "start_date": qmodels.PayloadSchemaType.DATETIME, + "end_date": qmodels.PayloadSchemaType.DATETIME, + } + + def _ensure_payload_indexes(self, name: str) -> None: + """Create the filterable payload indexes, ignoring the ones that exist. + + Idempotent by design: Qdrant answers an already-present index with an + error we can safely log and move past, which is cheaper than asking + for the collection's index list first. + """ + for field_name, schema in self.PAYLOAD_INDEXES.items(): + try: + self.client.create_payload_index( + collection_name=name, + field_name=field_name, + field_schema=schema, + ) + except Exception as exc: + logger.debug("Payload index %s skipped/exists: %s", field_name, exc) def ensure_collection(self, name: str, recreate: bool = False) -> dict[str, Any]: vector_size = get_vector_size() @@ -216,6 +316,10 @@ def ensure_collection(self, name: str, recreate: bool = False) -> dict[str, Any] exists = False if exists and not recreate: + # Indexes, not the collection: a collection created before a field + # existed is missing its index, and ingest is the one path that + # reliably runs against every live collection. + self._ensure_payload_indexes(name) return {"index": name, "created": False, "backend": self.backend} if exists and recreate: @@ -229,25 +333,7 @@ def ensure_collection(self, name: str, recreate: bool = False) -> dict[str, Any] ), ) - payload_indexes = { - "doc_id": qmodels.PayloadSchemaType.KEYWORD, - "workflow_id": qmodels.PayloadSchemaType.KEYWORD, - "filename": qmodels.PayloadSchemaType.KEYWORD, - "instance": qmodels.PayloadSchemaType.KEYWORD, - "chunk_num": qmodels.PayloadSchemaType.INTEGER, - "is_reference": qmodels.PayloadSchemaType.BOOL, - "type": qmodels.PayloadSchemaType.KEYWORD, - "source": qmodels.PayloadSchemaType.KEYWORD, - } - for field_name, schema in payload_indexes.items(): - try: - self.client.create_payload_index( - collection_name=name, - field_name=field_name, - field_schema=schema, - ) - except Exception as exc: - logger.debug("Payload index %s skipped/exists: %s", field_name, exc) + self._ensure_payload_indexes(name) return { "index": name, @@ -435,10 +521,21 @@ def search( hybrid_alpha: float = 0.6, ef_search: int = 256, attributes_to_retrieve: Optional[list[str]] = None, + apply_validity: bool = True, + valid_on: Optional[str] = None, ) -> dict[str, Any]: + """Search `name`, by default returning only chunks valid today. + + `valid_on` (`YYYY-MM-DD`) answers the question for another day and + `apply_validity=False` drops the period filter altogether - both exist + for the operator who needs to see what an expired document still holds, + and neither is what a caller serving an end user should pass. + """ mode = (search_mode or "TENSOR").upper() + as_of = (valid_on or self.today()) if apply_validity else None query_filter = _build_filter( exclude_reference=exclude_reference, + valid_on=as_of, ) if mode == "LEXICAL": @@ -468,7 +565,12 @@ def search( hit = _hit_from_point(point) hit["_score"] = score hits.append(hit) - return {"hits": hits, "backend": self.backend, "search_mode": mode} + return { + "hits": hits, + "backend": self.backend, + "search_mode": mode, + "valid_on": as_of, + } vector = embed_query(query, use_e5_prefix=use_e5_prefix) search_params = qmodels.SearchParams(hnsw_ef=ef_search) if ef_search else None @@ -499,4 +601,9 @@ def search( for hit in hits: trimmed.append({k: v for k, v in hit.items() if k in allowed}) hits = trimmed - return {"hits": hits, "backend": self.backend, "search_mode": mode} + return { + "hits": hits, + "backend": self.backend, + "search_mode": mode, + "valid_on": as_of, + } diff --git a/scripts/verify_document_validity_e2e.py b/scripts/verify_document_validity_e2e.py new file mode 100644 index 0000000..e53dd20 --- /dev/null +++ b/scripts/verify_document_validity_e2e.py @@ -0,0 +1,205 @@ +""" +End-to-end check of document validity against a live Qdrant. + +Run against a local stack: uv run python scripts/verify_document_validity_e2e.py +Needs Qdrant on :6333 and the small embedding model cached; it creates and +deletes its own scratch collection and a throwaway SQLite file, and touches +neither the real index nor the real db. + +Real path, no stubs on the parts that matter: real SQLite rows, the real +_prepare_records + QdrantVectorStore.upsert the ingest activity calls, real +embeddings, a real Qdrant collection, and the real search filter. Only the +embedding *model* is swapped for a small cached one so this does not download +2GB to prove a date comparison. +""" + +import os +import sys +import tempfile + +REPO = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +sys.path.insert(0, REPO) + +DB = os.path.join(tempfile.mkdtemp(), "e2e.db") +COLLECTION = "e2e_validity_check" + +os.environ["DOCUMENT_DB_PATH"] = DB +os.environ["AUTH_DISABLED"] = "true" +os.environ["VECTOR_DB_URL"] = "http://localhost:6333" +os.environ["VECTOR_DB_COLLECTION_NAME"] = COLLECTION +os.environ["EMBEDDING_PROVIDER"] = "sentence_transformers" +os.environ["EMBEDDING_MODEL"] = "sentence-transformers/all-MiniLM-L6-v2" +os.environ["EMBEDDING_VECTOR_SIZE"] = "384" +os.environ["EMBEDDING_DIM"] = "384" +os.environ["HF_HUB_OFFLINE"] = "1" + +from datetime import datetime # noqa: E402 + +from pipeline import db # noqa: E402 +from pipeline.activities import _prepare_records, _validity_fields_from_doc # noqa: E402 +from pipeline.vector_store.qdrant_store import QdrantVectorStore # noqa: E402 + +db.DB_PATH = DB +db.init_db() + +PASS, FAIL = [], [] + + +def check(label, actual, expected): + if actual == expected: + PASS.append(label) + print(f" PASS {label}") + else: + FAIL.append(label) + print(f" FAIL {label}\n expected {expected}\n actual {actual}") + + +def clock_at(stamp): + return lambda: datetime.strptime(stamp, "%Y-%m-%d") + + +def make_document(workflow_id, text, valid_from=None, valid_to=None, legacy=False): + db.upsert_document( + workflow_id=workflow_id, + document_id=workflow_id, + filename=f"{workflow_id}.pdf", + filepath=f"/books/{workflow_id}.pdf", + stage="chunk_review", + valid_from=valid_from, + valid_to=valid_to, + ) + if legacy: + # A document that predates the columns: NULL, as the migration leaves it. + db.set_document_validity(workflow_id, None, None) + doc = db.get_document(workflow_id) + chunks = [{"chunk_number": 1, "original_text": text, "token_count": 8, "is_excluded": False}] + kwargs = {} if legacy else _validity_fields_from_doc(doc) + return _prepare_records( + document_id=workflow_id, + filename=f"{workflow_id}.pdf", + chunks=chunks, + workflow_id=workflow_id, + instance="bv", + **kwargs, + ) + + +print("\n=== 1. ingest: four documents, one per validity state ===") +records = [] +records += make_document("wf-active", "Kisan credit card subsidy for active season") +records += make_document("wf-expired", "Kisan credit card subsidy expired scheme", "2024-01-01", "2025-01-01") +records += make_document("wf-future", "Kisan credit card subsidy future scheme", "2027-01-01", "2028-01-01") +records += make_document("wf-legacy", "Kisan credit card subsidy legacy document", legacy=True) + +by_doc = {r["doc_id"]: r for r in records} +today = db.get_document("wf-active")["valid_from"] +check("upload stamps today as the default start", by_doc["wf-active"]["start_date"], today) +check( + "upload stamps a year out as the default end", + by_doc["wf-active"]["end_date"], + db.get_document("wf-active")["valid_to"], +) +check("approver-set period reaches the payload", by_doc["wf-expired"]["start_date"], "2024-01-01") +check("legacy document carries no start_date", "start_date" in by_doc["wf-legacy"], False) +check("legacy document carries no end_date", "end_date" in by_doc["wf-legacy"], False) + +store = QdrantVectorStore() +store.ensure_collection(COLLECTION, recreate=True) +result = store.upsert(COLLECTION, records, batch_size=8) +check("all four chunks upserted", result["records_ingested"], 4) + +info = store.client.get_collection(COLLECTION) +indexed = set((info.payload_schema or {}).keys()) +check("start_date is indexed in Qdrant", "start_date" in indexed, True) +check("end_date is indexed in Qdrant", "end_date" in indexed, True) + + +def search_docs(**kwargs): + hits = store.search(COLLECTION, "kisan credit card subsidy", limit=10, **kwargs)["hits"] + return sorted(h["doc_id"] for h in hits) + + +print("\n=== 2. search today (real clock) ===") +check( + "today returns the active and the undated document only", + search_docs(), + ["wf-active", "wf-legacy"], +) + +print("\n=== 3. search with an injected clock ===") +# Past the active document's default year (today + 1y) and inside the future +# document's window, so exactly one of the two is live. +future_store = QdrantVectorStore(client=store.client, clock=clock_at("2027-12-01")) +check( + "in Dec 2027 the future document is live and the active one has expired", + sorted(h["doc_id"] for h in future_store.search(COLLECTION, "kisan credit card subsidy", limit=10)["hits"]), + ["wf-future", "wf-legacy"], +) +past_store = QdrantVectorStore(client=store.client, clock=clock_at("2024-06-01")) +check( + "in 2024 only the expired document was live", + sorted(h["doc_id"] for h in past_store.search(COLLECTION, "kisan credit card subsidy", limit=10)["hits"]), + ["wf-expired", "wf-legacy"], +) + +print("\n=== 4. boundaries are inclusive ===") +check( + "live on its own first day", + sorted(h["doc_id"] for h in QdrantVectorStore(client=store.client, clock=clock_at("2024-01-01")).search(COLLECTION, "kisan credit card subsidy", limit=10)["hits"]), + ["wf-expired", "wf-legacy"], +) +check( + "live on its own last day", + sorted(h["doc_id"] for h in QdrantVectorStore(client=store.client, clock=clock_at("2025-01-01")).search(COLLECTION, "kisan credit card subsidy", limit=10)["hits"]), + ["wf-expired", "wf-legacy"], +) +check( + "gone the day after it ends", + sorted(h["doc_id"] for h in QdrantVectorStore(client=store.client, clock=clock_at("2025-01-02")).search(COLLECTION, "kisan credit card subsidy", limit=10)["hits"]), + ["wf-legacy"], +) +check( + "absent the day before it starts", + sorted(h["doc_id"] for h in QdrantVectorStore(client=store.client, clock=clock_at("2023-12-31")).search(COLLECTION, "kisan credit card subsidy", limit=10)["hits"]), + ["wf-legacy"], +) + +print("\n=== 5. operator overrides ===") +check("valid_on answers for another day", search_docs(valid_on="2027-12-01"), ["wf-future", "wf-legacy"]) +check( + "include_expired returns everything", + search_docs(apply_validity=False), + ["wf-active", "wf-expired", "wf-future", "wf-legacy"], +) + +print("\n=== 6. lexical mode filters too ===") +check( + "LEXICAL mode applies the same period filter", + sorted(h["doc_id"] for h in store.search(COLLECTION, "kisan", limit=10, search_mode="LEXICAL")["hits"]), + ["wf-active", "wf-legacy"], +) + +print("\n=== 7. document-scoped reads ignore validity ===") +check( + "an expired document's chunks are still listable", + len(store.list_by_doc_id(COLLECTION, "wf-expired")), + 1, +) + +print("\n=== 8. reingest after the approver shortens the period ===") +db.set_document_validity("wf-active", "2026-01-01", "2026-01-31") +doc = db.get_document("wf-active") +again = _prepare_records( + document_id="wf-active", + filename="wf-active.pdf", + chunks=[{"chunk_number": 1, "original_text": "Kisan credit card subsidy for active season", "token_count": 8}], + workflow_id="wf-active", + instance="bv", + **_validity_fields_from_doc(doc), +) +store.upsert(COLLECTION, again, batch_size=8) +check("the shortened period takes it out of today's results", search_docs(), ["wf-legacy"]) + +store.client.delete_collection(COLLECTION) +print(f"\n{len(PASS)} passed, {len(FAIL)} failed") +sys.exit(1 if FAIL else 0) diff --git a/tests/test_activities.py b/tests/test_activities.py index 00f3890..c76a3d6 100644 --- a/tests/test_activities.py +++ b/tests/test_activities.py @@ -266,6 +266,110 @@ def test_prepare_records_uses_edited_text(self): assert records[0]["text"] == "Edited" +class TestPrepareRecordsValidity: + """The validity period every ingested chunk carries.""" + + CHUNKS = [ + {"chunk_number": 1, "original_text": "Chunk one", "token_count": 5}, + {"chunk_number": 2, "original_text": "Chunk two", "token_count": 5}, + ] + + @pytest.mark.unit + def test_stamps_the_period_on_every_chunk(self): + # Every chunk, not just the first: the filter runs per point, so a + # chunk without dates would outlive its own document. + from pipeline.activities import _prepare_records + + records = _prepare_records( + document_id="test-doc", + filename="test.pdf", + chunks=self.CHUNKS, + valid_from="2026-09-22", + valid_to="2027-09-22", + ) + + assert len(records) == 2 + for record in records: + assert record["start_date"] == "2026-09-22" + assert record["end_date"] == "2027-09-22" + + @pytest.mark.unit + def test_omits_the_period_when_the_document_has_none(self): + # Absent, not null — the search filter's missing-field branch is what + # keeps such a chunk answerable. + from pipeline.activities import _prepare_records + + records = _prepare_records( + document_id="test-doc", filename="test.pdf", chunks=self.CHUNKS + ) + + assert "start_date" not in records[0] + assert "end_date" not in records[0] + + @pytest.mark.unit + def test_omits_the_period_when_only_one_end_is_stored(self): + from pipeline.activities import _prepare_records + + records = _prepare_records( + document_id="test-doc", + filename="test.pdf", + chunks=self.CHUNKS, + valid_from="2026-09-22", + ) + + assert "start_date" not in records[0] + + @pytest.mark.unit + def test_the_period_is_in_the_passage_schema(self): + from pipeline.activities import _passage_schema_field_names + + assert {"start_date", "end_date"} <= _passage_schema_field_names() + + +class TestValidityFieldsFromDoc: + """What the ingest activities read off a documents row.""" + + @pytest.mark.unit + def test_uses_the_stored_period(self): + from pipeline.activities import _validity_fields_from_doc + + fields = _validity_fields_from_doc( + {"valid_from": "2026-10-01", "valid_to": "2026-12-31"} + ) + + assert fields == {"valid_from": "2026-10-01", "valid_to": "2026-12-31"} + + @pytest.mark.unit + def test_falls_back_to_the_upload_day_for_a_row_with_no_period(self): + # A document uploaded before validity existed, being reingested: its + # own upload day is a truer start than today, which would silently + # extend its life by another year. + from pipeline.activities import _validity_fields_from_doc + + fields = _validity_fields_from_doc({"created_at": "2024-05-01T09:30:00"}) + + assert fields == {"valid_from": "2024-05-01", "valid_to": "2025-05-01"} + + @pytest.mark.unit + def test_falls_back_to_today_when_the_row_has_no_upload_day_either(self): + from datetime import date + + from pipeline.activities import _validity_fields_from_doc + + fields = _validity_fields_from_doc({}) + + assert fields["valid_from"] == date.today().isoformat() + + @pytest.mark.unit + def test_always_returns_both_ends(self): + # The ingest path must never write half a period. + from pipeline.activities import _validity_fields_from_doc + + for doc in ({}, None, {"valid_from": "2026-01-01"}, {"created_at": "bogus"}): + fields = _validity_fields_from_doc(doc) + assert fields["valid_from"] and fields["valid_to"] + + class TestUpdateDocumentState: """Tests for the state update activity.""" diff --git a/tests/test_api.py b/tests/test_api.py index c275db2..d77bcd5 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -223,6 +223,243 @@ def test_a_rejected_window_stores_nothing_and_promotes_nothing( mock_temporal_client.start_workflow.assert_not_called() +class TestDocumentValidityEndpoints: + """The validity period a reviewer sets alongside the document type. + + The period is stamped on upload so the form is never blank, and the PATCH + that carries the document type carries the reviewer's edit to it. + """ + + def _uploaded_document(self, db_connection, workflow_id): + db_connection.upsert_document( + workflow_id=workflow_id, + document_id=f"doc-{workflow_id}", + filename="test.pdf", + filepath="/app/books/test.pdf", + stage="chunk_review", + ) + + @pytest.mark.api + @pytest.mark.unit + def test_a_new_document_is_valid_from_today_for_a_year(self, test_client, db_connection): + from pipeline.document_validity import default_period + + workflow_id = "validity-001" + self._uploaded_document(db_connection, workflow_id) + expected = default_period() + + doc = test_client.get(f"/documents/{workflow_id}").json() + + assert doc["valid_from"] == expected.start_date + assert doc["valid_to"] == expected.end_date + + @pytest.mark.api + @pytest.mark.unit + def test_stores_the_reviewers_edit(self, test_client, db_connection): + workflow_id = "validity-002" + self._uploaded_document(db_connection, workflow_id) + + response = test_client.patch( + f"/documents/{workflow_id}/scheme-metadata", + json={ + "document_kind": "advisory", + "valid_from": "2026-10-01", + "valid_to": "2026-12-31", + }, + ) + + assert response.status_code == 200 + assert response.json()["valid_from"] == "2026-10-01" + assert response.json()["valid_to"] == "2026-12-31" + stored = db_connection.get_document(workflow_id) + assert stored["valid_from"] == "2026-10-01" + assert stored["valid_to"] == "2026-12-31" + + @pytest.mark.api + @pytest.mark.unit + def test_moving_only_the_end_keeps_the_stored_start(self, test_client, db_connection): + workflow_id = "validity-003" + self._uploaded_document(db_connection, workflow_id) + db_connection.set_document_validity(workflow_id, "2026-01-01", "2026-06-30") + + response = test_client.patch( + f"/documents/{workflow_id}/scheme-metadata", + json={"document_kind": "advisory", "valid_to": "2027-06-30"}, + ) + + assert response.status_code == 200 + assert response.json()["valid_from"] == "2026-01-01" + assert response.json()["valid_to"] == "2027-06-30" + + @pytest.mark.api + @pytest.mark.unit + def test_classifying_without_dates_leaves_the_period_alone(self, test_client, db_connection): + workflow_id = "validity-004" + self._uploaded_document(db_connection, workflow_id) + db_connection.set_document_validity(workflow_id, "2026-01-01", "2026-06-30") + + test_client.patch( + f"/documents/{workflow_id}/scheme-metadata", + json={"document_kind": "advisory"}, + ) + + stored = db_connection.get_document(workflow_id) + assert stored["valid_from"] == "2026-01-01" + assert stored["valid_to"] == "2026-06-30" + + @pytest.mark.api + @pytest.mark.unit + def test_rejects_an_end_before_the_start(self, test_client, db_connection): + workflow_id = "validity-005" + self._uploaded_document(db_connection, workflow_id) + + response = test_client.patch( + f"/documents/{workflow_id}/scheme-metadata", + json={ + "document_kind": "advisory", + "valid_from": "2026-12-31", + "valid_to": "2026-01-01", + }, + ) + + assert response.status_code == 400 + assert "cannot be before" in response.json()["detail"] + + @pytest.mark.api + @pytest.mark.unit + def test_rejects_a_malformed_date(self, test_client, db_connection): + workflow_id = "validity-006" + self._uploaded_document(db_connection, workflow_id) + + response = test_client.patch( + f"/documents/{workflow_id}/scheme-metadata", + json={"document_kind": "advisory", "valid_to": "31-12-2026"}, + ) + + assert response.status_code == 400 + assert "YYYY-MM-DD" in response.json()["detail"] + + @pytest.mark.api + @pytest.mark.unit + def test_a_rejected_period_stores_nothing(self, test_client, db_connection): + # Same contract the prod gate has: a 400 leaves the document as it was, + # kind included, rather than half-applying the PATCH. + workflow_id = "validity-007" + self._uploaded_document(db_connection, workflow_id) + db_connection.set_document_validity(workflow_id, "2026-01-01", "2026-06-30") + + test_client.patch( + f"/documents/{workflow_id}/scheme-metadata", + json={"document_kind": "advisory", "valid_to": "not-a-date"}, + ) + + stored = db_connection.get_document(workflow_id) + assert stored["valid_from"] == "2026-01-01" + assert stored["valid_to"] == "2026-06-30" + assert (stored["document_kind"] or "document") == "document" + + @pytest.mark.api + @pytest.mark.unit + def test_the_period_is_listed_with_the_document(self, test_client, db_connection): + workflow_id = "validity-008" + self._uploaded_document(db_connection, workflow_id) + db_connection.set_document_validity(workflow_id, "2026-01-01", "2026-06-30") + + listed = test_client.get("/documents").json() + row = next(d for d in listed if d["workflow_id"] == workflow_id) + + assert row["valid_from"] == "2026-01-01" + assert row["valid_to"] == "2026-06-30" + + +class TestSearchValidity: + """Search must not answer from a document outside its validity period.""" + + @pytest.mark.api + @pytest.mark.unit + def test_applies_validity_by_default(self, test_client, monkeypatch): + from datetime import date + + captured = {} + + class FakeStore: + backend = "qdrant" + + def search(self, **kwargs): + captured.update(kwargs) + return {"hits": [], "valid_on": kwargs.get("valid_on") or date.today().isoformat()} + + monkeypatch.setattr( + "pipeline.vector_store.get_vector_store", lambda: FakeStore() + ) + + response = test_client.post("/search", json={"query": "kisan"}) + + assert response.status_code == 200 + assert captured["apply_validity"] is True + config = response.json()["effective_config"] + assert config["apply_validity"] is True + assert config["valid_on"] == date.today().isoformat() + + @pytest.mark.api + @pytest.mark.unit + def test_an_operator_can_ask_about_another_day(self, test_client, monkeypatch): + captured = {} + + class FakeStore: + backend = "qdrant" + + def search(self, **kwargs): + captured.update(kwargs) + return {"hits": [], "valid_on": kwargs.get("valid_on")} + + monkeypatch.setattr( + "pipeline.vector_store.get_vector_store", lambda: FakeStore() + ) + + response = test_client.post( + "/search", json={"query": "kisan", "valid_on": "2027-01-01"} + ) + + assert response.status_code == 200 + assert captured["valid_on"] == "2027-01-01" + assert response.json()["effective_config"]["valid_on"] == "2027-01-01" + + @pytest.mark.api + @pytest.mark.unit + def test_an_operator_can_include_expired_documents(self, test_client, monkeypatch): + captured = {} + + class FakeStore: + backend = "qdrant" + + def search(self, **kwargs): + captured.update(kwargs) + return {"hits": [], "valid_on": None} + + monkeypatch.setattr( + "pipeline.vector_store.get_vector_store", lambda: FakeStore() + ) + + response = test_client.post( + "/search", json={"query": "kisan", "include_expired": True} + ) + + assert response.status_code == 200 + assert captured["apply_validity"] is False + assert response.json()["effective_config"]["apply_validity"] is False + + @pytest.mark.api + @pytest.mark.unit + def test_rejects_a_malformed_valid_on(self, test_client): + response = test_client.post( + "/search", json={"query": "kisan", "valid_on": "01-01-2027"} + ) + + assert response.status_code == 400 + assert "YYYY-MM-DD" in response.json()["detail"] + + class TestPageEndpoints: """Tests for page operations.""" diff --git a/tests/test_document_repository.py b/tests/test_document_repository.py index 1752af8..da5220a 100644 --- a/tests/test_document_repository.py +++ b/tests/test_document_repository.py @@ -8,6 +8,7 @@ import pytest from pipeline.document_repository import DocumentRepository +from pipeline.document_validity import ValidityPeriod from pipeline.network_validity import ValidityWindow @@ -117,3 +118,60 @@ def test_reads_a_window_written_through_the_db_layer(self, db_connection, sample @pytest.mark.db def test_an_unapproved_document_has_no_window(self, db_connection, sample_document): assert DocumentRepository().get_network_validity(sample_document["workflow_id"]) is None + + +class TestGetValidity: + @pytest.mark.unit + def test_returns_the_stored_period(self): + repo = DocumentRepository( + FakeDb(document={"valid_from": "2026-09-22", "valid_to": "2027-09-22"}) + ) + + assert repo.get_validity("wf-1") == ValidityPeriod( + start_date="2026-09-22", end_date="2027-09-22" + ) + + @pytest.mark.unit + def test_returns_none_for_a_missing_document(self): + assert DocumentRepository(FakeDb(document=None)).get_validity("wf-nope") is None + + @pytest.mark.unit + def test_returns_none_when_the_document_has_no_period(self): + # Uploaded before validity existed: its chunks carry no dates and are + # searchable without restriction. + repo = DocumentRepository(FakeDb(document={"valid_from": None, "valid_to": None})) + + assert repo.get_validity("wf-1") is None + + @pytest.mark.unit + def test_reads_a_real_row_written_by_db(self, db_connection): + db_connection.upsert_document( + workflow_id="wf-validity", + document_id="doc-validity", + filename="a.pdf", + filepath="/books/a.pdf", + ) + db_connection.set_document_validity("wf-validity", "2026-10-01", "2026-12-31") + + assert DocumentRepository(db_connection).get_validity("wf-validity") == ValidityPeriod( + start_date="2026-10-01", end_date="2026-12-31" + ) + + @pytest.mark.unit + def test_a_freshly_uploaded_document_already_has_a_period(self, db_connection): + # Nothing asks for it on upload — db stamps the default so no document + # is ever ingested without one. + from datetime import date + + db_connection.upsert_document( + workflow_id="wf-fresh", + document_id="doc-fresh", + filename="b.pdf", + filepath="/books/b.pdf", + ) + + period = DocumentRepository(db_connection).get_validity("wf-fresh") + + assert period is not None + assert period.start_date == date.today().isoformat() + assert period.is_active_on(date.today().isoformat()) diff --git a/tests/test_document_validity.py b/tests/test_document_validity.py new file mode 100644 index 0000000..2f9cc8b --- /dev/null +++ b/tests/test_document_validity.py @@ -0,0 +1,189 @@ +"""Unit tests for the period a document's chunks are searchable in. + +The clock is injected everywhere it matters, so these tests pin a day rather +than expressing expectations relative to whatever day they run on. +""" + +from datetime import datetime + +import pytest + +from pipeline.document_validity import ( + DEFAULT_VALIDITY_YEARS, + DocumentValidityError, + ValidityPeriod, + add_years, + default_period, + parse_period, + period_from_row, + period_from_upload_date, + system_clock, + today, +) + + +def clock_at(stamp: str): + """A clock frozen at `stamp` (`YYYY-MM-DD`).""" + return lambda: datetime.strptime(stamp, "%Y-%m-%d") + + +FROZEN = clock_at("2026-09-22") + + +class TestToday: + @pytest.mark.unit + def test_reads_the_injected_clock(self): + assert today(FROZEN) == "2026-09-22" + + @pytest.mark.unit + def test_falls_back_to_the_system_clock(self): + assert today() == system_clock().date().isoformat() + + +class TestAddYears: + @pytest.mark.unit + def test_moves_a_date_a_year_on(self): + assert add_years("2026-09-22") == "2027-09-22" + + @pytest.mark.unit + def test_default_is_one_year(self): + assert DEFAULT_VALIDITY_YEARS == 1 + + @pytest.mark.unit + def test_leap_day_lands_on_the_28th(self): + # 2027-02-29 does not exist; 1 March would push the period into the + # wrong month, so the last day of February is the honest answer. + assert add_years("2024-02-29") == "2025-02-28" + + @pytest.mark.unit + def test_leap_day_to_a_leap_year_keeps_the_29th(self): + assert add_years("2023-02-28") == "2024-02-28" + + @pytest.mark.unit + def test_rejects_a_malformed_date(self): + with pytest.raises(DocumentValidityError, match="YYYY-MM-DD"): + add_years("22-09-2026") + + +class TestDefaultPeriod: + @pytest.mark.unit + def test_starts_today_and_runs_a_year(self): + # What an uploader's document gets before anyone edits anything. + assert default_period(FROZEN) == ValidityPeriod( + start_date="2026-09-22", end_date="2027-09-22" + ) + + @pytest.mark.unit + def test_is_active_on_its_own_first_day(self): + # The whole point of defaulting the start to the upload day: a document + # uploaded this morning has to be searchable this afternoon. + assert default_period(FROZEN).is_active_on("2026-09-22") + + @pytest.mark.unit + def test_is_active_on_its_own_last_day(self): + assert default_period(FROZEN).is_active_on("2027-09-22") + + @pytest.mark.unit + def test_is_not_active_the_day_after_it_ends(self): + assert not default_period(FROZEN).is_active_on("2027-09-23") + + @pytest.mark.unit + def test_is_not_active_the_day_before_it_starts(self): + assert not default_period(FROZEN).is_active_on("2026-09-21") + + +class TestPeriodFromUploadDate: + @pytest.mark.unit + def test_anchors_on_the_upload_day_not_today(self): + # A document uploaded two years ago must not have its life quietly + # extended by a year just because it is being reingested today. + assert period_from_upload_date("2024-05-01") == ValidityPeriod( + start_date="2024-05-01", end_date="2025-05-01" + ) + + @pytest.mark.unit + def test_rejects_a_malformed_upload_date(self): + with pytest.raises(DocumentValidityError, match="upload date"): + period_from_upload_date("") + + +class TestParsePeriod: + @pytest.mark.unit + def test_defaults_both_ends(self): + assert parse_period(clock=FROZEN) == ValidityPeriod( + start_date="2026-09-22", end_date="2027-09-22" + ) + + @pytest.mark.unit + @pytest.mark.parametrize("blank", [None, "", " "]) + def test_defaults_a_blank_end_to_a_year_past_the_start(self, blank): + assert parse_period("2026-01-15", blank, clock=FROZEN).end_date == "2027-01-15" + + @pytest.mark.unit + @pytest.mark.parametrize("blank", [None, "", " "]) + def test_defaults_a_blank_start_to_today(self, blank): + assert parse_period(blank, "2030-01-01", clock=FROZEN).start_date == "2026-09-22" + + @pytest.mark.unit + def test_keeps_an_approver_edited_period(self): + assert parse_period("2026-10-01", "2026-12-31", clock=FROZEN) == ValidityPeriod( + start_date="2026-10-01", end_date="2026-12-31" + ) + + @pytest.mark.unit + def test_a_single_day_period_is_legal(self): + assert parse_period("2026-09-22", "2026-09-22", clock=FROZEN).end_date == "2026-09-22" + + @pytest.mark.unit + def test_a_backdated_start_is_legal(self): + # Digitising an order issued last year is a real case; the period + # describes the content, not when someone got around to uploading it. + assert parse_period("2025-01-01", "2026-12-31", clock=FROZEN).start_date == "2025-01-01" + + @pytest.mark.unit + def test_trims_surrounding_whitespace(self): + period = parse_period(" 2026-10-01 ", " 2026-12-31 ", clock=FROZEN) + + assert period == ValidityPeriod(start_date="2026-10-01", end_date="2026-12-31") + + @pytest.mark.unit + def test_rejects_an_end_before_the_start(self): + with pytest.raises(DocumentValidityError, match="cannot be before"): + parse_period("2026-12-31", "2026-01-01", clock=FROZEN) + + @pytest.mark.unit + @pytest.mark.parametrize("bad", ["22-09-2026", "2026/09/22", "next year", "2026-13-01"]) + def test_rejects_a_malformed_date(self, bad): + with pytest.raises(DocumentValidityError, match="YYYY-MM-DD"): + parse_period(bad, "2027-01-01", clock=FROZEN) + + @pytest.mark.unit + def test_names_the_end_it_rejected(self): + with pytest.raises(DocumentValidityError, match="end date"): + parse_period("2026-01-01", "not-a-date", clock=FROZEN) + + +class TestPeriodFromRow: + @pytest.mark.unit + def test_rebuilds_a_stored_period(self): + assert period_from_row("2026-01-01", "2026-12-31") == ValidityPeriod( + start_date="2026-01-01", end_date="2026-12-31" + ) + + @pytest.mark.unit + @pytest.mark.parametrize( + "start,end", + [(None, None), ("2026-01-01", None), (None, "2026-12-31"), ("", ""), (" ", "2026-12-31")], + ) + def test_returns_none_when_either_end_is_missing(self, start, end): + # A document uploaded before validity existed. None means "no stated + # period", which search reads as always current. + assert period_from_row(start, end) is None + + @pytest.mark.unit + def test_returns_none_for_a_row_this_module_never_wrote(self): + assert period_from_row("01/01/2026", "31/12/2026") is None + + @pytest.mark.unit + def test_returns_none_for_an_inverted_stored_period(self): + assert period_from_row("2026-12-31", "2026-01-01") is None diff --git a/tests/test_qdrant_validity.py b/tests/test_qdrant_validity.py new file mode 100644 index 0000000..56ab6e4 --- /dev/null +++ b/tests/test_qdrant_validity.py @@ -0,0 +1,229 @@ +"""Unit tests for validity filtering in the Qdrant store. + +Two styles on purpose: the filter-shape tests pin the Qdrant conditions the +store builds (no server needed), and the fake-client tests pin that search +actually applies them and honours the injected clock. +""" + +from datetime import datetime + +import pytest +from qdrant_client.http import models as qmodels + +from pipeline.vector_store.qdrant_store import ( + PAYLOAD_FIELDS, + QdrantVectorStore, + _build_filter, + _record_payload, + _validity_conditions, +) + + +def clock_at(stamp: str): + return lambda: datetime.strptime(stamp, "%Y-%m-%d") + + +TODAY = "2026-09-22" + + +class FakeClient: + """Records the filter it was asked to search with.""" + + def __init__(self): + self.query_filter = None + self.scroll_filter = None + + def get_collection(self, name): + raise RuntimeError("collection missing") + + def query_points(self, collection_name, query, query_filter, limit, with_payload, search_params=None): + self.query_filter = query_filter + return type("Result", (), {"points": []})() + + def scroll(self, collection_name, scroll_filter=None, limit=None, offset=None, with_payload=True, with_vectors=False): + self.scroll_filter = scroll_filter + return [], None + + +def dates_in(filt): + """The (key, operator, day) triples the filter compares dates with. + + The day is normalised back to `YYYY-MM-DD`: qdrant-client's DatetimeRange + widens the date we pass into a datetime, which is the wire form and not + what these tests are about. + """ + found = [] + for condition in filt.must or []: + for branch in getattr(condition, "should", None) or []: + rng = getattr(branch, "range", None) + if rng is None: + continue + for op in ("lte", "gte", "lt", "gt"): + value = getattr(rng, op, None) + if value is not None: + day = getattr(value, "date", lambda: value)() + found.append((branch.key, op, str(day))) + return found + + +def empty_keys_in(filt): + """The payload keys the filter accepts as missing.""" + keys = [] + for condition in filt.must or []: + for branch in getattr(condition, "should", None) or []: + is_empty = getattr(branch, "is_empty", None) + if is_empty is not None: + keys.append(is_empty.key) + return keys + + +class TestValidityConditions: + @pytest.mark.unit + def test_bounds_both_ends_around_the_given_day(self): + filt = qmodels.Filter(must=_validity_conditions(TODAY)) + + assert sorted(dates_in(filt)) == [ + ("end_date", "gte", TODAY), + ("start_date", "lte", TODAY), + ] + + @pytest.mark.unit + def test_bounds_are_inclusive(self): + # lte/gte, not lt/gt: a document starting today answers today, and its + # end date names the last day it answers. + filt = qmodels.Filter(must=_validity_conditions(TODAY)) + + assert {op for _, op, _ in dates_in(filt)} == {"lte", "gte"} + + @pytest.mark.unit + def test_accepts_a_chunk_missing_either_date(self): + # This is what keeps the pre-validity corpus searchable. + filt = qmodels.Filter(must=_validity_conditions(TODAY)) + + assert sorted(empty_keys_in(filt)) == ["end_date", "start_date"] + + +class TestBuildFilter: + @pytest.mark.unit + def test_adds_validity_when_a_day_is_given(self): + filt = _build_filter(valid_on=TODAY) + + assert len(dates_in(filt)) == 2 + + @pytest.mark.unit + def test_leaves_validity_out_when_no_day_is_given(self): + # The delete/list paths: an expired chunk is still that document's + # chunk, and a purge that skipped expired chunks would orphan them. + assert _build_filter(doc_id="doc-1") is not None + assert dates_in(_build_filter(doc_id="doc-1")) == [] + + @pytest.mark.unit + def test_returns_none_when_nothing_is_constrained(self): + assert _build_filter() is None + + @pytest.mark.unit + def test_validity_alone_is_enough_to_build_a_filter(self): + assert _build_filter(valid_on=TODAY) is not None + + @pytest.mark.unit + def test_composes_with_the_reference_exclusion(self): + filt = _build_filter(exclude_reference=True, valid_on=TODAY) + keys = [getattr(c, "key", None) for c in filt.must] + + assert "is_reference" in keys + assert len(dates_in(filt)) == 2 + + +class TestSearchAppliesValidity: + @pytest.mark.unit + def test_filters_on_the_injected_clocks_today(self): + client = FakeClient() + store = QdrantVectorStore(client=client, clock=clock_at(TODAY)) + + store.search("idx", "kisan", search_mode="LEXICAL") + + assert sorted(dates_in(client.scroll_filter)) == [ + ("end_date", "gte", TODAY), + ("start_date", "lte", TODAY), + ] + + @pytest.mark.unit + def test_reports_the_day_it_applied(self): + store = QdrantVectorStore(client=FakeClient(), clock=clock_at(TODAY)) + + result = store.search("idx", "kisan", search_mode="LEXICAL") + + assert result["valid_on"] == TODAY + + @pytest.mark.unit + def test_valid_on_overrides_the_clock(self): + client = FakeClient() + store = QdrantVectorStore(client=client, clock=clock_at(TODAY)) + + result = store.search("idx", "kisan", search_mode="LEXICAL", valid_on="2027-01-01") + + assert result["valid_on"] == "2027-01-01" + assert ("start_date", "lte", "2027-01-01") in dates_in(client.scroll_filter) + + @pytest.mark.unit + def test_apply_validity_false_drops_the_period_filter(self): + client = FakeClient() + store = QdrantVectorStore(client=client, clock=clock_at(TODAY)) + + result = store.search( + "idx", "kisan", search_mode="LEXICAL", exclude_reference=False, apply_validity=False + ) + + assert result["valid_on"] is None + assert client.scroll_filter is None + + @pytest.mark.unit + def test_validity_is_on_by_default(self): + # A caller that forgets to ask must not serve an expired document. + client = FakeClient() + store = QdrantVectorStore(client=client, clock=clock_at(TODAY)) + + store.search("idx", "kisan", search_mode="LEXICAL") + + assert len(dates_in(client.scroll_filter)) == 2 + + @pytest.mark.unit + def test_defaults_to_the_real_clock(self): + store = QdrantVectorStore(client=FakeClient()) + + assert store.today() == datetime.now().date().isoformat() + + +class TestPayload: + @pytest.mark.unit + def test_carries_the_period_onto_the_point(self): + payload = _record_payload( + {"text": "x", "start_date": "2026-09-22", "end_date": "2027-09-22"} + ) + + assert payload["start_date"] == "2026-09-22" + assert payload["end_date"] == "2027-09-22" + + @pytest.mark.unit + def test_omits_the_period_when_the_record_has_none(self): + # Written as absent rather than null, which is what the missing-field + # branch of the validity filter matches on. + payload = _record_payload({"text": "x"}) + + assert "start_date" not in payload + assert "end_date" not in payload + + @pytest.mark.unit + def test_the_period_is_a_declared_payload_field(self): + assert {"start_date", "end_date"} <= set(PAYLOAD_FIELDS) + + @pytest.mark.unit + def test_the_period_is_indexed_as_a_datetime(self): + assert ( + QdrantVectorStore.PAYLOAD_INDEXES["start_date"] + == qmodels.PayloadSchemaType.DATETIME + ) + assert ( + QdrantVectorStore.PAYLOAD_INDEXES["end_date"] + == qmodels.PayloadSchemaType.DATETIME + ) diff --git a/ui/src/views/DocumentOpsView.jsx b/ui/src/views/DocumentOpsView.jsx index 4f8be6f..2808ad8 100644 --- a/ui/src/views/DocumentOpsView.jsx +++ b/ui/src/views/DocumentOpsView.jsx @@ -109,6 +109,29 @@ function previewSchemeCode(title) { return slug.slice(0, 8) || 'scheme' } +/** + * Validity the document is searchable in. Prefilled rather than blank: the + * server already stamped the upload day and a year out on upload, so the + * reviewer is confirming or moving a real period, not inventing one. Mirrors + * pipeline/document_validity.py - the server revalidates whatever is sent. + */ +function plusOneYearISODate(stamp) { + const [year, month, day] = (stamp || '').split('-') + if (!year || !month || !day) return '' + // Clamp 29 Feb to 28 Feb in a non-leap year, matching add_years() server-side. + const moved = new Date(Number(year) + 1, Number(month) - 1, Number(day)) + const movedMonth = String(moved.getMonth() + 1).padStart(2, '0') + const movedDay = String(moved.getDate()).padStart(2, '0') + if (moved.getMonth() !== Number(month) - 1) return `${Number(year) + 1}-${month}-28` + return `${moved.getFullYear()}-${movedMonth}-${movedDay}` +} + +function validateValidity(validFrom, validTo) { + if (!validFrom || !validTo) return 'Set both a start and an end date.' + if (validTo < validFrom) return 'End date cannot be before the start date.' + return '' +} + const DOCUMENT_KIND_OPTIONS = [ { value: 'scheme', label: 'Scheme' }, { value: 'advisory', label: 'Advisory' }, @@ -118,15 +141,30 @@ const DOCUMENT_KIND_OPTIONS = [ function DocumentClassificationPanel({ doc, workflowId, canClassify, onSaved }) { const hasKind = doc.document_kind && doc.document_kind !== 'document' - const [kind, setKind] = useState(hasKind && !DOCUMENT_KIND_OPTIONS.some(o => o.value === doc.document_kind) ? '__custom__' : (doc.document_kind || '')) + // 'document' is the unclassified default, not a choice the panel offers, so + // it seeds the select as empty — otherwise the select renders blank while the + // save button reads "Saved", which tells a reviewer the opposite of the truth. + const [kind, setKind] = useState( + hasKind && !DOCUMENT_KIND_OPTIONS.some(o => o.value === doc.document_kind) + ? '__custom__' + : (hasKind ? doc.document_kind : '') + ) const [customKind, setCustomKind] = useState(hasKind && !DOCUMENT_KIND_OPTIONS.some(o => o.value === doc.document_kind) ? doc.document_kind : '') const [schemeName, setSchemeName] = useState(doc.scheme_name || '') + const [validFrom, setValidFrom] = useState(doc.valid_from || todayISODate()) + const [validTo, setValidTo] = useState( + doc.valid_to || plusOneYearISODate(doc.valid_from || todayISODate()) + ) const [saving, setSaving] = useState(false) const [error, setError] = useState('') const effectiveKind = kind === '__custom__' ? customKind.trim().toLowerCase() : kind const isScheme = effectiveKind === 'scheme' - const canSave = Boolean(effectiveKind) && (!isScheme || schemeName.trim().length > 0) + const validityError = validateValidity(validFrom, validTo) + const canSave = + Boolean(effectiveKind) && + (!isScheme || schemeName.trim().length > 0) && + !validityError async function save() { if (!canSave || saving) return @@ -139,6 +177,8 @@ function DocumentClassificationPanel({ doc, workflowId, canClassify, onSaved }) body: JSON.stringify({ document_kind: effectiveKind, ...(isScheme ? { scheme_name: schemeName.trim() } : {}), + valid_from: validFrom, + valid_to: validTo, }), }) await onSaved() @@ -149,12 +189,16 @@ function DocumentClassificationPanel({ doc, workflowId, canClassify, onSaved }) } } - const alreadySaved = doc.document_kind === effectiveKind && (!isScheme || doc.scheme_name === schemeName.trim()) + const alreadySaved = + doc.document_kind === effectiveKind + && (!isScheme || doc.scheme_name === schemeName.trim()) + && doc.valid_from === validFrom + && doc.valid_to === validTo return (

- Document type — used by the Master Catalog / AI layer + Document type and validity — used by the Master Catalog / AI layer

@@ -170,6 +214,27 @@ function DocumentClassificationPanel({ doc, workflowId, canClassify, onSaved })
+
+ Valid from + setValidFrom(e.target.value)} + /> +
+
+ Valid until + setValidTo(e.target.value)} + /> +
{kind === '__custom__' && (
Custom type @@ -208,6 +273,15 @@ function DocumentClassificationPanel({ doc, workflowId, canClassify, onSaved }) )}
+ {canClassify && validityError ? ( +

{validityError}

+ ) : null} + {canClassify && !validityError ? ( +

+ Search only answers from this document between these dates. Defaults to a + year from upload — move either end before publishing to dev. +

+ ) : null} {error ?

{error}

: null} {!canClassify && !hasKind ? (

You don't have permission to set document type.

From 39dc95a440f20f9a1b0173edc28e9b8c7fa68597 Mon Sep 17 00:00:00 2001 From: shruti0025 Date: Tue, 22 Sep 2026 23:47:39 +0530 Subject: [PATCH 03/11] feat: remove the network catalog validity window [#111] The announcement lifetime is no longer required. Document validity, added separately, is the only validity the pipeline now tracks. BREAKING CHANGE: POST /documents/{id}/approve-prod no longer accepts a body, and network_valid_from/network_valid_to are gone from the document API. Consumers reading those fields must drop them. The documents columns are left in place, unread, rather than dropped from live databases. --- AGENTS.md | 3 +- CONTEXT.md | 6 +- ...ts-the-network-catalogs-validity-window.md | 21 ---- .../0006-document-validity-filters-search.md | 10 +- docs/SYSTEM_DESIGN.md | 9 +- docs/architecture-pipeline-rbac.md | 5 +- pipeline/activities.py | 14 +-- pipeline/api.py | 33 +---- pipeline/catalog_builder.py | 23 +--- pipeline/db.py | 33 ----- pipeline/discovery_publish_service.py | 12 +- pipeline/document_repository.py | 20 +-- pipeline/document_validity.py | 10 +- pipeline/models.py | 17 --- pipeline/network_validity.py | 118 ------------------ tests/test_activities.py | 104 +-------------- tests/test_api.py | 109 ++++++---------- tests/test_catalog_builder.py | 38 ------ tests/test_discovery_publish_service.py | 59 --------- tests/test_document_repository.py | 41 ------ tests/test_network_validity.py | 118 ------------------ ui/src/views/DocumentOpsView.jsx | 92 +------------- 22 files changed, 77 insertions(+), 818 deletions(-) delete mode 100644 docs/ADR/0005-approver-sets-the-network-catalogs-validity-window.md delete mode 100644 pipeline/network_validity.py delete mode 100644 tests/test_network_validity.py diff --git a/AGENTS.md b/AGENTS.md index e65ff2e..f588bdb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,8 +62,7 @@ Core modules (all under `pipeline/`): - `document_repository.py` — `DocumentRepository`, a domain layer over `db.py` for document reads; `db.py` stays one-function-per-query with no domain knowledge - `network_constants.py` — fixed values sent on the network (catalog/resource ids, topics, languages, Beckn version). The file v2 edits when the AI layer starts deriving them - `catalog_builder.py` — pure builder mapping a document's knowledge kind (`advisory`/`scheme`) to the single `OnDemand` Beckn catalog announced for that kind; returns `None` for any other kind. No env, no I/O -- `document_validity.py` — pure module owning Document Validity: the period a document's chunks answer searches in, the upload-day/+1-year default, and the clock injection point. Shared by `db.py` (stamping on upload), `scheme_catalog.py` (validating a reviewer's edit), `activities.py` (stamping chunk payloads) and `vector_store/qdrant_store.py` (filtering at search time). Not the same window as `network_validity.py` — see CONTEXT.md -- `network_validity.py` — pure module owning the Announcement Lifetime: what a legal `validity` window is, the today/today default an approver starts from, and how it renders on the wire. Shared by `api.py` (validating approver input) and `catalog_builder.py` (placing it on the catalog) +- `document_validity.py` — pure module owning Document Validity: the period a document's chunks answer searches in, the upload-day/+1-year default, and the clock injection point. Shared by `db.py` (stamping on upload), `scheme_catalog.py` (validating a reviewer's edit), `activities.py` (stamping chunk payloads) and `vector_store/qdrant_store.py` (filtering at search time) - `discovery_publish_service.py` — `DiscoveryPublishService`: owns the Publish to Network env vars and the HTTP call. Deliberately Temporal-free - `models.py` — Pydantic models, including `DocumentStage` enum and `PIPELINE_STAGES` (the stepper-UI stage list) - `config.py` — `Config` dataclass reading env vars, with defaults diff --git a/CONTEXT.md b/CONTEXT.md index 597a2b8..2ced2cf 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -28,13 +28,9 @@ _Avoid_: "catalog" alone. What a document *is* to the network — `advisory`, `scheme`, `video`, or an operator-entered slug — held in `documents.document_kind`. Asserted by a reviewer during the pipeline, never inferred from the file, and absent until then (every document starts as the default `document`). Only `advisory` and `scheme` map to a Network Catalog Envelope; the rest publish nothing. _Avoid_: "document type" — collides with `source_type`/`canonical_input_type`, which describe the input *format* (pdf, spreadsheet). Also avoid saying an advisory is "uploaded": what is uploaded is a file, which only becomes an advisory when someone classifies it. -**Announcement Lifetime**: -The `validity` window (`startDate`/`endDate`) carried on the Network Catalog Envelope, named by the prod approver at `approve-prod` and stored as `documents.network_valid_from` / `network_valid_to`. Start is the approval date; end is prepopulated with the same day and is the approver's to move. Because the envelope is kind-level, the most recently published window is the live one for that whole Knowledge Kind — see `docs/ADR/0005-approver-sets-the-network-catalogs-validity-window.md`. -_Avoid_: "document lifetime" — the document itself is not what expires, the announcement is. Also avoid "expiry": there is a start as well as an end. - **Document Validity**: The period a document's chunks answer searches in, held as `documents.valid_from` / `documents.valid_to` and stamped onto every chunk's vector payload as `start_date` / `end_date`. Defaulted on upload to the upload day plus one year, then confirmed or moved by the reviewer in the same form that sets the Knowledge Kind. Both ends are inclusive, and search filters on it so an expired or not-yet-started document is never answered from. Chunks ingested before it existed carry neither date and stay searchable — see `docs/ADR/0006-document-validity-filters-search.md`. -_Avoid_: "validity" alone — ambiguous with the Announcement Lifetime, which is a different window (kind-level, `startDate`/`endDate` on the wire, named by the prod approver). This one is document-level, never leaves the vector store, and expires a document's answers rather than an announcement. +_Avoid_: "expiry" — there is a start as well as an end. Also avoid calling it the document's lifetime: the document stays in the console, editable and reingestable, once its period has passed; only its search answers stop. **network_visible**: An operator-controlled flag on a document/scheme (not a pipeline stage) that gates whether it's exposed to other BAPs through the pull-based Scheme Catalog snapshot. Independent of Publish to Network. diff --git a/docs/ADR/0005-approver-sets-the-network-catalogs-validity-window.md b/docs/ADR/0005-approver-sets-the-network-catalogs-validity-window.md deleted file mode 100644 index 31a0bc6..0000000 --- a/docs/ADR/0005-approver-sets-the-network-catalogs-validity-window.md +++ /dev/null @@ -1,21 +0,0 @@ -# The prod approver sets the published catalog's validity window - -Context: `publishing_to_network` announced a catalog with no lifetime at all. Under `updateMode: MERGE` on a permanent catalog id (ADR 0003), that announcement stands indefinitely — a provider that stops serving a knowledge kind has no way to say so on the wire, and no operator ever states how long the claim should hold. The `network-specs` pack answers this at two levels: `resourceAttributes.validity` on a resource (a `TimePeriod` of `startsAt`/`endsAt`), and a **catalog-level** `validity` of `startDate`/`endDate`, which every provider-catalog example in `schema/examples/` carries. - -Decision: -- The **prod approver** names the window, at the same gate where they already decide a document is fit for PROD. `POST /documents/{id}/approve-prod` takes an optional `{network_valid_from, network_valid_to}` body. -- **Start date is the approval date**, defaulted server-side and shown read-only in the UI: the announcement begins when it is published, and nothing is gained by letting an approver backdate a claim the network only learns about now. -- **End date is prepopulated with the same day and is the one field the approver edits.** Today/today is a deliberate floor rather than a guess — an approver who wants the catalog to stand longer has to say so, instead of inheriting a default nobody chose. -- **Validated before anything is promoted.** The window is parsed and stored at the top of `approve-prod`, before any signal is sent or `PromoteToProdWorkflow` is started, so a rejected window is a plain 400 that leaves the document parked at the gate. `publishing_to_network` therefore only ever reads a window that passed `network_validity.parse_window`. Validation is deliberately thin: a real `YYYY-MM-DD` at each end, and an end not before the start. -- The window lands on the **catalog**, not on `resourceAttributes`. ADR 0003 pins the resource to `informationMode: OnDemand`, whose profile omits `validity`; catalog-level `validity` is where the specs' own examples put a published announcement's lifetime. Dates are widened to the instants those examples use, with the end date inclusive to `23:59:59Z` of that day. -- Stored on the document (`documents.network_valid_from` / `network_valid_to`) and read back by the activity, not threaded through as a workflow argument — that keeps every existing call site and every in-flight Temporal execution untouched, the same reasoning that already applies to `document_kind` in `publish_catalog_to_network`. -- A document with **no stored window publishes without a `validity`**, exactly as before. That covers everything promoted before this change. It is not defaulted to today on read: doing so would expire an established catalog the first time an old document was re-ingested. - -Known limitation, accepted: the envelope is **kind-level** (ADR 0003), so the window an approver sets for one document becomes the window for its whole knowledge kind — the last publish wins under MERGE. A per-document lifetime is not expressible until v2 introduces per-document, AI-derived catalogs; until then "lifetime of the document" means "lifetime of the announcement this document triggered". Operators approving several documents of one kind should expect the most recent window to be the live one. - -Alternatives considered: -- **Collect the window at `ready_for_ingestion` (Publish to Dev) instead.** Rejected: that gate is about DEV content quality and is open to `state_contributor`; the lifetime of a network-facing claim belongs with the super admin who makes that claim (ADR 0004). -- **Put it on `resourceAttributes.validity`.** Rejected: contradicts ADR 0003's OnDemand shape, and the existing `test_catalog_builder` guard against exactly this field. -- **Make both dates required with no default.** Rejected: it breaks every body-less `approve-prod` caller (scripts, the bulk pattern) for no safety gain, since the endpoint validates whatever it ends up with either way. -- **Let the approver backdate the start.** Rejected as the default, but the API still accepts and validates a supplied `network_valid_from` rather than silently ignoring it — an operator scripting a deliberate window gets what they asked for or a 400, never a quiet override. -- **Auto-expire the catalog by re-publishing `isActive: false` at `endDate`.** Out of scope: it needs a scheduler this pipeline does not have, and the window on the wire is what the Discovery Service is meant to enforce. diff --git a/docs/ADR/0006-document-validity-filters-search.md b/docs/ADR/0006-document-validity-filters-search.md index 54604b3..1a112ed 100644 --- a/docs/ADR/0006-document-validity-filters-search.md +++ b/docs/ADR/0006-document-validity-filters-search.md @@ -1,9 +1,11 @@ # Document validity filters search -Context: a chunk, once ingested, answered searches forever. Agricultural content does not work that way — a rabi sowing advisory is wrong advice in June, and a scheme circular superseded last year is worse than no answer. Nothing in the pipeline could express "this stopped being true", so the only way to retire content was to disable the whole document, losing it from the operator console too. ADR 0005 added a window at the prod gate, but that one is the *Announcement Lifetime*: kind-level, on the wire, about whether this provider still claims to serve a knowledge kind. It says nothing about whether any given document's content is current. +Context: a chunk, once ingested, answered searches forever. Agricultural content does not work that way — a rabi sowing advisory is wrong advice in June, and a scheme circular superseded last year is worse than no answer. Nothing in the pipeline could express "this stopped being true", so the only way to retire content was to disable the whole document, losing it from the operator console too. + +A short-lived predecessor (ADR 0005, since removed along with its code) put a *catalog* validity window on the Network Catalog Envelope at the prod gate. That window was kind-level and about whether this provider still claims to serve a knowledge kind at all; it never spoke to whether a given document's content was current, which is what this ADR is about. Decision: -- **A separate, document-level concept.** `documents.valid_from` / `documents.valid_to`, stamped onto every chunk's Qdrant payload as `start_date` / `end_date`, owned by a new pure module `pipeline/document_validity.py`. Deliberately not folded into `network_validity.py`: the two windows have different defaults, different authors, different moments, and different destinations, and CONTEXT.md already warns against conflating them. They share a date format and nothing else. +- **A document-level concept, owned in one place.** `documents.valid_from` / `documents.valid_to`, stamped onto every chunk's Qdrant payload as `start_date` / `end_date`, owned by a pure module `pipeline/document_validity.py` that holds the rules so the API, the ingest activity and the vector store never re-derive them. - **Defaulted on upload, not asked for.** `db.upsert_document` stamps the upload day and one year out on INSERT. An uploader is not asked a question they cannot answer, and no document can reach the index without a period. A year is the business's review cadence, not a technical limit. - **Confirmed or moved by the reviewer, in the form that already exists.** The dates sit beside the Knowledge Kind selector and travel on the same `PATCH /documents/{id}/scheme-metadata`. Choosing the period is part of classifying a document, not a separate errand, and the reviewer sees real prefilled dates rather than an empty field. Either end may be sent alone; the other is taken from what is stored, so moving the end date does not require restating the start. - **Validated like a bad kind, not like a bad type.** `document_validity.parse_period` raises, `apply_scheme_metadata` lets it through as a `ValueError`, and the endpoint answers 400 — the same shape a bad `document_kind` gets, not a Pydantic 422. A rejected period stores nothing, kind included. @@ -17,8 +19,8 @@ Decision: Known limitation, accepted: expiry is enforced at **read** time, not by a sweeper. An expired document's vectors stay in Qdrant, still costing storage and still visible to anything that queries Qdrant directly without this filter (including `include_expired`). That is the right trade for now — deleting on expiry would make un-expiring a document a reingest — but a caller bypassing `/search` is not protected. Alternatives considered: -- **Reuse `network_valid_from`/`network_valid_to`.** Rejected: ADR 0005 gives those a today/today default and a kind-level meaning on the wire. Reusing them would either expire every document the day it is approved or corrupt the announcement window, and would collapse two concepts CONTEXT.md keeps apart. -- **Collect the dates at the prod gate with the Announcement Lifetime.** Rejected: DEV search is filtered too, and the prod gate is reached long after ingestion — a document would be searchable with no period for most of its life. +- **Reuse the catalog validity window from ADR 0005.** Rejected at the time, and moot since that feature was removed: those dates defaulted to today/today and carried a kind-level meaning on the wire, so reusing them would have expired every document the day it was approved. +- **Collect the dates at the prod gate.** Rejected: DEV search is filtered too, and the prod gate is reached long after ingestion — a document would be searchable with no period for most of its life. - **Store epoch integers instead of `YYYY-MM-DD` strings.** Rejected: Qdrant's `DatetimeRange` accepts date-only strings (verified against a live instance), and an operator reading a chunk payload can see `2027-03-31` rather than decoding a number. - **Filter after retrieval, in the API.** Rejected: it silently shrinks the candidate set below `top_k`, so an index full of expired documents would return a short page of results rather than the valid ones further down. - **Ask the uploader for the dates.** Rejected: the uploader is often not the person who knows how long content holds, and blocking upload on it would slow the common case for a value the reviewer sets anyway. diff --git a/docs/SYSTEM_DESIGN.md b/docs/SYSTEM_DESIGN.md index 451142f..7fbaac3 100644 --- a/docs/SYSTEM_DESIGN.md +++ b/docs/SYSTEM_DESIGN.md @@ -195,11 +195,10 @@ read and write paths stay consistent. 1. Super Admin sees all states’ documents in `approval_for_prod` 2. Reviews pages/chunks (optional quality check) -3. Sets the **Announcement Lifetime** — start date is the approval date, end date prepopulated with the same day and editable -4. `POST /documents/{id}/approve-prod` (`RequireAdmin`) with `{network_valid_from, network_valid_to}`; the window is validated and stored before anything is promoted, so a bad window is a 400 that promotes nothing -5. Signal `approve_prod` **or** `PromoteToProdWorkflow` -6. `promote_document_to_prod_qdrant` → **PROD Qdrant** -7. Stage `completed`; audit `promote_to_prod`. The stored window rides along on the `publishing_to_network` catalog as `validity` +3. `POST /documents/{id}/approve-prod` (`RequireAdmin`) +4. Signal `approve_prod` **or** `PromoteToProdWorkflow` +5. `promote_document_to_prod_qdrant` → **PROD Qdrant** +6. Stage `completed`; audit `promote_to_prod` ### 5.3 Search diff --git a/docs/architecture-pipeline-rbac.md b/docs/architecture-pipeline-rbac.md index b1649c1..91e4358 100644 --- a/docs/architecture-pipeline-rbac.md +++ b/docs/architecture-pipeline-rbac.md @@ -225,7 +225,7 @@ Signals (workflow): `approve_ocr`, `approve_translation`, `approve_chunks`, `app API (examples): - `POST /documents/{id}/approve-ocr` … `approve-ingestion` → state-capable roles with `review` -- `POST /documents/{id}/approve-prod` → **`RequireAdmin`** (Super Admin). Also carries the Announcement Lifetime (`network_valid_from` / `network_valid_to`), validated before any promotion is triggered — see `docs/ADR/0005-approver-sets-the-network-catalogs-validity-window.md` +- `POST /documents/{id}/approve-prod` → **`RequireAdmin`** (Super Admin) UI gates: `DocumentOpsView` maps `approve_prod` → permission `admin`; other approvals → `review`. @@ -298,8 +298,7 @@ sequenceDiagram UI->>API: GET /auth/me - unrestricted + admin SA->>UI: Open Approval for Prod queue SA->>API: Review document detail - SA->>UI: Set announcement lifetime - start today, end editable - SA->>API: POST /documents/id/approve-prod with validity window + SA->>API: POST /documents/id/approve-prod API->>T: Signal approve_prod or PromoteToProdWorkflow T->>PROD: promote_document_to_prod_qdrant T->>API: Set stage completed diff --git a/pipeline/activities.py b/pipeline/activities.py index ea282dd..7623f20 100644 --- a/pipeline/activities.py +++ b/pipeline/activities.py @@ -1456,22 +1456,18 @@ async def ingest_document_from_db( async def publish_catalog_to_network(workflow_id: str, transaction_id: str) -> dict: """POST a catalog/publish envelope to the Discovery Service and record the exchange. - The catalog depends on the document's knowledge kind and on the lifetime the - prod approver set for it. Both are read here rather than passed in as - activity arguments - that keeps every workflow call site, and any workflow - already in flight, untouched. + The catalog depends on the document's knowledge kind, read here rather than + passed in as an activity argument - that keeps every workflow call site, and + any workflow already in flight, untouched. """ from . import db repository = DocumentRepository() document_kind = normalize_document_kind(repository.get_document_kind(workflow_id)) - # Set at the prod approval gate and already validated there; None only for a - # document promoted before approvers named a lifetime. - validity = repository.get_network_validity(workflow_id) service = DiscoveryPublishService() result = await asyncio.to_thread( - service.publish, transaction_id, document_kind, workflow_id, validity + service.publish, transaction_id, document_kind, workflow_id ) if result["skipped"]: @@ -1530,8 +1526,6 @@ async def publish_catalog_to_network(workflow_id: str, transaction_id: str) -> d return { "status": "skipped" if result["skipped"] else "published", "document_kind": document_kind, - "valid_from": validity.start_date if validity else None, - "valid_to": validity.end_date if validity else None, "transaction_id": transaction_id, "message_id": None if result["skipped"] else result["envelope"]["context"]["messageId"], "result_status": result["result_status"], diff --git a/pipeline/api.py b/pipeline/api.py index b1ed6f3..5b26cf1 100644 --- a/pipeline/api.py +++ b/pipeline/api.py @@ -95,7 +95,6 @@ OperationQueueEntry, OperationQueueResponse, PageUpdate, - ProdApprovalRequest, RegisterFolderRequest, RegisterRequest, ReindexStateRequest, @@ -104,7 +103,6 @@ SearchSettingsUpdate, SettingsAuditResponse, ) -from .network_validity import ValidityWindowError, parse_window from .workflows import ( ChunkingOnlyWorkflow, DocumentPipelineWorkflow, @@ -456,8 +454,6 @@ def _document_summary_from_row(doc: dict, current_job: Optional[dict] = None) -> network_visible=bool(int(doc["network_visible"])) if doc.get("network_visible") is not None else True, prod_ready_requested_at=doc.get("prod_ready_requested_at"), prod_ready_requested_by_username=doc.get("prod_ready_requested_by_username"), - network_valid_from=doc.get("network_valid_from"), - network_valid_to=doc.get("network_valid_to"), valid_from=doc.get("valid_from"), valid_to=doc.get("valid_to"), ) @@ -1539,8 +1535,6 @@ def _build_document_detail(doc: dict) -> DocumentDetail: network_visible=bool(int(doc["network_visible"])) if doc.get("network_visible") is not None else True, prod_ready_requested_at=doc.get("prod_ready_requested_at"), prod_ready_requested_by_username=doc.get("prod_ready_requested_by_username"), - network_valid_from=doc.get("network_valid_from"), - network_valid_to=doc.get("network_valid_to"), valid_from=doc.get("valid_from"), valid_to=doc.get("valid_to"), ) @@ -3009,16 +3003,8 @@ async def request_prod_ready(workflow_id: str, user: RequireReview): async def approve_prod( workflow_id: str, user: RequireAdmin, - approval: Optional[ProdApprovalRequest] = None, ): - """Superadmin-only: promote DEV-ingested vectors into PROD Qdrant. - - The approver also names the lifetime of the network announcement this - promotion leads to - start date today, end date theirs to set. The window is - validated and stored here, before anything is signaled or started, so a - rejected window costs nothing: `publishing_to_network` only ever reads a - window that passed through `parse_window`. - """ + """Superadmin-only: promote DEV-ingested vectors into PROD Qdrant.""" doc = _require_document_for_user(workflow_id, user) stage = doc.get("stage") # Promoting an already-completed document is a fresh PROD write, so it is @@ -3038,19 +3024,6 @@ async def approve_prod( "(expected 'approval_for_prod' or 'completed').", ) - # Validated before any promotion is triggered: the publish that follows - # reads these dates off the document, and a 400 here must leave the - # document exactly where it was. - try: - validity = parse_window( - approval.network_valid_from if approval else None, - approval.network_valid_to if approval else None, - ) - except ValidityWindowError as exc: - raise HTTPException(400, f"Cannot approve for prod: {exc}") from None - - db.set_network_validity(workflow_id, validity.start_date, validity.end_date) - # Prefer signaling the running main workflow when it is waiting at approval_for_prod. signaled = False if stage == "approval_for_prod": @@ -3108,8 +3081,6 @@ async def approve_prod( "next_stage": "ingesting_prod", "signaled": signaled, "promote_workflow_id": promote_workflow_id, - "network_valid_from": validity.start_date, - "network_valid_to": validity.end_date, }, user=user, ) @@ -3118,8 +3089,6 @@ async def approve_prod( "workflow_id": workflow_id, "signaled": signaled, "promote_workflow_id": promote_workflow_id, - "network_valid_from": validity.start_date, - "network_valid_to": validity.end_date, "next_stage": "ingesting_prod", } diff --git a/pipeline/catalog_builder.py b/pipeline/catalog_builder.py index f4c25f7..7b7da27 100644 --- a/pipeline/catalog_builder.py +++ b/pipeline/catalog_builder.py @@ -5,10 +5,9 @@ document of that kind - a seeker that matches the resource comes back to ask the actual question, so there is nothing per-document to put on the wire. -Pure: no env, no I/O. Values live in network_constants.py, the validity window -in network_validity.py, and the transport in discovery_publish_service.py. See -docs/ADR/0003-static-ondemand-network-catalog-per-knowledge-kind.md and -docs/ADR/0005-approver-set-validity-window-on-the-network-catalog.md. +Pure: no env, no I/O. Values live in network_constants.py and the transport in +discovery_publish_service.py. See +docs/ADR/0003-static-ondemand-network-catalog-per-knowledge-kind.md. """ import copy @@ -33,7 +32,6 @@ SCHEMES_RESOURCE_NAME, SERVED_LANGUAGES, ) -from .network_validity import ValidityWindow, to_catalog_validity _ADVISORY_RESOURCE_ATTRIBUTES = { "@context": f"{SCHEMA_CONTEXT_BASE}/KnowledgeAdvisory/v0.1/context.jsonld", @@ -82,23 +80,12 @@ def normalize_document_kind(document_kind: Optional[str]) -> str: return (document_kind or DEFAULT_DOCUMENT_KIND).strip().lower() -def build_catalog( - document_kind: Optional[str], - validity: Optional[ValidityWindow] = None, -) -> Optional[dict]: +def build_catalog(document_kind: Optional[str]) -> Optional[dict]: """Return the catalog to announce for `document_kind`, or None. None means this kind has nothing to announce - `document`, `video` and any operator-entered custom slug. Callers must skip the publish rather than send an empty `catalogs` array, which the spec rejects (`minItems: 1`). - - `validity` is the lifetime an approver set for the document being published. - It lands on the **catalog**, not on `resourceAttributes`: the OnDemand - KnowledgeAdvisory profile omits a resource-level `validity` (ADR 0003), and - catalog-level `validity` is where the network specs' own provider-catalog - examples put a published announcement's lifetime. Omitted entirely when - None, which leaves a pre-existing announcement's lifetime untouched under - `updateMode: MERGE`. """ spec = _KNOWLEDGE_KIND_CATALOGS.get(normalize_document_kind(document_kind)) if spec is None: @@ -117,6 +104,4 @@ def build_catalog( } ], } - if validity is not None: - catalog["validity"] = to_catalog_validity(validity) return catalog diff --git a/pipeline/db.py b/pipeline/db.py index 7385479..fbf648c 100644 --- a/pipeline/db.py +++ b/pipeline/db.py @@ -120,10 +120,6 @@ def init_db(): _add_column_if_missing(conn, "documents", "prod_ready_requested_at", "TEXT") _add_column_if_missing(conn, "documents", "prod_ready_requested_by_user_id", "TEXT") _add_column_if_missing(conn, "documents", "prod_ready_requested_by_username", "TEXT") - # Lifetime the prod approver set for the network announcement. - # NULL on documents promoted before approvers named one. - _add_column_if_missing(conn, "documents", "network_valid_from", "TEXT") - _add_column_if_missing(conn, "documents", "network_valid_to", "TEXT") # Period this document's chunks are searchable in, stamped on # upload and editable by the approver. NULL on documents uploaded # before validity existed - search reads that as always current @@ -1722,35 +1718,6 @@ def mark_prod_ready_requested( return get_document(workflow_id) -def set_network_validity( - workflow_id: str, - valid_from: str, - valid_to: str, -) -> Optional[dict]: - """Record the lifetime the prod approver set for this document's network - announcement. Both ends are `YYYY-MM-DD`; validation belongs to the caller - (`pipeline/network_validity.parse_window`), not to this SQL layer.""" - with _db_lock: - with get_connection() as conn: - conn.execute( - """ - UPDATE documents - SET network_valid_from = ?, - network_valid_to = ?, - updated_at = ? - WHERE workflow_id = ? - """, - ( - valid_from, - valid_to, - datetime.utcnow().isoformat(), - workflow_id, - ), - ) - conn.commit() - return get_document(workflow_id) - - def set_document_validity( workflow_id: str, valid_from: str, diff --git a/pipeline/discovery_publish_service.py b/pipeline/discovery_publish_service.py index a7ac9cf..d34f2b8 100644 --- a/pipeline/discovery_publish_service.py +++ b/pipeline/discovery_publish_service.py @@ -18,7 +18,6 @@ from .catalog_builder import build_catalog from .network_constants import BECKN_VERSION, RESULT_ACCEPTED -from .network_validity import ValidityWindow logger = logging.getLogger(__name__) @@ -48,7 +47,6 @@ def publish( transaction_id: str, document_kind: Optional[str], workflow_id: Optional[str] = None, - validity: Optional[ValidityWindow] = None, ) -> dict: """ POST a catalog/publish envelope for `document_kind`. @@ -57,16 +55,11 @@ def publish( reused across retries) - only `messageId` is generated fresh here, per Beckn convention. - `validity` is the lifetime the approver set for the document being - published; None announces the catalog without one. The window is - already validated by the time it reaches here - see - `network_validity.parse_window`. - A kind with no catalog mapped to it makes no HTTP call and comes back with `skipped=True`: the spec declares `message.catalogs` as `minItems: 1`, so there is no valid "publish nothing" request to send. """ - catalog = build_catalog(document_kind, validity) + catalog = build_catalog(document_kind) if catalog is None: logger.info( "workflow_id=%s document_kind=%s network_publish_skipped=True", @@ -109,13 +102,12 @@ def publish( url = f"{self.endpoint.rstrip('/')}/publish" logger.info( "workflow_id=%s catalog_id=%s transaction_id=%s message_id=%s " - "network_publish_url=%s network_validity=%s", + "network_publish_url=%s", workflow_id, catalog["id"], transaction_id, envelope["context"]["messageId"], url, - catalog.get("validity"), ) logger.debug("Request to %s with \n body %s", url, envelope) try: diff --git a/pipeline/document_repository.py b/pipeline/document_repository.py index 1acbd39..c5fb758 100644 --- a/pipeline/document_repository.py +++ b/pipeline/document_repository.py @@ -9,9 +9,7 @@ from typing import Optional from . import db -from .document_validity import ValidityPeriod -from .document_validity import period_from_row as validity_period_from_row -from .network_validity import ValidityWindow, window_from_row +from .document_validity import ValidityPeriod, period_from_row class DocumentRepository: @@ -32,20 +30,6 @@ def get_document_kind(self, workflow_id: str) -> Optional[str]: return None return doc.get("document_kind") - def get_network_validity(self, workflow_id: str) -> Optional[ValidityWindow]: - """The lifetime the prod approver set for this document's network - announcement, or None when it has none. - - None is not an error: documents promoted before approvers named a - window have no stored dates, and those publish without a validity. - """ - doc = self._db.get_document(workflow_id) - if not doc: - return None - return window_from_row( - doc.get("network_valid_from"), doc.get("network_valid_to") - ) - def get_validity(self, workflow_id: str) -> Optional[ValidityPeriod]: """The period this document's chunks are searchable in, or None when it has none. @@ -57,4 +41,4 @@ def get_validity(self, workflow_id: str) -> Optional[ValidityPeriod]: doc = self._db.get_document(workflow_id) if not doc: return None - return validity_period_from_row(doc.get("valid_from"), doc.get("valid_to")) + return period_from_row(doc.get("valid_from"), doc.get("valid_to")) diff --git a/pipeline/document_validity.py b/pipeline/document_validity.py index e555fcd..7c58f41 100644 --- a/pipeline/document_validity.py +++ b/pipeline/document_validity.py @@ -8,12 +8,6 @@ the ingest activity can stamp the same period onto every chunk, and the vector store can build a search filter from it without any of them re-deriving it. -Deliberately separate from `pipeline/network_validity.py`, which owns the -Announcement Lifetime: a different window, with different defaults (today / -today, not today / a year out), named by a different person at a different -moment, and carried on the wire rather than in a chunk payload. They share a -date format and nothing else; see `CONTEXT.md` on not conflating the two. - `pipeline/activities.py` stamps the period onto chunk payloads as `start_date` / `end_date`; `pipeline/vector_store/qdrant_store.py` filters on those fields at search time; `pipeline/api.py` validates approver input here. @@ -42,8 +36,8 @@ class DocumentValidityError(ValueError): def system_clock() -> datetime: - """The real clock. Local, matching `network_validity.today()`'s - `date.today()`, so both windows agree on what day it is.""" + """The real clock. Local rather than UTC: a period is a calendar date an + operator picked, so "today" should mean their today.""" return datetime.now() diff --git a/pipeline/models.py b/pipeline/models.py index b84aff4..7f06afd 100644 --- a/pipeline/models.py +++ b/pipeline/models.py @@ -213,9 +213,6 @@ class DocumentSummary(BaseModel): network_visible: bool = True prod_ready_requested_at: Optional[str] = None prod_ready_requested_by_username: Optional[str] = None - # Lifetime of the network announcement, set by the prod approver. - network_valid_from: Optional[str] = None - network_valid_to: Optional[str] = None # Period this document's chunks are searchable in. Stamped on upload and # editable alongside the document type; NULL on documents uploaded before # validity existed. @@ -223,20 +220,6 @@ class DocumentSummary(BaseModel): valid_to: Optional[str] = None -class ProdApprovalRequest(BaseModel): - """POST /documents/{id}/approve-prod body. - - Both ends are `YYYY-MM-DD` and both are optional: an omitted end defaults to - today, so a body-less approval still publishes a well-formed window. The - dates are validated in the endpoint (see `network_validity.parse_window`) - rather than by a Pydantic validator, so a bad window comes back as the same - 400 an approver gets for a bad stage, not a 422 with a Pydantic trace. - """ - - network_valid_from: Optional[str] = None - network_valid_to: Optional[str] = None - - class SchemeMetadataUpdate(BaseModel): """PATCH /documents/{id}/scheme-metadata body. diff --git a/pipeline/network_validity.py b/pipeline/network_validity.py deleted file mode 100644 index 6347c91..0000000 --- a/pipeline/network_validity.py +++ /dev/null @@ -1,118 +0,0 @@ -""" -The validity window a published catalog is announced with. - -An operator names how long this provider's announcement should stand; this -module owns what a legal window is, what the default one is, and how it renders -on the wire. Pure: no env, no I/O, no db - so the API can validate an operator's -input and the activity can rebuild the same window from a stored row without -either of them re-deriving the rules. - -`pipeline/catalog_builder.py` places the rendered window on the catalog; -`pipeline/api.py` validates operator input against `parse_window`. -""" - -from dataclasses import dataclass -from datetime import date, datetime -from typing import Optional - -# Operators think in dates, not instants - the window is stored and exchanged -# as a plain calendar date and only widened to an instant on the wire. -DATE_FORMAT = "%Y-%m-%d" - - -class ValidityWindowError(ValueError): - """An operator-supplied window that cannot be published.""" - - -@dataclass(frozen=True) -class ValidityWindow: - """Inclusive calendar span, both ends `YYYY-MM-DD`.""" - - start_date: str - end_date: str - - -def today() -> str: - """Today's date in the stored form.""" - return date.today().strftime(DATE_FORMAT) - - -def default_window() -> ValidityWindow: - """The window an approver is offered before editing anything. - - Both ends are today: the announcement starts the day it is approved, and - the end is a deliberate floor rather than a guess - an approver who wants - the catalog to stand longer has to say so. - """ - stamp = today() - return ValidityWindow(start_date=stamp, end_date=stamp) - - -def _parse_date(value: str, field: str) -> date: - try: - return datetime.strptime(value.strip(), DATE_FORMAT).date() - except (AttributeError, ValueError): - raise ValidityWindowError( - f"{field} must be a calendar date formatted as YYYY-MM-DD, got {value!r}." - ) from None - - -def parse_window( - start_date: Optional[str] = None, - end_date: Optional[str] = None, -) -> ValidityWindow: - """Validate an operator-supplied window, defaulting either end to today. - - Raises `ValidityWindowError` - never returns a half-valid window, so a - caller that gets a window back can publish it unchecked. - """ - default = default_window() - raw_start = start_date if (start_date or "").strip() else default.start_date - raw_end = end_date if (end_date or "").strip() else default.end_date - - start = _parse_date(raw_start, "start date") - end = _parse_date(raw_end, "end date") - - if end < start: - raise ValidityWindowError( - f"end date ({end.strftime(DATE_FORMAT)}) cannot be before " - f"start date ({start.strftime(DATE_FORMAT)})." - ) - - return ValidityWindow( - start_date=start.strftime(DATE_FORMAT), - end_date=end.strftime(DATE_FORMAT), - ) - - -def window_from_row( - start_date: Optional[str], - end_date: Optional[str], -) -> Optional[ValidityWindow]: - """Rebuild a stored window, or None when the document has none. - - None is the normal case for a document promoted before windows existed, and - means "announce without a validity" - not an error, and not a silent - fallback to today, which would expire an established catalog. - """ - if not (start_date or "").strip() or not (end_date or "").strip(): - return None - try: - return parse_window(start_date, end_date) - except ValidityWindowError: - # A row this module never wrote. Announcing no window beats announcing - # a malformed one the Discovery Service would reject the catalog for. - return None - - -def to_catalog_validity(window: ValidityWindow) -> dict: - """The `validity` object a catalog carries on the wire. - - Widened from dates to instants because the spec's catalog examples are - date-times; the end date is inclusive, so it runs to the last second of - that day rather than its midnight boundary. - """ - return { - "startDate": f"{window.start_date}T00:00:00Z", - "endDate": f"{window.end_date}T23:59:59Z", - } diff --git a/tests/test_activities.py b/tests/test_activities.py index c76a3d6..051cc1a 100644 --- a/tests/test_activities.py +++ b/tests/test_activities.py @@ -506,7 +506,7 @@ class FakeService: def __init__(self): pass - def publish(self, transaction_id, document_kind, workflow_id=None, validity=None): + def publish(self, transaction_id, document_kind, workflow_id=None): assert transaction_id == "txn-1" assert document_kind == "advisory" assert workflow_id == "wf-1" @@ -534,10 +534,6 @@ def fake_add_artifact(**kwargs): assert result == { "status": "published", "document_kind": "advisory", - # No approver window stored for this document - see - # TestPublishedLifetime for the case where one is. - "valid_from": None, - "valid_to": None, "transaction_id": "txn-1", "message_id": "msg-1", "result_status": "ACCEPTED", @@ -551,94 +547,6 @@ def fake_add_artifact(**kwargs): assert recorded["metadata"]["result_status"] == "ACCEPTED" assert recorded["metadata"]["errors"] == [] - @pytest.mark.unit - @pytest.mark.asyncio - async def test_hands_the_stored_window_to_the_service(self, monkeypatch): - """The window the approver set at the prod gate reaches the publish. - - Read here rather than passed as an activity argument, so workflows - already in flight keep working - that is exactly what makes this worth - a test of its own. - """ - import pipeline.activities as activities - import pipeline.db as db - from pipeline.network_validity import ValidityWindow - - seen = {} - - class FakeService: - def publish(self, transaction_id, document_kind, workflow_id=None, validity=None): - seen["validity"] = validity - return { - "skipped": True, - "envelope": None, - "status_code": None, - "response_body": None, - "result_status": None, - "errors": [], - } - - monkeypatch.setattr(activities, "DiscoveryPublishService", FakeService) - monkeypatch.setattr( - activities, - "_upload_file_to_minio", - lambda *a, **k: ("minio://documents/network_publish_skipped.json", 45, "application/json"), - ) - monkeypatch.setattr( - db, - "get_document", - lambda workflow_id: { - "document_kind": "advisory", - "network_valid_from": "2026-09-16", - "network_valid_to": "2026-12-31", - }, - ) - monkeypatch.setattr(db, "get_latest_document_job", lambda workflow_id: None) - monkeypatch.setattr(db, "add_document_artifact", lambda **kwargs: None) - - result = await activities.publish_catalog_to_network("wf-v", "txn-v") - - assert seen["validity"] == ValidityWindow( - start_date="2026-09-16", end_date="2026-12-31" - ) - assert result["valid_from"] == "2026-09-16" - assert result["valid_to"] == "2026-12-31" - - @pytest.mark.unit - @pytest.mark.asyncio - async def test_a_document_with_no_window_publishes_without_one(self, monkeypatch): - # Promoted before approvers named a lifetime: still publishes. - import pipeline.activities as activities - import pipeline.db as db - - seen = {} - - class FakeService: - def publish(self, transaction_id, document_kind, workflow_id=None, validity=None): - seen["validity"] = validity - return { - "skipped": True, - "envelope": None, - "status_code": None, - "response_body": None, - "result_status": None, - "errors": [], - } - - monkeypatch.setattr(activities, "DiscoveryPublishService", FakeService) - monkeypatch.setattr( - activities, - "_upload_file_to_minio", - lambda *a, **k: ("minio://documents/network_publish_skipped.json", 45, "application/json"), - ) - monkeypatch.setattr(db, "get_document", lambda workflow_id: {"document_kind": "advisory"}) - monkeypatch.setattr(db, "get_latest_document_job", lambda workflow_id: None) - monkeypatch.setattr(db, "add_document_artifact", lambda **kwargs: None) - - await activities.publish_catalog_to_network("wf-none", "txn-none") - - assert seen["validity"] is None - @pytest.mark.unit @pytest.mark.asyncio async def test_publish_failure_propagates_and_records_nothing(self, monkeypatch): @@ -646,7 +554,7 @@ async def test_publish_failure_propagates_and_records_nothing(self, monkeypatch) import pipeline.db as db class FakeService: - def publish(self, transaction_id, document_kind, workflow_id=None, validity=None): + def publish(self, transaction_id, document_kind, workflow_id=None): raise RuntimeError("discovery service unreachable") monkeypatch.setattr(activities, "DiscoveryPublishService", FakeService) @@ -668,7 +576,7 @@ async def test_unmapped_kind_records_a_skipped_artifact(self, monkeypatch): import pipeline.db as db class FakeService: - def publish(self, transaction_id, document_kind, workflow_id=None, validity=None): + def publish(self, transaction_id, document_kind, workflow_id=None): return { "skipped": True, "envelope": None, @@ -695,8 +603,6 @@ def publish(self, transaction_id, document_kind, workflow_id=None, validity=None assert result == { "status": "skipped", "document_kind": "document", - "valid_from": None, - "valid_to": None, "transaction_id": "txn-2", "message_id": None, "result_status": None, @@ -715,7 +621,7 @@ async def test_non_rejected_results_complete_the_stage(self, monkeypatch, result import pipeline.db as db class FakeService: - def publish(self, transaction_id, document_kind, workflow_id=None, validity=None): + def publish(self, transaction_id, document_kind, workflow_id=None): return { "skipped": False, "envelope": {"context": {"messageId": "msg-3"}}, @@ -749,7 +655,7 @@ async def test_rejected_result_records_the_errors_then_fails(self, monkeypatch): errors = [{"code": "SCH_SCHEMA_VALIDATION_FAILED", "message": "topics is required"}] class FakeService: - def publish(self, transaction_id, document_kind, workflow_id=None, validity=None): + def publish(self, transaction_id, document_kind, workflow_id=None): return { "skipped": False, "envelope": {"context": {"messageId": "msg-4"}}, diff --git a/tests/test_api.py b/tests/test_api.py index d77bcd5..d08f47b 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -109,11 +109,11 @@ def test_get_document_not_found(self, test_client): assert response.status_code == 404 -class TestProdApprovalLifetime: - """The prod approver names the lifetime of the network announcement. +class TestProdApproval: + """Promoting to PROD takes no body. - The window is validated and stored before anything is promoted, so a - rejected window must leave the document exactly where it was. + It used to carry an announcement window; these pin that the gate still + works without one and that no validity leaks back into the contract. """ def _document_at_the_gate(self, db_connection, workflow_id): @@ -127,100 +127,71 @@ def _document_at_the_gate(self, db_connection, workflow_id): @pytest.mark.api @pytest.mark.unit - def test_stores_the_approvers_window(self, test_client, db_connection): - from pipeline.network_validity import today - - workflow_id = "prod-lifetime-001" - self._document_at_the_gate(db_connection, workflow_id) - - response = test_client.post( - f"/documents/{workflow_id}/approve-prod", - json={"network_valid_to": "2099-12-31"}, - ) - - assert response.status_code == 200 - assert response.json()["network_valid_from"] == today() - assert response.json()["network_valid_to"] == "2099-12-31" - stored = db_connection.get_document(workflow_id) - assert stored["network_valid_from"] == today() - assert stored["network_valid_to"] == "2099-12-31" - - @pytest.mark.api - @pytest.mark.unit - def test_a_body_less_approval_defaults_both_ends_to_today(self, test_client, db_connection): - from pipeline.network_validity import today - - workflow_id = "prod-lifetime-002" + def test_a_body_less_approval_is_accepted(self, test_client, db_connection): + workflow_id = "prod-approval-001" self._document_at_the_gate(db_connection, workflow_id) response = test_client.post(f"/documents/{workflow_id}/approve-prod") assert response.status_code == 200 - stored = db_connection.get_document(workflow_id) - assert stored["network_valid_from"] == today() - assert stored["network_valid_to"] == today() + assert response.json()["approved"] == "prod" + assert response.json()["next_stage"] == "ingesting_prod" @pytest.mark.api @pytest.mark.unit - def test_the_window_is_surfaced_on_the_document(self, test_client, db_connection): - workflow_id = "prod-lifetime-003" + def test_the_response_carries_no_validity(self, test_client, db_connection): + workflow_id = "prod-approval-002" self._document_at_the_gate(db_connection, workflow_id) - test_client.post( - f"/documents/{workflow_id}/approve-prod", - json={"network_valid_to": "2099-12-31"}, - ) - doc = test_client.get(f"/documents/{workflow_id}").json() + body = test_client.post(f"/documents/{workflow_id}/approve-prod").json() - assert doc["network_valid_to"] == "2099-12-31" + assert not [key for key in body if "valid" in key] @pytest.mark.api @pytest.mark.unit - def test_rejects_an_end_before_the_start(self, test_client, db_connection): - workflow_id = "prod-lifetime-004" + def test_an_unexpected_body_does_not_break_the_gate(self, test_client, db_connection): + # A caller still sending the old window must not get a 422 — the + # endpoint simply has no body to bind any more. + workflow_id = "prod-approval-003" self._document_at_the_gate(db_connection, workflow_id) response = test_client.post( f"/documents/{workflow_id}/approve-prod", - json={"network_valid_from": "2026-09-16", "network_valid_to": "2026-09-15"}, + json={"network_valid_from": "2026-09-22", "network_valid_to": "2099-12-31"}, ) - assert response.status_code == 400 - assert "cannot be before" in response.json()["detail"] + assert response.status_code == 200 @pytest.mark.api @pytest.mark.unit - def test_rejects_a_malformed_date(self, test_client, db_connection): - workflow_id = "prod-lifetime-005" + def test_the_document_carries_no_validity_window(self, test_client, db_connection): + workflow_id = "prod-approval-004" self._document_at_the_gate(db_connection, workflow_id) + test_client.post(f"/documents/{workflow_id}/approve-prod") - response = test_client.post( - f"/documents/{workflow_id}/approve-prod", - json={"network_valid_to": "31-12-2099"}, - ) + doc = test_client.get(f"/documents/{workflow_id}").json() - assert response.status_code == 400 - assert "YYYY-MM-DD" in response.json()["detail"] + assert "network_valid_from" not in doc + assert "network_valid_to" not in doc + # Document Validity is a different thing and is still there. + assert doc["valid_from"] @pytest.mark.api @pytest.mark.unit - def test_a_rejected_window_stores_nothing_and_promotes_nothing( - self, test_client, db_connection, mock_temporal_client - ): - workflow_id = "prod-lifetime-006" - self._document_at_the_gate(db_connection, workflow_id) - mock_temporal_client.start_workflow.reset_mock() - - test_client.post( - f"/documents/{workflow_id}/approve-prod", - json={"network_valid_to": "not-a-date"}, + def test_the_wrong_stage_is_still_refused(self, test_client, db_connection): + workflow_id = "prod-approval-005" + db_connection.upsert_document( + workflow_id=workflow_id, + document_id=f"doc-{workflow_id}", + filename="test.pdf", + filepath="/app/books/test.pdf", + stage="chunk_review", ) - stored = db_connection.get_document(workflow_id) - assert stored["network_valid_from"] is None - assert stored["network_valid_to"] is None - assert stored["stage"] == "approval_for_prod" - mock_temporal_client.start_workflow.assert_not_called() + response = test_client.post(f"/documents/{workflow_id}/approve-prod") + + assert response.status_code == 400 + assert "chunk_review" in response.json()["detail"] class TestDocumentValidityEndpoints: @@ -342,8 +313,8 @@ def test_rejects_a_malformed_date(self, test_client, db_connection): @pytest.mark.api @pytest.mark.unit def test_a_rejected_period_stores_nothing(self, test_client, db_connection): - # Same contract the prod gate has: a 400 leaves the document as it was, - # kind included, rather than half-applying the PATCH. + # A 400 leaves the document as it was, kind included, rather than + # half-applying the PATCH. workflow_id = "validity-007" self._uploaded_document(db_connection, workflow_id) db_connection.set_document_validity(workflow_id, "2026-01-01", "2026-06-30") diff --git a/tests/test_catalog_builder.py b/tests/test_catalog_builder.py index 577fab4..24fbfa4 100644 --- a/tests/test_catalog_builder.py +++ b/tests/test_catalog_builder.py @@ -13,7 +13,6 @@ SCHEMES_CATALOG_ID, SCHEMES_RESOURCE_ID, ) -from pipeline.network_validity import ValidityWindow # Forbidden by the KnowledgeAdvisory schema under informationMode OnDemand. # Both advisory and scheme resources use this schema. @@ -132,43 +131,6 @@ def test_returns_none_so_the_caller_skips_publishing(self, kind): assert build_catalog(kind) is None -class TestValidityWindow: - @pytest.mark.unit - @pytest.mark.parametrize("kind", ["advisory", "scheme"]) - def test_omitted_when_the_document_has_no_window(self, kind): - # Documents promoted before approvers named a window: MERGE leaves an - # existing announcement's lifetime untouched rather than clearing it. - assert "validity" not in build_catalog(kind) - - @pytest.mark.unit - @pytest.mark.parametrize("kind", ["advisory", "scheme"]) - def test_lands_on_the_catalog_when_the_approver_set_one(self, kind): - catalog = build_catalog( - kind, ValidityWindow(start_date="2026-09-16", end_date="2026-12-31") - ) - - assert catalog["validity"] == { - "startDate": "2026-09-16T00:00:00Z", - "endDate": "2026-12-31T23:59:59Z", - } - - @pytest.mark.unit - def test_never_lands_on_resource_attributes(self): - # `validity` is absent from the OnDemand KnowledgeAdvisory profile - # (ADR 0003) — the catalog is the only legal home for it. - catalog = build_catalog( - "advisory", ValidityWindow(start_date="2026-09-16", end_date="2026-12-31") - ) - - assert "validity" not in catalog["resources"][0]["resourceAttributes"] - - @pytest.mark.unit - def test_an_unmapped_kind_still_publishes_nothing(self): - assert build_catalog( - "video", ValidityWindow(start_date="2026-09-16", end_date="2026-12-31") - ) is None - - class TestBuilderIsolation: @pytest.mark.unit def test_mutating_a_built_catalog_does_not_leak_into_the_next(self): diff --git a/tests/test_discovery_publish_service.py b/tests/test_discovery_publish_service.py index bd685cd..30052a9 100644 --- a/tests/test_discovery_publish_service.py +++ b/tests/test_discovery_publish_service.py @@ -439,62 +439,3 @@ def test_skipped_publish_logs_no_request(self, monkeypatch, caplog): messages = [r.getMessage() for r in caplog.records] assert any("network_publish_skipped=True" in m for m in messages) assert not any("network_publish_url=" in m for m in messages) - - -class TestPublishedValidityWindow: - """The lifetime the prod approver set travels on the catalog it publishes.""" - - @pytest.mark.unit - def test_envelope_carries_the_approvers_window(self, monkeypatch): - from pipeline.network_validity import ValidityWindow - - service = _service(monkeypatch) - client = _client_returning( - _on_publish_response("cat-oan-knowledge-provider-advisories", "ACCEPTED") - ) - - with patch("pipeline.discovery_publish_service.httpx.Client", return_value=client): - result = service.publish( - transaction_id="txn-validity", - document_kind="advisory", - validity=ValidityWindow(start_date="2026-09-16", end_date="2026-12-31"), - ) - - catalog = client.post.call_args.kwargs["json"]["message"]["catalogs"][0] - assert catalog["validity"] == { - "startDate": "2026-09-16T00:00:00Z", - "endDate": "2026-12-31T23:59:59Z", - } - # The recorded envelope is what an operator inspects after the fact. - assert result["envelope"]["message"]["catalogs"][0]["validity"] == catalog["validity"] - - @pytest.mark.unit - def test_omits_validity_when_the_document_has_none(self, monkeypatch): - # Pre-existing behaviour for documents promoted before windows existed. - service = _service(monkeypatch) - client = _client_returning( - _on_publish_response("cat-oan-knowledge-provider-advisories", "ACCEPTED") - ) - - with patch("pipeline.discovery_publish_service.httpx.Client", return_value=client): - service.publish(transaction_id="txn-none", document_kind="advisory") - - catalog = client.post.call_args.kwargs["json"]["message"]["catalogs"][0] - assert "validity" not in catalog - - @pytest.mark.unit - def test_a_window_does_not_make_an_unmapped_kind_publishable(self, monkeypatch): - from pipeline.network_validity import ValidityWindow - - service = _service(monkeypatch) - client = _client_returning(MagicMock()) - - with patch("pipeline.discovery_publish_service.httpx.Client", return_value=client): - result = service.publish( - transaction_id="txn-skip", - document_kind="video", - validity=ValidityWindow(start_date="2026-09-16", end_date="2026-12-31"), - ) - - assert result["skipped"] is True - client.post.assert_not_called() diff --git a/tests/test_document_repository.py b/tests/test_document_repository.py index da5220a..efedd29 100644 --- a/tests/test_document_repository.py +++ b/tests/test_document_repository.py @@ -9,7 +9,6 @@ from pipeline.document_repository import DocumentRepository from pipeline.document_validity import ValidityPeriod -from pipeline.network_validity import ValidityWindow class FakeDb: @@ -58,31 +57,6 @@ def test_passes_the_workflow_id_through(self): assert fake.get_document_called_with == "wf-42" -class TestGetNetworkValidity: - @pytest.mark.unit - def test_rebuilds_the_stored_window(self): - repo = DocumentRepository( - FakeDb(document={"network_valid_from": "2026-09-16", "network_valid_to": "2026-12-31"}) - ) - - assert repo.get_network_validity("wf-1") == ValidityWindow( - start_date="2026-09-16", end_date="2026-12-31" - ) - - @pytest.mark.unit - def test_returns_none_for_a_missing_document(self): - assert DocumentRepository(FakeDb(document=None)).get_network_validity("wf-nope") is None - - @pytest.mark.unit - def test_returns_none_when_no_approver_set_a_window(self): - # Documents promoted before the approval gate collected one. - repo = DocumentRepository( - FakeDb(document={"network_valid_from": None, "network_valid_to": None}) - ) - - assert repo.get_network_validity("wf-1") is None - - class TestAgainstRealSqlite: """Guards against db.py drifting out from under the repository.""" @@ -104,21 +78,6 @@ def test_unclassified_document_reads_as_the_column_default(self, db_connection, def test_missing_document_reads_as_none(self, db_connection): assert DocumentRepository().get_document_kind("no-such-workflow") is None - @pytest.mark.unit - @pytest.mark.db - def test_reads_a_window_written_through_the_db_layer(self, db_connection, sample_document): - workflow_id = sample_document["workflow_id"] - db_connection.set_network_validity(workflow_id, "2026-09-16", "2026-12-31") - - assert DocumentRepository().get_network_validity(workflow_id) == ValidityWindow( - start_date="2026-09-16", end_date="2026-12-31" - ) - - @pytest.mark.unit - @pytest.mark.db - def test_an_unapproved_document_has_no_window(self, db_connection, sample_document): - assert DocumentRepository().get_network_validity(sample_document["workflow_id"]) is None - class TestGetValidity: @pytest.mark.unit diff --git a/tests/test_network_validity.py b/tests/test_network_validity.py deleted file mode 100644 index b1f9cf4..0000000 --- a/tests/test_network_validity.py +++ /dev/null @@ -1,118 +0,0 @@ -"""Unit tests for the published catalog's validity window.""" - -from datetime import date, timedelta - -import pytest - -from pipeline.network_validity import ( - ValidityWindow, - ValidityWindowError, - default_window, - parse_window, - to_catalog_validity, - today, - window_from_row, -) - -TODAY = date.today().isoformat() -TOMORROW = (date.today() + timedelta(days=1)).isoformat() -YESTERDAY = (date.today() - timedelta(days=1)).isoformat() - - -class TestDefaultWindow: - @pytest.mark.unit - def test_both_ends_are_today(self): - # The approver is offered today/today and edits the end from there. - assert default_window() == ValidityWindow(start_date=TODAY, end_date=TODAY) - - @pytest.mark.unit - def test_today_is_the_stored_form(self): - assert today() == TODAY - - -class TestParseWindow: - @pytest.mark.unit - def test_defaults_both_ends_to_today(self): - assert parse_window() == ValidityWindow(start_date=TODAY, end_date=TODAY) - - @pytest.mark.unit - @pytest.mark.parametrize("blank", [None, "", " "]) - def test_defaults_a_blank_end_to_today(self, blank): - assert parse_window(TODAY, blank).end_date == TODAY - - @pytest.mark.unit - def test_keeps_an_approver_widened_end(self): - window = parse_window(TODAY, TOMORROW) - - assert window == ValidityWindow(start_date=TODAY, end_date=TOMORROW) - - @pytest.mark.unit - def test_same_day_window_is_legal(self): - # The default itself — an end equal to the start must not be rejected. - assert parse_window(TODAY, TODAY).end_date == TODAY - - @pytest.mark.unit - def test_trims_surrounding_whitespace(self): - assert parse_window(f" {TODAY} ", f" {TOMORROW} ").start_date == TODAY - - @pytest.mark.unit - def test_rejects_an_end_before_the_start(self): - with pytest.raises(ValidityWindowError, match="cannot be before"): - parse_window(TODAY, YESTERDAY) - - @pytest.mark.unit - @pytest.mark.parametrize( - "bad", - ["15-09-2026", "2026/09/15", "2026-13-01", "2026-02-30", "next tuesday", "2026-09-15T00:00:00Z"], - ) - def test_rejects_anything_that_is_not_a_calendar_date(self, bad): - with pytest.raises(ValidityWindowError, match="YYYY-MM-DD"): - parse_window(TODAY, bad) - - @pytest.mark.unit - def test_names_which_end_was_wrong(self): - with pytest.raises(ValidityWindowError, match="start date"): - parse_window("not-a-date", TODAY) - - -class TestWindowFromRow: - @pytest.mark.unit - def test_rebuilds_a_stored_window(self): - assert window_from_row(TODAY, TOMORROW) == ValidityWindow( - start_date=TODAY, end_date=TOMORROW - ) - - @pytest.mark.unit - @pytest.mark.parametrize( - "start,end", - [(None, None), (TODAY, None), (None, TOMORROW), ("", ""), (" ", TOMORROW)], - ) - def test_a_document_with_no_window_announces_without_one(self, start, end): - # Documents promoted before approvers named a window: not an error, and - # deliberately not defaulted to today, which would expire the catalog. - assert window_from_row(start, end) is None - - @pytest.mark.unit - def test_a_malformed_row_announces_without_a_window(self): - # Better an announcement with no validity than one the Discovery - # Service rejects the whole catalog over. - assert window_from_row("15-09-2026", TOMORROW) is None - assert window_from_row(TOMORROW, TODAY) is None - - -class TestToCatalogValidity: - @pytest.mark.unit - def test_widens_dates_to_the_instants_the_spec_examples_use(self): - validity = to_catalog_validity(ValidityWindow(start_date="2026-09-16", end_date="2026-12-31")) - - assert validity == { - "startDate": "2026-09-16T00:00:00Z", - "endDate": "2026-12-31T23:59:59Z", - } - - @pytest.mark.unit - def test_end_date_is_inclusive_to_the_last_second_of_that_day(self): - # A same-day window must still be a non-empty span. - validity = to_catalog_validity(ValidityWindow(start_date="2026-09-16", end_date="2026-09-16")) - - assert validity["startDate"] < validity["endDate"] diff --git a/ui/src/views/DocumentOpsView.jsx b/ui/src/views/DocumentOpsView.jsx index 2808ad8..c2b3acb 100644 --- a/ui/src/views/DocumentOpsView.jsx +++ b/ui/src/views/DocumentOpsView.jsx @@ -298,54 +298,6 @@ function todayISODate() { return `${now.getFullYear()}-${month}-${day}` } -/** - * Why a bad window is blocked rather than silently corrected: the approver is - * the only person who knows how long the announcement should stand, so a bad - * window is a question for them, not something to round into shape. Mirrors the - * server-side check in pipeline/network_validity.parse_window. - */ -function validateLifetime(startDate, endDate) { - if (!startDate || !endDate) return 'Set both a start and an end date.' - if (endDate < startDate) return 'End date cannot be before the start date.' - return '' -} - -/** - * Lifetime the super admin gives the network announcement this promotion leads - * to. Start date is the approval date and not editable - the announcement - * begins when it is published. End date opens at the same day and is the one - * thing the approver sets. - */ -function NetworkLifetimePanel({ startDate, endDate, onEndDateChange, error, disabled }) { - return ( -
-

- Announcement lifetime - published to the discovery service -

-
-
- Start date (approval date) - - {startDate} - -
-
- End date - onEndDateChange(e.target.value)} - /> -
-
- {error ?

{error}

: null} -
- ) -} - // Which permission each mutating action requires. Approvals / edits are // review; anything that re-runs pipeline stages or touches the index is pipeline. const ACTION_PERMISSION = { @@ -458,10 +410,6 @@ export default function DocumentOpsView() { const [highlightedChunk, setHighlightedChunk] = useState(null) const [panelLoading, setPanelLoading] = useState({}) const [actionPending, setActionPending] = useState(null) - // Start date is fixed at the approval date; only the end date is the - // approver's to move. Both re-seed per document (see the reset effect below). - const [lifetimeStart, setLifetimeStart] = useState(todayISODate) - const [lifetimeEnd, setLifetimeEnd] = useState(todayISODate) const requestIdRef = useRef(0) const attemptedPanelsRef = useRef({}) const activeTabRef = useRef(activeTab) @@ -720,10 +668,6 @@ export default function DocumentOpsView() { setPanelErrors({}) setPanelLoading({}) setMessage('') - // Re-approving carries no memory of the previous document's window: the - // start date is always today, and the end date opens there again. - setLifetimeStart(todayISODate()) - setLifetimeEnd(todayISODate()) load({ soft: false }) }, [workflowId, load]) @@ -841,18 +785,6 @@ export default function DocumentOpsView() { return } else if (action === 'restore_document') { await fetchJson(`/documents/${workflowId}/restore`, { method: 'POST' }) - } else if (action === 'approve_prod') { - // The window travels with the approval itself: the API validates and - // stores it before it signals anything, so a rejected window leaves the - // document parked at the gate rather than promoted-but-unannounced. - await fetchJson(`/documents/${workflowId}/approve-prod`, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ - network_valid_from: lifetimeStart, - network_valid_to: lifetimeEnd, - }), - }) } else { await fetchJson(`/documents/${workflowId}/${action.replace(/_/g, '-')}`, { method: 'POST' }) } @@ -918,9 +850,6 @@ export default function DocumentOpsView() { (doc?.document_kind && doc.document_kind !== 'document') || doc?.scheme_code || doc?.scheme_name ) const ingestBlockedByClassification = doc?.stage === 'ready_for_ingestion' && !isDocClassified - const showsLifetimePanel = visibleActions.includes('approve_prod') - const lifetimeError = showsLifetimePanel ? validateLifetime(lifetimeStart, lifetimeEnd) : '' - const prodBlockedByLifetime = showsLifetimePanel && Boolean(lifetimeError) const sortedPages = useMemo(() => [...pages].sort((a, b) => a.page_number - b.page_number), [pages]) const reviewedPages = useMemo(() => pages.filter(p => p.is_reviewed).length, [pages]) const reviewedChunks = useMemo(() => chunks.filter(c => c.is_reviewed).length, [chunks]) @@ -1080,20 +1009,17 @@ export default function DocumentOpsView() {
{visibleActions.slice(0, 4).map(action => { const blockedByClassification = action === 'approve_ingestion' && ingestBlockedByClassification - const blockedByLifetime = action === 'approve_prod' && prodBlockedByLifetime return ( ) })} @@ -1131,16 +1055,6 @@ export default function DocumentOpsView() { /> )} - {showsLifetimePanel && ( - - )} - {message ? ( ) : null} From 52595c0bbbb7b716958aac221a7389dcf86582d2 Mon Sep 17 00:00:00 2001 From: shruti0025 Date: Wed, 23 Sep 2026 08:40:33 +0530 Subject: [PATCH 04/11] refactor: move the validity default out of the db layer [#111] db.upsert_document computed the default period itself, putting domain knowledge in a layer that is meant to hold none. The upload endpoints now compute it and pass it down; db.py stores the dates verbatim. Also parses created_at with the date library instead of slicing its first ten characters, so a timestamp in an unexpected shape reports "unreadable" rather than whatever those characters happen to spell. --- AGENTS.md | 2 +- .../0006-document-validity-filters-search.md | 2 +- pipeline/activities.py | 6 +- pipeline/api.py | 10 +++ pipeline/db.py | 16 ++--- pipeline/document_validity.py | 48 +++++++++++++++ scripts/verify_document_validity_e2e.py | 7 ++- tests/test_api.py | 31 ++++++++-- tests/test_document_repository.py | 29 ++++++--- tests/test_document_validity.py | 61 +++++++++++++++++++ 10 files changed, 179 insertions(+), 33 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f588bdb..34bee03 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,7 +62,7 @@ Core modules (all under `pipeline/`): - `document_repository.py` — `DocumentRepository`, a domain layer over `db.py` for document reads; `db.py` stays one-function-per-query with no domain knowledge - `network_constants.py` — fixed values sent on the network (catalog/resource ids, topics, languages, Beckn version). The file v2 edits when the AI layer starts deriving them - `catalog_builder.py` — pure builder mapping a document's knowledge kind (`advisory`/`scheme`) to the single `OnDemand` Beckn catalog announced for that kind; returns `None` for any other kind. No env, no I/O -- `document_validity.py` — pure module owning Document Validity: the period a document's chunks answer searches in, the upload-day/+1-year default, and the clock injection point. Shared by `db.py` (stamping on upload), `scheme_catalog.py` (validating a reviewer's edit), `activities.py` (stamping chunk payloads) and `vector_store/qdrant_store.py` (filtering at search time) +- `document_validity.py` — pure module owning Document Validity: the period a document's chunks answer searches in, the upload-day/+1-year default, and the clock injection point. Shared by `api.py` (stamping the default at upload), `scheme_catalog.py` (validating a reviewer's edit), `activities.py` (stamping chunk payloads) and `vector_store/qdrant_store.py` (filtering at search time). `db.py` stores the dates it is handed and defaults nothing - `discovery_publish_service.py` — `DiscoveryPublishService`: owns the Publish to Network env vars and the HTTP call. Deliberately Temporal-free - `models.py` — Pydantic models, including `DocumentStage` enum and `PIPELINE_STAGES` (the stepper-UI stage list) - `config.py` — `Config` dataclass reading env vars, with defaults diff --git a/docs/ADR/0006-document-validity-filters-search.md b/docs/ADR/0006-document-validity-filters-search.md index 1a112ed..f0afbe6 100644 --- a/docs/ADR/0006-document-validity-filters-search.md +++ b/docs/ADR/0006-document-validity-filters-search.md @@ -6,7 +6,7 @@ A short-lived predecessor (ADR 0005, since removed along with its code) put a *c Decision: - **A document-level concept, owned in one place.** `documents.valid_from` / `documents.valid_to`, stamped onto every chunk's Qdrant payload as `start_date` / `end_date`, owned by a pure module `pipeline/document_validity.py` that holds the rules so the API, the ingest activity and the vector store never re-derive them. -- **Defaulted on upload, not asked for.** `db.upsert_document` stamps the upload day and one year out on INSERT. An uploader is not asked a question they cannot answer, and no document can reach the index without a period. A year is the business's review cadence, not a technical limit. +- **Defaulted on upload, not asked for.** The upload endpoints stamp the upload day and one year out. An uploader is not asked a question they cannot answer, and no document reaches the index without a period. A year is the business's review cadence, not a technical limit. The default is computed in `api.py` and passed down: `db.upsert_document` stores the dates verbatim on INSERT and has no opinion on what they should be, keeping `db.py` free of domain knowledge as the rest of that layer is. A row created around the upload path (a script, a backfill) therefore has no period, which search reads as "no stated period" — the same as the pre-validity corpus. - **Confirmed or moved by the reviewer, in the form that already exists.** The dates sit beside the Knowledge Kind selector and travel on the same `PATCH /documents/{id}/scheme-metadata`. Choosing the period is part of classifying a document, not a separate errand, and the reviewer sees real prefilled dates rather than an empty field. Either end may be sent alone; the other is taken from what is stored, so moving the end date does not require restating the start. - **Validated like a bad kind, not like a bad type.** `document_validity.parse_period` raises, `apply_scheme_metadata` lets it through as a `ValueError`, and the endpoint answers 400 — the same shape a bad `document_kind` gets, not a Pydantic 422. A rejected period stores nothing, kind included. - **Both ends inclusive.** A document uploaded this morning starts today and must answer this afternoon; an end date names the last day it answers rather than the first day it does not. The requirement was written as "today greater than start and less than end", but exclusive bounds would hide a document on its own first day, which is the default every document gets. diff --git a/pipeline/activities.py b/pipeline/activities.py index 7623f20..fc6bd30 100644 --- a/pipeline/activities.py +++ b/pipeline/activities.py @@ -773,11 +773,7 @@ def _validity_fields_from_doc(doc: dict | None) -> dict: doc.get("valid_from"), doc.get("valid_to") ) if period is None: - upload_day = str(doc.get("created_at") or "")[:10] - try: - period = document_validity.period_from_upload_date(upload_day) - except document_validity.DocumentValidityError: - period = document_validity.default_period() + period = document_validity.period_from_upload_timestamp(doc.get("created_at")) return {"valid_from": period.start_date, "valid_to": period.end_date} diff --git a/pipeline/api.py b/pipeline/api.py index 5b26cf1..3075c30 100644 --- a/pipeline/api.py +++ b/pipeline/api.py @@ -1009,6 +1009,10 @@ async def start_document_workflow( # Save to SQLite for visibility during processing actor = _actor_from_user(user) + # The period this document is searchable in starts here, at the one moment + # that knows the upload day. A reviewer confirms or moves it later, on the + # form that sets the document type. + validity = document_validity.default_period() db.upsert_document( workflow_id=workflow_id, document_id=document_id, @@ -1024,6 +1028,8 @@ async def start_document_workflow( uploaded_by_username=actor.get("username") or None, uploaded_by_email=actor.get("email") or None, uploaded_by_roles=actor.get("roles_csv") or None, + valid_from=validity.start_date, + valid_to=validity.end_date, ) job_id = db.create_document_job( workflow_id=workflow_id, @@ -1180,6 +1186,8 @@ async def upload_and_process( # Save to SQLite for visibility during processing actor = _actor_from_user(user) + # See the manifest upload path above: the period starts at the upload. + validity = document_validity.default_period() db.upsert_document( workflow_id=workflow_id, document_id=document_id, @@ -1195,6 +1203,8 @@ async def upload_and_process( uploaded_by_username=actor.get("username") or None, uploaded_by_email=actor.get("email") or None, uploaded_by_roles=actor.get("roles_csv") or None, + valid_from=validity.start_date, + valid_to=validity.end_date, ) job_id = db.create_document_job( workflow_id=workflow_id, diff --git a/pipeline/db.py b/pipeline/db.py index fbf648c..30b3ff0 100644 --- a/pipeline/db.py +++ b/pipeline/db.py @@ -15,8 +15,6 @@ from threading import Lock from typing import Optional -from . import document_validity - # Database path - can be configured via environment DB_PATH = os.environ.get("DOCUMENT_DB_PATH", "/data/documents.db") @@ -686,16 +684,14 @@ def upsert_document( stamp alone so a re-upload / restart cannot silently reassign tenants. Uploader identity is set on INSERT, and filled on UPDATE only when currently null. - ``valid_from``/``valid_to`` are the document's validity period, likewise - INSERT-only: an uploader does not choose it (it defaults to the upload day - and a year out) and an approver's later edit must survive a restart that - re-registers the same workflow. + ``valid_from``/``valid_to`` are the document's validity period, stored as + given and likewise INSERT-only, so an approver's later edit survives a + restart that re-registers the same workflow. What the period should default + to is the caller's call (see ``pipeline/document_validity.py``); omitting + both stores NULL, which search reads as "no stated period". """ now = datetime.utcnow().isoformat() instance_value = (instance or os.environ.get("DEFAULT_INSTANCE") or "default").strip().lower() or "default" - default_period = document_validity.default_period() - valid_from_value = (valid_from or "").strip() or default_period.start_date - valid_to_value = (valid_to or "").strip() or default_period.end_date with _db_lock: with get_connection() as conn: @@ -777,7 +773,7 @@ def upsert_document( original_artifact_id, normalized_artifact_id, latest_job_id, instance_value, uploaded_by_user_id, uploaded_by_username, uploaded_by_email, uploaded_by_roles, - valid_from_value, valid_to_value, + valid_from, valid_to, )) conn.commit() diff --git a/pipeline/document_validity.py b/pipeline/document_validity.py index 7c58f41..a2cb3db 100644 --- a/pipeline/document_validity.py +++ b/pipeline/document_validity.py @@ -98,6 +98,54 @@ def period_from_upload_date(upload_date: str) -> ValidityPeriod: return ValidityPeriod(start_date=stamp, end_date=add_years(stamp)) +def _parse_timestamp(value: Optional[str]) -> Optional[datetime]: + """Read a stored ISO-8601 timestamp, or None when it cannot be read. + + `db.py` writes `datetime.utcnow().isoformat()`, which round-trips through + `fromisoformat` exactly. The trailing-Z form is accepted too because rows + can come from other writers, and Python 3.10's `fromisoformat` rejects it. + """ + text = (value or "").strip() + if not text: + return None + if text.endswith(("Z", "z")): + text = text[:-1] + "+00:00" + try: + return datetime.fromisoformat(text) + except ValueError: + return None + + +def day_of(timestamp: Optional[str]) -> Optional[str]: + """The calendar day an ISO-8601 timestamp falls on, or None when unreadable. + + Parsed rather than sliced: a timestamp in an unexpected shape should come + back as "I cannot tell", not as whatever its first ten characters happen + to spell. + """ + parsed = _parse_timestamp(timestamp) + return None if parsed is None else parsed.date().strftime(DATE_FORMAT) + + +def period_from_upload_timestamp( + timestamp: Optional[str], + clock: Optional[Clock] = None, +) -> ValidityPeriod: + """The default period for a document that has none stored, anchored on the + day it was uploaded. + + Its own upload day is a truer start than today, which would silently extend + an old document's life by another year. A timestamp that cannot be read + falls back to today's default rather than raising: this runs on the ingest + path, where refusing to ingest over an unparseable audit column would be a + worse answer than giving the document a period starting now. + """ + upload_day = day_of(timestamp) + if upload_day is None: + return default_period(clock) + return period_from_upload_date(upload_day) + + def _parse_date(value: Optional[str], field: str) -> date: try: return datetime.strptime((value or "").strip(), DATE_FORMAT).date() diff --git a/scripts/verify_document_validity_e2e.py b/scripts/verify_document_validity_e2e.py index e53dd20..7fa8200 100644 --- a/scripts/verify_document_validity_e2e.py +++ b/scripts/verify_document_validity_e2e.py @@ -35,7 +35,7 @@ from datetime import datetime # noqa: E402 -from pipeline import db # noqa: E402 +from pipeline import db, document_validity # noqa: E402 from pipeline.activities import _prepare_records, _validity_fields_from_doc # noqa: E402 from pipeline.vector_store.qdrant_store import QdrantVectorStore # noqa: E402 @@ -59,6 +59,11 @@ def clock_at(stamp): def make_document(workflow_id, text, valid_from=None, valid_to=None, legacy=False): + # Stands in for the upload endpoint, which is what decides the period a new + # document starts with — db.py stores what it is given and defaults nothing. + if not legacy and valid_from is None and valid_to is None: + default = document_validity.default_period() + valid_from, valid_to = default.start_date, default.end_date db.upsert_document( workflow_id=workflow_id, document_id=workflow_id, diff --git a/tests/test_api.py b/tests/test_api.py index d08f47b..ecf1f5a 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -173,8 +173,8 @@ def test_the_document_carries_no_validity_window(self, test_client, db_connectio assert "network_valid_from" not in doc assert "network_valid_to" not in doc - # Document Validity is a different thing and is still there. - assert doc["valid_from"] + # Document Validity is a different thing and is still reported. + assert "valid_from" in doc @pytest.mark.api @pytest.mark.unit @@ -212,18 +212,37 @@ def _uploaded_document(self, db_connection, workflow_id): @pytest.mark.api @pytest.mark.unit - def test_a_new_document_is_valid_from_today_for_a_year(self, test_client, db_connection): + def test_upload_stamps_today_and_a_year_out(self, test_client, sample_pdf_content): + # The upload endpoint owns this default — db.py stores what it is given + # and has no opinion on what a period should be — so the guarantee is + # only real if it is asserted through the endpoint that grants it. from pipeline.document_validity import default_period - workflow_id = "validity-001" - self._uploaded_document(db_connection, workflow_id) expected = default_period() - doc = test_client.get(f"/documents/{workflow_id}").json() + response = test_client.post( + "/upload", + files={"file": ("validity-upload.pdf", sample_pdf_content, "application/pdf")}, + ) + assert response.status_code == 200 + doc = test_client.get(f"/documents/{response.json()['workflow_id']}").json() assert doc["valid_from"] == expected.start_date assert doc["valid_to"] == expected.end_date + @pytest.mark.api + @pytest.mark.unit + def test_a_document_stored_without_a_period_reports_none(self, test_client, db_connection): + # Rows written outside the upload path (scripts, backfills) carry no + # period, and the API surfaces that honestly rather than inventing one. + workflow_id = "validity-000" + self._uploaded_document(db_connection, workflow_id) + + doc = test_client.get(f"/documents/{workflow_id}").json() + + assert doc["valid_from"] is None + assert doc["valid_to"] is None + @pytest.mark.api @pytest.mark.unit def test_stores_the_reviewers_edit(self, test_client, db_connection): diff --git a/tests/test_document_repository.py b/tests/test_document_repository.py index efedd29..33a49c0 100644 --- a/tests/test_document_repository.py +++ b/tests/test_document_repository.py @@ -117,11 +117,10 @@ def test_reads_a_real_row_written_by_db(self, db_connection): ) @pytest.mark.unit - def test_a_freshly_uploaded_document_already_has_a_period(self, db_connection): - # Nothing asks for it on upload — db stamps the default so no document - # is ever ingested without one. - from datetime import date - + def test_a_row_written_without_a_period_reads_as_none(self, db_connection): + # db.py stores what it is given and defaults nothing — the upload + # endpoint decides the period (see TestDocumentValidityEndpoints in + # test_api.py). A row created around it simply has none. db_connection.upsert_document( workflow_id="wf-fresh", document_id="doc-fresh", @@ -129,8 +128,20 @@ def test_a_freshly_uploaded_document_already_has_a_period(self, db_connection): filepath="/books/b.pdf", ) - period = DocumentRepository(db_connection).get_validity("wf-fresh") + assert DocumentRepository(db_connection).get_validity("wf-fresh") is None - assert period is not None - assert period.start_date == date.today().isoformat() - assert period.is_active_on(date.today().isoformat()) + @pytest.mark.unit + @pytest.mark.db + def test_a_period_passed_to_the_db_layer_is_stored_verbatim(self, db_connection): + db_connection.upsert_document( + workflow_id="wf-stamped", + document_id="doc-stamped", + filename="c.pdf", + filepath="/books/c.pdf", + valid_from="2026-09-23", + valid_to="2027-09-23", + ) + + assert DocumentRepository(db_connection).get_validity("wf-stamped") == ValidityPeriod( + start_date="2026-09-23", end_date="2027-09-23" + ) diff --git a/tests/test_document_validity.py b/tests/test_document_validity.py index 2f9cc8b..8e177dd 100644 --- a/tests/test_document_validity.py +++ b/tests/test_document_validity.py @@ -13,10 +13,12 @@ DocumentValidityError, ValidityPeriod, add_years, + day_of, default_period, parse_period, period_from_row, period_from_upload_date, + period_from_upload_timestamp, system_clock, today, ) @@ -187,3 +189,62 @@ def test_returns_none_for_a_row_this_module_never_wrote(self): @pytest.mark.unit def test_returns_none_for_an_inverted_stored_period(self): assert period_from_row("2026-12-31", "2026-01-01") is None + + +class TestDayOf: + """Reading the calendar day out of a stored timestamp.""" + + @pytest.mark.unit + def test_reads_what_the_db_layer_writes(self): + # db.py stores datetime.utcnow().isoformat(). + assert day_of("2026-09-23T09:30:00.123456") == "2026-09-23" + + @pytest.mark.unit + @pytest.mark.parametrize( + "stamp", + [ + "2026-09-23T09:30:00", + "2026-09-23 09:30:00", + "2026-09-23T09:30:00+05:30", + "2026-09-23", + ], + ) + def test_reads_the_other_shapes_a_row_can_carry(self, stamp): + assert day_of(stamp) == "2026-09-23" + + @pytest.mark.unit + def test_reads_a_trailing_z(self): + # Python 3.10's fromisoformat rejects "Z"; a row can still hold one. + assert day_of("2026-09-23T09:30:00Z") == "2026-09-23" + + @pytest.mark.unit + @pytest.mark.parametrize("bad", [None, "", " ", "garbage", "2026-13-45", "23/09/2026"]) + def test_unreadable_timestamps_report_none(self, bad): + # The point of parsing rather than slicing: an unexpected shape comes + # back as "I cannot tell" instead of its first ten characters. + assert day_of(bad) is None + + @pytest.mark.unit + def test_does_not_slice_a_misleading_prefix(self): + # "23/09/2026" sliced to ten characters looks like a date and is not. + assert day_of("23/09/2026 09:30:00") is None + + +class TestPeriodFromUploadTimestamp: + @pytest.mark.unit + def test_anchors_on_the_day_the_timestamp_falls_on(self): + assert period_from_upload_timestamp("2024-05-01T09:30:00.123456") == ValidityPeriod( + start_date="2024-05-01", end_date="2025-05-01" + ) + + @pytest.mark.unit + @pytest.mark.parametrize("bad", [None, "", "garbage"]) + def test_falls_back_to_todays_default_when_unreadable(self, bad): + # On the ingest path: refusing to ingest over an unparseable audit + # column would be a worse answer than a period starting now. + assert period_from_upload_timestamp(bad, clock=FROZEN) == default_period(FROZEN) + + @pytest.mark.unit + def test_never_raises(self): + for value in (None, "", " ", "garbage", "2026-13-45", "0001-01-01T00:00:00"): + assert period_from_upload_timestamp(value, clock=FROZEN) is not None From 02e48bdbcfb7a80b0ad47dd7ceac2231819083c9 Mon Sep 17 00:00:00 2001 From: shruti0025 Date: Wed, 23 Sep 2026 09:52:41 +0530 Subject: [PATCH 05/11] fix: zero-pad dates so validity parses on glibc [#111] --- pipeline/document_validity.py | 44 ++++++++++++++++++++++++-------- tests/test_document_validity.py | 45 ++++++++++++++++++++++++++++++--- 2 files changed, 76 insertions(+), 13 deletions(-) diff --git a/pipeline/document_validity.py b/pipeline/document_validity.py index a2cb3db..bd09887 100644 --- a/pipeline/document_validity.py +++ b/pipeline/document_validity.py @@ -19,6 +19,11 @@ # Operators think in calendar dates, not instants - a period is stored, # exchanged and filtered as a plain `YYYY-MM-DD` day. +# +# Parsing only. Dates are rendered with `date.isoformat()`, which is the same +# format but zero-pads the year on every platform - glibc's strftime does not +# ("%Y" of year 1 is "1-01-01" on Linux and "0001-01-01" on macOS), and a date +# this module emitted would then fail the parse this module does. DATE_FORMAT = "%Y-%m-%d" # How long a freshly uploaded document is presumed to stay current. A year is @@ -60,7 +65,7 @@ def is_active_on(self, on_date: str) -> bool: def today(clock: Optional[Clock] = None) -> str: """Today's date in the stored form.""" - return (clock or system_clock)().date().strftime(DATE_FORMAT) + return (clock or system_clock)().date().isoformat() def add_years(stamp: str, years: int = DEFAULT_VALIDITY_YEARS) -> str: @@ -68,13 +73,25 @@ def add_years(stamp: str, years: int = DEFAULT_VALIDITY_YEARS) -> str: 29 February lands on 28 February in a non-leap year - the alternative (1 March) would push the period into the wrong month for no gain. + + A stamp near `date.max` cannot move forward at all; that comes back as a + `DocumentValidityError` rather than the raw `ValueError` the stdlib + raises, so every failure out of this module is the one kind callers + already handle. """ anchor = _parse_date(stamp, "date") + target_year = anchor.year + years try: - moved = anchor.replace(year=anchor.year + years) + moved = anchor.replace(year=target_year) except ValueError: - moved = anchor.replace(year=anchor.year + years, day=28) - return moved.strftime(DATE_FORMAT) + try: + moved = anchor.replace(year=target_year, day=28) + except ValueError: + raise DocumentValidityError( + f"{stamp} cannot move {years} year(s) forward: " + f"year {target_year} is outside the supported range." + ) from None + return moved.isoformat() def default_period(clock: Optional[Clock] = None) -> ValidityPeriod: @@ -94,7 +111,7 @@ def period_from_upload_date(upload_date: str) -> ValidityPeriod: upload day is a truer start than today, which would silently extend a two-year-old document's life by another year. """ - stamp = _parse_date(upload_date, "upload date").strftime(DATE_FORMAT) + stamp = _parse_date(upload_date, "upload date").isoformat() return ValidityPeriod(start_date=stamp, end_date=add_years(stamp)) @@ -124,7 +141,7 @@ def day_of(timestamp: Optional[str]) -> Optional[str]: to spell. """ parsed = _parse_timestamp(timestamp) - return None if parsed is None else parsed.date().strftime(DATE_FORMAT) + return None if parsed is None else parsed.date().isoformat() def period_from_upload_timestamp( @@ -143,7 +160,14 @@ def period_from_upload_timestamp( upload_day = day_of(timestamp) if upload_day is None: return default_period(clock) - return period_from_upload_date(upload_day) + try: + return period_from_upload_date(upload_day) + except DocumentValidityError: + # Belt and braces: `day_of` emits a form `period_from_upload_date` + # accepts, so this is unreachable today. It stays because the promise + # this function makes is "never raises", and a caller on the ingest + # path should not be the one to discover that drifted. + return default_period(clock) def _parse_date(value: Optional[str], field: str) -> date: @@ -169,18 +193,18 @@ def parse_period( """ raw_start = start_date if (start_date or "").strip() else today(clock) start = _parse_date(raw_start, "start date") - stamp = start.strftime(DATE_FORMAT) + stamp = start.isoformat() raw_end = end_date if (end_date or "").strip() else add_years(stamp) end = _parse_date(raw_end, "end date") if end < start: raise DocumentValidityError( - f"end date ({end.strftime(DATE_FORMAT)}) cannot be before " + f"end date ({end.isoformat()}) cannot be before " f"start date ({stamp})." ) - return ValidityPeriod(start_date=stamp, end_date=end.strftime(DATE_FORMAT)) + return ValidityPeriod(start_date=stamp, end_date=end.isoformat()) def period_from_row( diff --git a/tests/test_document_validity.py b/tests/test_document_validity.py index 8e177dd..3b23ede 100644 --- a/tests/test_document_validity.py +++ b/tests/test_document_validity.py @@ -66,6 +66,20 @@ def test_rejects_a_malformed_date(self): with pytest.raises(DocumentValidityError, match="YYYY-MM-DD"): add_years("22-09-2026") + @pytest.mark.unit + def test_zero_pads_a_year_below_1000(self): + # strftime("%Y") does not zero-pad on glibc — year 1 renders as + # "1-01-01" on Linux and "0001-01-01" on macOS — so a date this module + # emitted would fail the parse this module does, on Linux only. + assert add_years("0001-01-01") == "0002-01-01" + + @pytest.mark.unit + def test_a_date_that_cannot_move_forward_is_a_domain_error(self): + # date.max is year 9999; the stdlib raises a bare ValueError there, + # which callers handling DocumentValidityError would not catch. + with pytest.raises(DocumentValidityError, match="outside the supported range"): + add_years("9999-12-31") + class TestDefaultPeriod: @pytest.mark.unit @@ -224,6 +238,12 @@ def test_unreadable_timestamps_report_none(self, bad): # back as "I cannot tell" instead of its first ten characters. assert day_of(bad) is None + @pytest.mark.unit + def test_zero_pads_a_year_below_1000(self): + # Must round-trip back through this module's own parser; see + # TestAddYears.test_zero_pads_a_year_below_1000 for why it can't. + assert day_of("0001-01-01T00:00:00") == "0001-01-01" + @pytest.mark.unit def test_does_not_slice_a_misleading_prefix(self): # "23/09/2026" sliced to ten characters looks like a date and is not. @@ -245,6 +265,25 @@ def test_falls_back_to_todays_default_when_unreadable(self, bad): assert period_from_upload_timestamp(bad, clock=FROZEN) == default_period(FROZEN) @pytest.mark.unit - def test_never_raises(self): - for value in (None, "", " ", "garbage", "2026-13-45", "0001-01-01T00:00:00"): - assert period_from_upload_timestamp(value, clock=FROZEN) is not None + @pytest.mark.parametrize( + "value", + [ + None, + "", + " ", + "garbage", + "2026-13-45", + "0001-01-01T00:00:00", # below the year strftime zero-pads + "9999-12-31T23:59:59", # cannot move a year forward + ], + ) + def test_never_raises(self, value): + # This runs on the ingest path: whatever an audit column holds, it must + # come back as a period rather than stopping the document. + assert period_from_upload_timestamp(value, clock=FROZEN) is not None + + @pytest.mark.unit + def test_a_timestamp_past_the_range_falls_back_to_today(self): + assert period_from_upload_timestamp( + "9999-12-31T23:59:59", clock=FROZEN + ) == default_period(FROZEN) From 172b012862538b995e6d334ab702411c96dd9b00 Mon Sep 17 00:00:00 2001 From: shruti0025 Date: Fri, 25 Sep 2026 00:30:47 +0530 Subject: [PATCH 06/11] fix: rename-adr-file [#111] --- ...filters-search.md => 0005-document-validity-filters-search.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename docs/ADR/{0006-document-validity-filters-search.md => 0005-document-validity-filters-search.md} (100%) diff --git a/docs/ADR/0006-document-validity-filters-search.md b/docs/ADR/0005-document-validity-filters-search.md similarity index 100% rename from docs/ADR/0006-document-validity-filters-search.md rename to docs/ADR/0005-document-validity-filters-search.md From 882a66e25a8610d2d8497127b59878cebbb52925 Mon Sep 17 00:00:00 2001 From: shruti0025 Date: Fri, 25 Sep 2026 00:56:44 +0530 Subject: [PATCH 07/11] fix: remove-dead-code-method-get-validity [#111] --- pipeline/document_repository.py | 14 ----- tests/test_db.py | 85 +++++++++++++++++++++++++++++++ tests/test_document_repository.py | 69 ------------------------- 3 files changed, 85 insertions(+), 83 deletions(-) diff --git a/pipeline/document_repository.py b/pipeline/document_repository.py index c5fb758..9f2e2c3 100644 --- a/pipeline/document_repository.py +++ b/pipeline/document_repository.py @@ -9,7 +9,6 @@ from typing import Optional from . import db -from .document_validity import ValidityPeriod, period_from_row class DocumentRepository: @@ -29,16 +28,3 @@ def get_document_kind(self, workflow_id: str) -> Optional[str]: if not doc: return None return doc.get("document_kind") - - def get_validity(self, workflow_id: str) -> Optional[ValidityPeriod]: - """The period this document's chunks are searchable in, or None when - it has none. - - None is not an error: documents uploaded before validity existed have - no stored dates, and their chunks are searchable without restriction - (see `document_validity.period_from_row`). - """ - doc = self._db.get_document(workflow_id) - if not doc: - return None - return period_from_row(doc.get("valid_from"), doc.get("valid_to")) diff --git a/tests/test_db.py b/tests/test_db.py index 1afc318..b771611 100644 --- a/tests/test_db.py +++ b/tests/test_db.py @@ -245,6 +245,91 @@ def test_purge_document_is_idempotent(self, db_connection): assert db_connection.purge_document("never-existed") == {} +class TestDocumentValidityColumns: + """The validity period columns on `documents`. + + db.py stores the dates it is handed and defaults nothing — the upload + endpoint decides what a new document's period should be (see + TestDocumentValidityEndpoints in test_api.py). These pin the storage half + of that split. + """ + + @pytest.mark.db + @pytest.mark.unit + def test_a_period_is_stored_verbatim(self, db_connection): + db_connection.upsert_document( + workflow_id="wf-stamped", + document_id="doc-stamped", + filename="c.pdf", + filepath="/books/c.pdf", + valid_from="2026-09-23", + valid_to="2027-09-23", + ) + + doc = db_connection.get_document("wf-stamped") + + assert doc["valid_from"] == "2026-09-23" + assert doc["valid_to"] == "2027-09-23" + + @pytest.mark.db + @pytest.mark.unit + def test_a_document_created_without_a_period_stores_null(self, db_connection): + # No default invented here — that is the caller's call. + db_connection.upsert_document( + workflow_id="wf-fresh", + document_id="doc-fresh", + filename="b.pdf", + filepath="/books/b.pdf", + ) + + doc = db_connection.get_document("wf-fresh") + + assert doc["valid_from"] is None + assert doc["valid_to"] is None + + @pytest.mark.db + @pytest.mark.unit + def test_set_document_validity_round_trips(self, db_connection): + db_connection.upsert_document( + workflow_id="wf-validity", + document_id="doc-validity", + filename="a.pdf", + filepath="/books/a.pdf", + ) + + db_connection.set_document_validity("wf-validity", "2026-10-01", "2026-12-31") + + doc = db_connection.get_document("wf-validity") + assert doc["valid_from"] == "2026-10-01" + assert doc["valid_to"] == "2026-12-31" + + @pytest.mark.db + @pytest.mark.unit + def test_a_period_is_not_overwritten_by_a_later_upsert(self, db_connection): + # INSERT-only: an approver's edit must survive a restart that + # re-registers the same workflow. + db_connection.upsert_document( + workflow_id="wf-keep", + document_id="doc-keep", + filename="d.pdf", + filepath="/books/d.pdf", + valid_from="2026-01-01", + valid_to="2026-06-30", + ) + + db_connection.upsert_document( + workflow_id="wf-keep", + document_id="doc-keep", + filename="d.pdf", + filepath="/books/d.pdf", + stage="ocr_review", + ) + + doc = db_connection.get_document("wf-keep") + assert doc["valid_from"] == "2026-01-01" + assert doc["valid_to"] == "2026-06-30" + + class TestPageOperations: """Tests for page CRUD operations.""" diff --git a/tests/test_document_repository.py b/tests/test_document_repository.py index 33a49c0..dbf6c4f 100644 --- a/tests/test_document_repository.py +++ b/tests/test_document_repository.py @@ -8,7 +8,6 @@ import pytest from pipeline.document_repository import DocumentRepository -from pipeline.document_validity import ValidityPeriod class FakeDb: @@ -77,71 +76,3 @@ def test_unclassified_document_reads_as_the_column_default(self, db_connection, @pytest.mark.db def test_missing_document_reads_as_none(self, db_connection): assert DocumentRepository().get_document_kind("no-such-workflow") is None - - -class TestGetValidity: - @pytest.mark.unit - def test_returns_the_stored_period(self): - repo = DocumentRepository( - FakeDb(document={"valid_from": "2026-09-22", "valid_to": "2027-09-22"}) - ) - - assert repo.get_validity("wf-1") == ValidityPeriod( - start_date="2026-09-22", end_date="2027-09-22" - ) - - @pytest.mark.unit - def test_returns_none_for_a_missing_document(self): - assert DocumentRepository(FakeDb(document=None)).get_validity("wf-nope") is None - - @pytest.mark.unit - def test_returns_none_when_the_document_has_no_period(self): - # Uploaded before validity existed: its chunks carry no dates and are - # searchable without restriction. - repo = DocumentRepository(FakeDb(document={"valid_from": None, "valid_to": None})) - - assert repo.get_validity("wf-1") is None - - @pytest.mark.unit - def test_reads_a_real_row_written_by_db(self, db_connection): - db_connection.upsert_document( - workflow_id="wf-validity", - document_id="doc-validity", - filename="a.pdf", - filepath="/books/a.pdf", - ) - db_connection.set_document_validity("wf-validity", "2026-10-01", "2026-12-31") - - assert DocumentRepository(db_connection).get_validity("wf-validity") == ValidityPeriod( - start_date="2026-10-01", end_date="2026-12-31" - ) - - @pytest.mark.unit - def test_a_row_written_without_a_period_reads_as_none(self, db_connection): - # db.py stores what it is given and defaults nothing — the upload - # endpoint decides the period (see TestDocumentValidityEndpoints in - # test_api.py). A row created around it simply has none. - db_connection.upsert_document( - workflow_id="wf-fresh", - document_id="doc-fresh", - filename="b.pdf", - filepath="/books/b.pdf", - ) - - assert DocumentRepository(db_connection).get_validity("wf-fresh") is None - - @pytest.mark.unit - @pytest.mark.db - def test_a_period_passed_to_the_db_layer_is_stored_verbatim(self, db_connection): - db_connection.upsert_document( - workflow_id="wf-stamped", - document_id="doc-stamped", - filename="c.pdf", - filepath="/books/c.pdf", - valid_from="2026-09-23", - valid_to="2027-09-23", - ) - - assert DocumentRepository(db_connection).get_validity("wf-stamped") == ValidityPeriod( - start_date="2026-09-23", end_date="2027-09-23" - ) From 3706d7a47cc9a4c547f051937a5abe824964e736 Mon Sep 17 00:00:00 2001 From: shruti0025 Date: Fri, 25 Sep 2026 01:03:10 +0530 Subject: [PATCH 08/11] fix: fix-tests [#111] --- tests/test_translation.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/test_translation.py b/tests/test_translation.py index ef67c83..472fc25 100644 --- a/tests/test_translation.py +++ b/tests/test_translation.py @@ -10,6 +10,13 @@ class TestTranslationService: def test_load_translation_config_defaults(self, monkeypatch): from pipeline.translation.service import load_translation_config + # AGRINET_* win over TRANSLATION_* in _gemma_endpoint/_gemma_model, so + # clearing only the TRANSLATION_* pair leaves this asserting whatever + # the developer's .env happens to hold. A real .env sets both, which is + # why this passed alone and failed in a full run (any test importing + # pipeline.api pulls .env into the process via load_dotenv). + monkeypatch.delenv("AGRINET_GEMMA_BASE_URL", raising=False) + monkeypatch.delenv("AGRINET_GEMMA_MODEL_NAME", raising=False) monkeypatch.delenv("TRANSLATION_PROVIDER", raising=False) monkeypatch.delenv("TRANSLATION_MODEL", raising=False) monkeypatch.setenv("TRANSLATION_VLLM_BASE_URL", "http://localhost:8000/v1") From a79fa4a9efe74d947a01781fb9a48641c6cf21ee Mon Sep 17 00:00:00 2001 From: shruti0025 Date: Fri, 25 Sep 2026 08:56:07 +0530 Subject: [PATCH 09/11] fix: use-valid-dates [#111] --- pipeline/api.py | 7 +++++-- pipeline/document_validity.py | 12 ++++++++++++ tests/test_api.py | 27 +++++++++++++++++++++++++++ tests/test_document_validity.py | 31 +++++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+), 2 deletions(-) diff --git a/pipeline/api.py b/pipeline/api.py index 3075c30..747704f 100644 --- a/pipeline/api.py +++ b/pipeline/api.py @@ -3937,9 +3937,12 @@ async def run_search(payload: dict, user: RequireSearch): valid_on = (payload.get("valid_on") or "").strip() or None if valid_on: try: - document_validity.parse_period(valid_on, valid_on) + # Reassigned, not just checked: parse_day returns the canonical + # zero-padded form, and the vector store's date range rejects the + # `2026-9-3` spelling that strptime happily accepts. + valid_on = document_validity.parse_day(valid_on, "valid_on") except document_validity.DocumentValidityError as exc: - raise HTTPException(400, f"Invalid valid_on: {exc}") from None + raise HTTPException(400, str(exc)) from None expanded_query = _expand_query(query, query_expansion_profile) # Qdrant embeddings apply E5 prefixes internally; don't pre-prefix here. search_query = expanded_query diff --git a/pipeline/document_validity.py b/pipeline/document_validity.py index bd09887..dea70f2 100644 --- a/pipeline/document_validity.py +++ b/pipeline/document_validity.py @@ -179,6 +179,18 @@ def _parse_date(value: Optional[str], field: str) -> date: ) from None +def parse_day(value: Optional[str], field: str = "date") -> str: + """Validate a single calendar date and return it in canonical form. + + The canonical form matters as much as the validation: `strptime` accepts + `2026-9-3`, so a caller that validated a date and then forwarded the raw + string would pass its own check and fail somewhere downstream that wants + the padded form. Returning the parsed date means a caller cannot hold a + validated date and a non-canonical one at the same time. + """ + return _parse_date(value, field).isoformat() + + def parse_period( start_date: Optional[str] = None, end_date: Optional[str] = None, diff --git a/tests/test_api.py b/tests/test_api.py index ecf1f5a..13cdc85 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -439,6 +439,33 @@ def search(self, **kwargs): assert captured["apply_validity"] is False assert response.json()["effective_config"]["apply_validity"] is False + @pytest.mark.api + @pytest.mark.unit + def test_normalises_valid_on_before_it_reaches_the_store(self, test_client, monkeypatch): + # strptime accepts "2026-9-3" but the vector store's date range does + # not, so validating without using the parsed value turned a bad + # request into a "Vector search failed" 400 blaming the store. + captured = {} + + class FakeStore: + backend = "qdrant" + + def search(self, **kwargs): + captured.update(kwargs) + return {"hits": [], "valid_on": kwargs.get("valid_on")} + + monkeypatch.setattr( + "pipeline.vector_store.get_vector_store", lambda: FakeStore() + ) + + response = test_client.post( + "/search", json={"query": "kisan", "valid_on": "2026-9-3"} + ) + + assert response.status_code == 200 + assert captured["valid_on"] == "2026-09-03" + assert response.json()["effective_config"]["valid_on"] == "2026-09-03" + @pytest.mark.api @pytest.mark.unit def test_rejects_a_malformed_valid_on(self, test_client): diff --git a/tests/test_document_validity.py b/tests/test_document_validity.py index 3b23ede..0f94abe 100644 --- a/tests/test_document_validity.py +++ b/tests/test_document_validity.py @@ -15,6 +15,7 @@ add_years, day_of, default_period, + parse_day, parse_period, period_from_row, period_from_upload_date, @@ -287,3 +288,33 @@ def test_a_timestamp_past_the_range_falls_back_to_today(self): assert period_from_upload_timestamp( "9999-12-31T23:59:59", clock=FROZEN ) == default_period(FROZEN) + + +class TestParseDay: + """Validating a single calendar date, and getting it back canonical.""" + + @pytest.mark.unit + def test_returns_the_date_it_validated(self): + assert parse_day("2026-09-03") == "2026-09-03" + + @pytest.mark.unit + def test_zero_pads_a_date_strptime_accepts_unpadded(self): + # The reason this returns a value instead of just raising: strptime + # takes "2026-9-3", and a caller forwarding that raw string fails in + # whatever consumer wants the padded form. + assert parse_day("2026-9-3") == "2026-09-03" + + @pytest.mark.unit + def test_trims_surrounding_whitespace(self): + assert parse_day(" 2026-09-03 ") == "2026-09-03" + + @pytest.mark.unit + @pytest.mark.parametrize("bad", [None, "", " ", "01-01-2027", "2026/09/03", "garbage", "2026-13-45"]) + def test_rejects_anything_that_is_not_a_calendar_date(self, bad): + with pytest.raises(DocumentValidityError, match="YYYY-MM-DD"): + parse_day(bad) + + @pytest.mark.unit + def test_names_the_field_in_the_error(self): + with pytest.raises(DocumentValidityError, match="valid_on"): + parse_day("nope", "valid_on") From fd07bcbae86bb898752ebced26555720a11d7673 Mon Sep 17 00:00:00 2001 From: shruti0025 Date: Fri, 25 Sep 2026 09:27:46 +0530 Subject: [PATCH 10/11] fix: add-tests-for-format-variations [#111] --- tests/test_qdrant_validity.py | 69 +++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/tests/test_qdrant_validity.py b/tests/test_qdrant_validity.py index 56ab6e4..8abf5c7 100644 --- a/tests/test_qdrant_validity.py +++ b/tests/test_qdrant_validity.py @@ -8,8 +8,10 @@ from datetime import datetime import pytest +from pydantic import ValidationError from qdrant_client.http import models as qmodels +from pipeline.document_validity import parse_day from pipeline.vector_store.qdrant_store import ( PAYLOAD_FIELDS, QdrantVectorStore, @@ -227,3 +229,70 @@ def test_the_period_is_indexed_as_a_datetime(self): QdrantVectorStore.PAYLOAD_INDEXES["end_date"] == qmodels.PayloadSchemaType.DATETIME ) + + +class TestValidOnMustBeCanonical: + """Why `valid_on` is canonicalised before it ever reaches the store. + + `strptime("%Y-%m-%d")` accepts "2026-1-2", so a date can pass our own + validation and still be unusable downstream. The constraint is the Python + client's, not Qdrant's: the server filters "2026-1-2" exactly as it + filters "2026-01-02" (verified against a live instance), but + `qdrant_client`'s DatetimeRange is a pydantic model that rejects the + unpadded spelling before a request is ever sent. + + Left unguarded this surfaced as `400 Vector search failed (qdrant)`, + blaming the vector store for a malformed request. These pin the real + constraint so the guard is never removed as redundant, and so a client + upgrade that changes the rule is noticed here rather than in production. + """ + + @pytest.mark.unit + def test_the_client_rejects_an_unpadded_day(self): + with pytest.raises(ValidationError): + _build_filter(valid_on="2026-1-2") + + @pytest.mark.unit + def test_the_client_accepts_the_canonical_day(self): + assert _build_filter(valid_on="2026-01-02") is not None + + @pytest.mark.unit + @pytest.mark.parametrize( + "raw", + [ + "2026-1-2", # the spelling that started this + "2026-01-02", + " 2026-9-3 ", # padded and trimmed + "2024-2-29", # leap day, unpadded + "0001-01-01", # the year strftime would not zero-pad + "9999-12-31", + ], + ) + def test_anything_parse_day_returns_is_usable_as_a_filter(self, raw): + # The contract between the two modules: document_validity decides what + # a valid day is, and whatever it hands back must be something the + # store can filter on. This is the assertion that must never break. + assert _build_filter(valid_on=parse_day(raw)) is not None + + @pytest.mark.unit + def test_the_stores_own_today_is_usable_as_a_filter(self): + # The other source of `valid_on`: when a caller passes none, search + # falls back to self.today(). That path needs the same guarantee. + store = QdrantVectorStore(client=FakeClient(), clock=clock_at(TODAY)) + + assert _build_filter(valid_on=store.today()) is not None + + @pytest.mark.unit + def test_search_builds_a_usable_filter_from_an_unpadded_override(self): + # End of the chain: the store is handed only canonical days, so a + # search that overrides the clock still builds a filter rather than + # raising out of pydantic. + client = FakeClient() + store = QdrantVectorStore(client=client, clock=clock_at(TODAY)) + + result = store.search( + "idx", "kisan", search_mode="LEXICAL", valid_on=parse_day("2026-1-2") + ) + + assert result["valid_on"] == "2026-01-02" + assert ("start_date", "lte", "2026-01-02") in dates_in(client.scroll_filter) From d49db830917f715803e7181f23fe3ba4f53c8f56 Mon Sep 17 00:00:00 2001 From: shruti0025 Date: Fri, 25 Sep 2026 16:20:51 +0530 Subject: [PATCH 11/11] fix: disable-ui-inputs-and-change-defaults [#111] --- AGENTS.md | 2 +- CONTEXT.md | 2 +- .../0005-document-validity-filters-search.md | 14 ++- pipeline/activities.py | 12 +- pipeline/document_validity.py | 102 ++++++++--------- scripts/verify_document_validity_e2e.py | 42 +++++-- tests/test_activities.py | 40 +++++-- tests/test_api.py | 36 +++++- tests/test_document_validity.py | 108 ++++++++---------- ui/.env.example | 7 ++ ui/Dockerfile.prod | 6 +- ui/src/config.js | 15 +++ ui/src/views/DocumentOpsView.jsx | 103 ++++++++--------- 13 files changed, 287 insertions(+), 202 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 34bee03..ef17b18 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,7 +62,7 @@ Core modules (all under `pipeline/`): - `document_repository.py` — `DocumentRepository`, a domain layer over `db.py` for document reads; `db.py` stays one-function-per-query with no domain knowledge - `network_constants.py` — fixed values sent on the network (catalog/resource ids, topics, languages, Beckn version). The file v2 edits when the AI layer starts deriving them - `catalog_builder.py` — pure builder mapping a document's knowledge kind (`advisory`/`scheme`) to the single `OnDemand` Beckn catalog announced for that kind; returns `None` for any other kind. No env, no I/O -- `document_validity.py` — pure module owning Document Validity: the period a document's chunks answer searches in, the upload-day/+1-year default, and the clock injection point. Shared by `api.py` (stamping the default at upload), `scheme_catalog.py` (validating a reviewer's edit), `activities.py` (stamping chunk payloads) and `vector_store/qdrant_store.py` (filtering at search time). `db.py` stores the dates it is handed and defaults nothing +- `document_validity.py` — pure module owning Document Validity: the period a document's chunks answer searches in, the upload-day start with no end by default, and the clock injection point. Shared by `api.py` (stamping the default at upload), `scheme_catalog.py` (validating a reviewer's edit), `activities.py` (stamping chunk payloads) and `vector_store/qdrant_store.py` (filtering at search time). `db.py` stores the dates it is handed and defaults nothing - `discovery_publish_service.py` — `DiscoveryPublishService`: owns the Publish to Network env vars and the HTTP call. Deliberately Temporal-free - `models.py` — Pydantic models, including `DocumentStage` enum and `PIPELINE_STAGES` (the stepper-UI stage list) - `config.py` — `Config` dataclass reading env vars, with defaults diff --git a/CONTEXT.md b/CONTEXT.md index 2ced2cf..9cf0d9f 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -29,7 +29,7 @@ What a document *is* to the network — `advisory`, `scheme`, `video`, or an ope _Avoid_: "document type" — collides with `source_type`/`canonical_input_type`, which describe the input *format* (pdf, spreadsheet). Also avoid saying an advisory is "uploaded": what is uploaded is a file, which only becomes an advisory when someone classifies it. **Document Validity**: -The period a document's chunks answer searches in, held as `documents.valid_from` / `documents.valid_to` and stamped onto every chunk's vector payload as `start_date` / `end_date`. Defaulted on upload to the upload day plus one year, then confirmed or moved by the reviewer in the same form that sets the Knowledge Kind. Both ends are inclusive, and search filters on it so an expired or not-yet-started document is never answered from. Chunks ingested before it existed carry neither date and stay searchable — see `docs/ADR/0006-document-validity-filters-search.md`. +The period a document's chunks answer searches in, held as `documents.valid_from` / `documents.valid_to` and stamped onto every chunk's vector payload as `start_date` / `end_date`. Defaulted on upload to start that day with no end, meaning it never expires; a reviewer can narrow either end in the same form that sets the Knowledge Kind, behind the `VITE_DOCUMENT_VALIDITY_ENABLED` flag. Both ends are inclusive, and search filters on it so an expired or not-yet-started document is never answered from. Chunks ingested before it existed carry neither date and stay searchable — see `docs/ADR/0005-document-validity-filters-search.md`. _Avoid_: "expiry" — there is a start as well as an end. Also avoid calling it the document's lifetime: the document stays in the console, editable and reingestable, once its period has passed; only its search answers stop. **network_visible**: diff --git a/docs/ADR/0005-document-validity-filters-search.md b/docs/ADR/0005-document-validity-filters-search.md index f0afbe6..5caf12e 100644 --- a/docs/ADR/0005-document-validity-filters-search.md +++ b/docs/ADR/0005-document-validity-filters-search.md @@ -6,7 +6,7 @@ A short-lived predecessor (ADR 0005, since removed along with its code) put a *c Decision: - **A document-level concept, owned in one place.** `documents.valid_from` / `documents.valid_to`, stamped onto every chunk's Qdrant payload as `start_date` / `end_date`, owned by a pure module `pipeline/document_validity.py` that holds the rules so the API, the ingest activity and the vector store never re-derive them. -- **Defaulted on upload, not asked for.** The upload endpoints stamp the upload day and one year out. An uploader is not asked a question they cannot answer, and no document reaches the index without a period. A year is the business's review cadence, not a technical limit. The default is computed in `api.py` and passed down: `db.upsert_document` stores the dates verbatim on INSERT and has no opinion on what they should be, keeping `db.py` free of domain knowledge as the rest of that layer is. A row created around the upload path (a script, a backfill) therefore has no period, which search reads as "no stated period" — the same as the pre-validity corpus. +- **Defaulted on upload, not asked for.** The upload endpoints stamp the upload day as the start, with no end. An uploader is not asked a question they cannot answer, and no document reaches the index without a period. An end nobody chose would retire content on a date nobody meant, so the default expires never and narrowing it is a deliberate act. The default is computed in `api.py` and passed down: `db.upsert_document` stores the dates verbatim on INSERT and has no opinion on what they should be, keeping `db.py` free of domain knowledge as the rest of that layer is. A row created around the upload path (a script, a backfill) therefore has no period, which search reads as "no stated period" — the same as the pre-validity corpus. - **Confirmed or moved by the reviewer, in the form that already exists.** The dates sit beside the Knowledge Kind selector and travel on the same `PATCH /documents/{id}/scheme-metadata`. Choosing the period is part of classifying a document, not a separate errand, and the reviewer sees real prefilled dates rather than an empty field. Either end may be sent alone; the other is taken from what is stored, so moving the end date does not require restating the start. - **Validated like a bad kind, not like a bad type.** `document_validity.parse_period` raises, `apply_scheme_metadata` lets it through as a `ValueError`, and the endpoint answers 400 — the same shape a bad `document_kind` gets, not a Pydantic 422. A rejected period stores nothing, kind included. - **Both ends inclusive.** A document uploaded this morning starts today and must answer this afternoon; an end date names the last day it answers rather than the first day it does not. The requirement was written as "today greater than start and less than end", but exclusive bounds would hide a document on its own first day, which is the default every document gets. @@ -25,3 +25,15 @@ Alternatives considered: - **Filter after retrieval, in the API.** Rejected: it silently shrinks the candidate set below `top_k`, so an index full of expired documents would return a short page of results rather than the valid ones further down. - **Ask the uploader for the dates.** Rejected: the uploader is often not the person who knows how long content holds, and blocking upload on it would slow the common case for a value the reviewer sets anyway. - **Default to no end date (never expires).** Rejected: it makes expiry opt-in, so content would go stale by default — the same failure this ADR exists to fix. + +## Amendment — open-ended by default, and gated in the UI (2026-09-25) + +The original decision gave every upload a one-year end date. Business revised that: a document should stay answerable until someone decides otherwise, so the default is now **start at the upload day, no end**. `end_date is None` means the document never expires, and the search filter already read a missing end that way — the `(end_date missing OR end_date >= today)` clause was written for the pre-validity corpus and covers this unchanged, which is why no filter logic moved. + +Consequences worth naming: +- `period_from_row` now treats the **start** as what decides whether a period exists. A row with a start and a NULL end rebuilds as an open-ended period rather than as "no period", which is the shape every upload now stores. +- A chunk with no expiry carries **no** `end_date` key in its Qdrant payload rather than a null one, so the missing-field branch matches. +- `add_years` and `DEFAULT_VALIDITY_YEARS` existed only to derive the old default end. Nothing derives one any more, so they were removed rather than left as dead code. +- Clearing an end date is how a reviewer returns a document to never-expiring: a blank end is a real answer, not a missing one. + +The reviewer-facing date fields sit behind `VITE_DOCUMENT_VALIDITY_ENABLED` (default off), following the `VITE_AUTH_ENABLED` pattern. The flag gates **only the entry fields** — the backend stamps a period on every upload and search filters on it regardless — so enabling it exposes an existing capability rather than switching one on. With it off, classifying a document sends no dates at all and leaves the stored period untouched. diff --git a/pipeline/activities.py b/pipeline/activities.py index fc6bd30..b29c479 100644 --- a/pipeline/activities.py +++ b/pipeline/activities.py @@ -724,7 +724,11 @@ def _prepare_records( } if validity is not None: record["start_date"] = validity.start_date - record["end_date"] = validity.end_date + # Omitted, not null, when the document never expires: the search + # filter's "end_date missing" branch is what keeps it answerable + # forever, and _record_payload drops None anyway. + if validity.end_date is not None: + record["end_date"] = validity.end_date if is_scheme: record["scheme_code"] = (scheme_code or "").strip().lower() record["scheme_name"] = resolved_scheme_name @@ -764,9 +768,9 @@ def _validity_fields_from_doc(doc: dict | None) -> dict: """Extract the validity kwargs for `_prepare_records` from a documents row. A row with no stored period is stamped with the default anchored on its - upload day, so every chunk written from here on carries validity. Old - points already in the index keep none until their document is reingested, - which is what keeps them searchable in the meantime. + upload day - start there, no end - so every chunk written from here on + carries a start. Old points already in the index keep none until their + document is reingested, which is what keeps them searchable meanwhile. """ doc = doc or {} period = document_validity.period_from_row( diff --git a/pipeline/document_validity.py b/pipeline/document_validity.py index dea70f2..c9662ef 100644 --- a/pipeline/document_validity.py +++ b/pipeline/document_validity.py @@ -4,7 +4,8 @@ A document is only worth answering from for as long as its content still holds. This module owns that rule: what a legal period is, the period an uploader's document starts life with, and whether a period is live on a given -day. Pure - no env, no I/O, no db - so the API can validate an approver's edit, +day. A period always has a start; its end is optional, and an absent end means +the document never expires. Pure - no env, no I/O, no db - so the API can validate an approver's edit, the ingest activity can stamp the same period onto every chunk, and the vector store can build a search filter from it without any of them re-deriving it. @@ -26,11 +27,6 @@ # this module emitted would then fail the parse this module does. DATE_FORMAT = "%Y-%m-%d" -# How long a freshly uploaded document is presumed to stay current. A year is -# the business's default review cadence, not a technical limit: the approver is -# shown it prefilled and is free to move either end. -DEFAULT_VALIDITY_YEARS = 1 - # Returns "now". Injected wherever the answer depends on the current day, so a # test can pin the day instead of building fixtures relative to the real one. Clock = Callable[[], datetime] @@ -48,19 +44,31 @@ def system_clock() -> datetime: @dataclass(frozen=True) class ValidityPeriod: - """Inclusive calendar span, both ends `YYYY-MM-DD`. + """Calendar span, `YYYY-MM-DD`, with an inclusive start and optional end. + + Inclusive on purpose: a document uploaded today starts today and must be + searchable the same day, and an end date names the last day it answers + rather than the first day it does not. - Inclusive on purpose: a document uploaded today defaults to a period - starting today and must be searchable the same day, and an end date names - the last day it answers rather than the first day it does not. + `end_date is None` means the document never expires. That is the default a + document is uploaded with - business asks that content stay answerable + until someone decides otherwise, rather than going dark on a date nobody + chose. """ start_date: str - end_date: str + end_date: Optional[str] = None + + @property + def expires(self) -> bool: + """Whether this period has an end at all.""" + return self.end_date is not None def is_active_on(self, on_date: str) -> bool: """Whether this period covers `on_date` (`YYYY-MM-DD`).""" - return self.start_date <= on_date <= self.end_date + if on_date < self.start_date: + return False + return self.end_date is None or on_date <= self.end_date def today(clock: Optional[Clock] = None) -> str: @@ -68,38 +76,12 @@ def today(clock: Optional[Clock] = None) -> str: return (clock or system_clock)().date().isoformat() -def add_years(stamp: str, years: int = DEFAULT_VALIDITY_YEARS) -> str: - """`stamp` moved forward by whole calendar years. - - 29 February lands on 28 February in a non-leap year - the alternative - (1 March) would push the period into the wrong month for no gain. - - A stamp near `date.max` cannot move forward at all; that comes back as a - `DocumentValidityError` rather than the raw `ValueError` the stdlib - raises, so every failure out of this module is the one kind callers - already handle. - """ - anchor = _parse_date(stamp, "date") - target_year = anchor.year + years - try: - moved = anchor.replace(year=target_year) - except ValueError: - try: - moved = anchor.replace(year=target_year, day=28) - except ValueError: - raise DocumentValidityError( - f"{stamp} cannot move {years} year(s) forward: " - f"year {target_year} is outside the supported range." - ) from None - return moved.isoformat() - - def default_period(clock: Optional[Clock] = None) -> ValidityPeriod: """The period a document gets on upload, before anyone edits it. - Starts the day it is uploaded and runs a year out, so a document is live - the moment it is ingested and expires without anyone having to remember to - expire it. + Starts the day it is uploaded and does not end: a document is live the + moment it is ingested and stays answerable until a reviewer says otherwise. + An end nobody chose would retire content on a date nobody meant. """ return period_from_upload_date(today(clock)) @@ -108,11 +90,11 @@ def period_from_upload_date(upload_date: str) -> ValidityPeriod: """The default period anchored on the day a document was uploaded. Used when stamping a document that predates the stored columns: its own - upload day is a truer start than today, which would silently extend a - two-year-old document's life by another year. + upload day is a truer start than today, which would date a two-year-old + document as though it arrived this morning. """ stamp = _parse_date(upload_date, "upload date").isoformat() - return ValidityPeriod(start_date=stamp, end_date=add_years(stamp)) + return ValidityPeriod(start_date=stamp, end_date=None) def _parse_timestamp(value: Optional[str]) -> Optional[datetime]: @@ -196,19 +178,22 @@ def parse_period( end_date: Optional[str] = None, clock: Optional[Clock] = None, ) -> ValidityPeriod: - """Validate an operator-supplied period, defaulting either end. + """Validate an operator-supplied period. - An omitted start defaults to today and an omitted end to a year past the - resolved start, so a caller that sends only one end still gets a complete - period. Raises `DocumentValidityError` - never returns a half-valid - period, so a caller holding one can store it unchecked. + An omitted start defaults to today. An omitted end means the document + never expires - blank is a real answer here, not a missing one, so a + reviewer who clears the end date is asking for exactly that. Raises + `DocumentValidityError` - never returns a half-valid period, so a caller + holding one can store it unchecked. """ raw_start = start_date if (start_date or "").strip() else today(clock) start = _parse_date(raw_start, "start date") stamp = start.isoformat() - raw_end = end_date if (end_date or "").strip() else add_years(stamp) - end = _parse_date(raw_end, "end date") + if not (end_date or "").strip(): + return ValidityPeriod(start_date=stamp, end_date=None) + + end = _parse_date(end_date, "end date") if end < start: raise DocumentValidityError( @@ -225,12 +210,17 @@ def period_from_row( ) -> Optional[ValidityPeriod]: """Rebuild a stored period, or None when the document has none. - None is the normal case for a document uploaded before validity existed. - It means "this document has no stated period", which search reads as - always current - not an error, and not a silent fallback to today, which - would retire every legacy document at once. + The start is what decides whether a period exists at all. A row with a + start and a NULL end is the normal shape - that is what an upload stores - + and it rebuilds as an open-ended period, not as "no period": the document + is searchable from its start day and never expires. + + None means "this document has no stated period", the case for a document + uploaded before validity existed. Search reads that as always current - + not an error, and not a silent fallback to today, which would retire every + legacy document at once. """ - if not (start_date or "").strip() or not (end_date or "").strip(): + if not (start_date or "").strip(): return None try: return parse_period(start_date, end_date) diff --git a/scripts/verify_document_validity_e2e.py b/scripts/verify_document_validity_e2e.py index 7fa8200..e2c1b66 100644 --- a/scripts/verify_document_validity_e2e.py +++ b/scripts/verify_document_validity_e2e.py @@ -63,7 +63,7 @@ def make_document(workflow_id, text, valid_from=None, valid_to=None, legacy=Fals # document starts with — db.py stores what it is given and defaults nothing. if not legacy and valid_from is None and valid_to is None: default = document_validity.default_period() - valid_from, valid_to = default.start_date, default.end_date + valid_from, valid_to = default.start_date, default.end_date # end is None db.upsert_document( workflow_id=workflow_id, document_id=workflow_id, @@ -100,9 +100,14 @@ def make_document(workflow_id, text, valid_from=None, valid_to=None, legacy=Fals today = db.get_document("wf-active")["valid_from"] check("upload stamps today as the default start", by_doc["wf-active"]["start_date"], today) check( - "upload stamps a year out as the default end", - by_doc["wf-active"]["end_date"], + "upload leaves the default end unset (never expires)", + "end_date" in by_doc["wf-active"], + False, +) +check( + "and stores NULL for it", db.get_document("wf-active")["valid_to"], + None, ) check("approver-set period reaches the payload", by_doc["wf-expired"]["start_date"], "2024-01-01") check("legacy document carries no start_date", "start_date" in by_doc["wf-legacy"], False) @@ -132,13 +137,13 @@ def search_docs(**kwargs): ) print("\n=== 3. search with an injected clock ===") -# Past the active document's default year (today + 1y) and inside the future -# document's window, so exactly one of the two is live. +# Inside the future document's window. The active document is open-ended, so +# it is still live here too — only the expired one has dropped out. future_store = QdrantVectorStore(client=store.client, clock=clock_at("2027-12-01")) check( - "in Dec 2027 the future document is live and the active one has expired", + "in Dec 2027 the future document has started and the expired one is gone", sorted(h["doc_id"] for h in future_store.search(COLLECTION, "kisan credit card subsidy", limit=10)["hits"]), - ["wf-future", "wf-legacy"], + ["wf-active", "wf-future", "wf-legacy"], ) past_store = QdrantVectorStore(client=store.client, clock=clock_at("2024-06-01")) check( @@ -169,8 +174,29 @@ def search_docs(**kwargs): ["wf-legacy"], ) +print("\n=== 4b. an open-ended document never expires ===") +for far_future in ("2030-01-01", "2099-12-31", "9999-12-31"): + check( + f"still searchable on {far_future}", + sorted(h["doc_id"] for h in QdrantVectorStore( + client=store.client, clock=clock_at(far_future) + ).search(COLLECTION, "kisan credit card subsidy", limit=10)["hits"]), + ["wf-active", "wf-legacy"], + ) +check( + "but not before its start day", + sorted(h["doc_id"] for h in QdrantVectorStore( + client=store.client, clock=clock_at("2020-01-01") + ).search(COLLECTION, "kisan credit card subsidy", limit=10)["hits"]), + ["wf-legacy"], +) + print("\n=== 5. operator overrides ===") -check("valid_on answers for another day", search_docs(valid_on="2027-12-01"), ["wf-future", "wf-legacy"]) +check( + "valid_on answers for another day", + search_docs(valid_on="2027-12-01"), + ["wf-active", "wf-future", "wf-legacy"], +) check( "include_expired returns everything", search_docs(apply_validity=False), diff --git a/tests/test_activities.py b/tests/test_activities.py index 051cc1a..79904c0 100644 --- a/tests/test_activities.py +++ b/tests/test_activities.py @@ -307,7 +307,11 @@ def test_omits_the_period_when_the_document_has_none(self): assert "end_date" not in records[0] @pytest.mark.unit - def test_omits_the_period_when_only_one_end_is_stored(self): + def test_a_start_with_no_end_stamps_a_start_and_omits_the_end(self): + # The normal shape since the business dropped the fixed expiry: the + # chunk is searchable from its start day and never expires. The end is + # omitted rather than null — the filter's "end_date missing" branch is + # what keeps it answerable. from pipeline.activities import _prepare_records records = _prepare_records( @@ -317,7 +321,8 @@ def test_omits_the_period_when_only_one_end_is_stored(self): valid_from="2026-09-22", ) - assert "start_date" not in records[0] + assert records[0]["start_date"] == "2026-09-22" + assert "end_date" not in records[0] @pytest.mark.unit def test_the_period_is_in_the_passage_schema(self): @@ -342,13 +347,13 @@ def test_uses_the_stored_period(self): @pytest.mark.unit def test_falls_back_to_the_upload_day_for_a_row_with_no_period(self): # A document uploaded before validity existed, being reingested: its - # own upload day is a truer start than today, which would silently - # extend its life by another year. + # own upload day is a truer start than today, which would date it as + # though it arrived this morning. from pipeline.activities import _validity_fields_from_doc fields = _validity_fields_from_doc({"created_at": "2024-05-01T09:30:00"}) - assert fields == {"valid_from": "2024-05-01", "valid_to": "2025-05-01"} + assert fields == {"valid_from": "2024-05-01", "valid_to": None} @pytest.mark.unit def test_falls_back_to_today_when_the_row_has_no_upload_day_either(self): @@ -361,13 +366,28 @@ def test_falls_back_to_today_when_the_row_has_no_upload_day_either(self): assert fields["valid_from"] == date.today().isoformat() @pytest.mark.unit - def test_always_returns_both_ends(self): - # The ingest path must never write half a period. + @pytest.mark.parametrize( + "doc", [{}, None, {"valid_from": "2026-01-01"}, {"created_at": "bogus"}] + ) + def test_always_returns_a_start(self, doc): + # The ingest path must never write a period with no start. The end is + # optional by design — None means the document never expires. + from pipeline.activities import _validity_fields_from_doc + + fields = _validity_fields_from_doc(doc) + + assert fields["valid_from"] + assert "valid_to" in fields + + @pytest.mark.unit + def test_a_stored_open_ended_period_survives(self): from pipeline.activities import _validity_fields_from_doc - for doc in ({}, None, {"valid_from": "2026-01-01"}, {"created_at": "bogus"}): - fields = _validity_fields_from_doc(doc) - assert fields["valid_from"] and fields["valid_to"] + fields = _validity_fields_from_doc( + {"valid_from": "2026-01-01", "valid_to": None} + ) + + assert fields == {"valid_from": "2026-01-01", "valid_to": None} class TestUpdateDocumentState: diff --git a/tests/test_api.py b/tests/test_api.py index 13cdc85..e9acfb5 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -212,13 +212,13 @@ def _uploaded_document(self, db_connection, workflow_id): @pytest.mark.api @pytest.mark.unit - def test_upload_stamps_today_and_a_year_out(self, test_client, sample_pdf_content): + def test_upload_starts_today_and_never_expires(self, test_client, sample_pdf_content): # The upload endpoint owns this default — db.py stores what it is given # and has no opinion on what a period should be — so the guarantee is # only real if it is asserted through the endpoint that grants it. - from pipeline.document_validity import default_period - - expected = default_period() + # Spelled out rather than compared against default_period(), so a + # change to that default fails here instead of agreeing with itself. + from datetime import date response = test_client.post( "/upload", @@ -227,8 +227,32 @@ def test_upload_stamps_today_and_a_year_out(self, test_client, sample_pdf_conten assert response.status_code == 200 doc = test_client.get(f"/documents/{response.json()['workflow_id']}").json() - assert doc["valid_from"] == expected.start_date - assert doc["valid_to"] == expected.end_date + assert doc["valid_from"] == date.today().isoformat() + assert doc["valid_to"] is None + + @pytest.mark.api + @pytest.mark.unit + def test_an_end_date_can_be_set_and_then_cleared(self, test_client, db_connection): + # Clearing is the way back to "never expires". Blank has to be a real + # answer, not a missing one, or an end date could never be undone. + workflow_id = "validity-009" + self._uploaded_document(db_connection, workflow_id) + + set_response = test_client.patch( + f"/documents/{workflow_id}/scheme-metadata", + json={"document_kind": "advisory", "valid_from": "2026-01-01", "valid_to": "2026-06-30"}, + ) + assert set_response.json()["valid_to"] == "2026-06-30" + + cleared = test_client.patch( + f"/documents/{workflow_id}/scheme-metadata", + json={"document_kind": "advisory", "valid_to": ""}, + ) + + assert cleared.status_code == 200 + assert cleared.json()["valid_to"] is None + assert cleared.json()["valid_from"] == "2026-01-01" + assert db_connection.get_document(workflow_id)["valid_to"] is None @pytest.mark.api @pytest.mark.unit diff --git a/tests/test_document_validity.py b/tests/test_document_validity.py index 0f94abe..f4939c7 100644 --- a/tests/test_document_validity.py +++ b/tests/test_document_validity.py @@ -9,10 +9,8 @@ import pytest from pipeline.document_validity import ( - DEFAULT_VALIDITY_YEARS, DocumentValidityError, ValidityPeriod, - add_years, day_of, default_period, parse_day, @@ -43,53 +41,20 @@ def test_falls_back_to_the_system_clock(self): assert today() == system_clock().date().isoformat() -class TestAddYears: - @pytest.mark.unit - def test_moves_a_date_a_year_on(self): - assert add_years("2026-09-22") == "2027-09-22" - - @pytest.mark.unit - def test_default_is_one_year(self): - assert DEFAULT_VALIDITY_YEARS == 1 - - @pytest.mark.unit - def test_leap_day_lands_on_the_28th(self): - # 2027-02-29 does not exist; 1 March would push the period into the - # wrong month, so the last day of February is the honest answer. - assert add_years("2024-02-29") == "2025-02-28" - - @pytest.mark.unit - def test_leap_day_to_a_leap_year_keeps_the_29th(self): - assert add_years("2023-02-28") == "2024-02-28" - - @pytest.mark.unit - def test_rejects_a_malformed_date(self): - with pytest.raises(DocumentValidityError, match="YYYY-MM-DD"): - add_years("22-09-2026") - - @pytest.mark.unit - def test_zero_pads_a_year_below_1000(self): - # strftime("%Y") does not zero-pad on glibc — year 1 renders as - # "1-01-01" on Linux and "0001-01-01" on macOS — so a date this module - # emitted would fail the parse this module does, on Linux only. - assert add_years("0001-01-01") == "0002-01-01" - - @pytest.mark.unit - def test_a_date_that_cannot_move_forward_is_a_domain_error(self): - # date.max is year 9999; the stdlib raises a bare ValueError there, - # which callers handling DocumentValidityError would not catch. - with pytest.raises(DocumentValidityError, match="outside the supported range"): - add_years("9999-12-31") - - class TestDefaultPeriod: @pytest.mark.unit - def test_starts_today_and_runs_a_year(self): - # What an uploader's document gets before anyone edits anything. + def test_starts_today_and_never_ends(self): + # What an uploader's document gets before anyone edits anything. The + # business asks that content stay answerable until someone decides + # otherwise, rather than expiring on a date nobody chose. assert default_period(FROZEN) == ValidityPeriod( - start_date="2026-09-22", end_date="2027-09-22" + start_date="2026-09-22", end_date=None ) + @pytest.mark.unit + def test_does_not_expire(self): + assert default_period(FROZEN).expires is False + @pytest.mark.unit def test_is_active_on_its_own_first_day(self): # The whole point of defaulting the start to the upload day: a document @@ -97,25 +62,23 @@ def test_is_active_on_its_own_first_day(self): assert default_period(FROZEN).is_active_on("2026-09-22") @pytest.mark.unit - def test_is_active_on_its_own_last_day(self): - assert default_period(FROZEN).is_active_on("2027-09-22") - - @pytest.mark.unit - def test_is_not_active_the_day_after_it_ends(self): - assert not default_period(FROZEN).is_active_on("2027-09-23") + @pytest.mark.parametrize("day", ["2027-09-23", "2099-01-01", "9999-12-31"]) + def test_is_still_active_however_far_out_you_look(self, day): + assert default_period(FROZEN).is_active_on(day) @pytest.mark.unit def test_is_not_active_the_day_before_it_starts(self): + # An open end does not mean an open start. assert not default_period(FROZEN).is_active_on("2026-09-21") class TestPeriodFromUploadDate: @pytest.mark.unit def test_anchors_on_the_upload_day_not_today(self): - # A document uploaded two years ago must not have its life quietly - # extended by a year just because it is being reingested today. + # A document uploaded two years ago must not be dated as though it + # arrived this morning just because it is being reingested today. assert period_from_upload_date("2024-05-01") == ValidityPeriod( - start_date="2024-05-01", end_date="2025-05-01" + start_date="2024-05-01", end_date=None ) @pytest.mark.unit @@ -126,15 +89,21 @@ def test_rejects_a_malformed_upload_date(self): class TestParsePeriod: @pytest.mark.unit - def test_defaults_both_ends(self): + def test_defaults_to_today_with_no_end(self): assert parse_period(clock=FROZEN) == ValidityPeriod( - start_date="2026-09-22", end_date="2027-09-22" + start_date="2026-09-22", end_date=None ) @pytest.mark.unit @pytest.mark.parametrize("blank", [None, "", " "]) - def test_defaults_a_blank_end_to_a_year_past_the_start(self, blank): - assert parse_period("2026-01-15", blank, clock=FROZEN).end_date == "2027-01-15" + def test_a_blank_end_means_never_expires(self, blank): + # Blank is a real answer here, not a missing one: a reviewer who + # clears the end date is asking for a document that never expires. + assert parse_period("2026-01-15", blank, clock=FROZEN).end_date is None + + @pytest.mark.unit + def test_an_explicit_end_is_still_honoured(self): + assert parse_period("2026-01-15", "2026-06-30", clock=FROZEN).end_date == "2026-06-30" @pytest.mark.unit @pytest.mark.parametrize("blank", [None, "", " "]) @@ -190,13 +159,24 @@ def test_rebuilds_a_stored_period(self): @pytest.mark.unit @pytest.mark.parametrize( "start,end", - [(None, None), ("2026-01-01", None), (None, "2026-12-31"), ("", ""), (" ", "2026-12-31")], + [(None, None), (None, "2026-12-31"), ("", ""), (" ", "2026-12-31")], ) - def test_returns_none_when_either_end_is_missing(self, start, end): + def test_returns_none_when_the_start_is_missing(self, start, end): # A document uploaded before validity existed. None means "no stated - # period", which search reads as always current. + # period", which search reads as always current. The start is what + # decides whether a period exists at all. assert period_from_row(start, end) is None + @pytest.mark.unit + @pytest.mark.parametrize("blank", [None, "", " "]) + def test_a_start_with_no_end_is_an_open_ended_period(self, blank): + # The normal shape: this is exactly what an upload stores. It must + # rebuild as a real period, not as "no period" — the document is + # searchable from its start day and never expires. + assert period_from_row("2026-01-01", blank) == ValidityPeriod( + start_date="2026-01-01", end_date=None + ) + @pytest.mark.unit def test_returns_none_for_a_row_this_module_never_wrote(self): assert period_from_row("01/01/2026", "31/12/2026") is None @@ -255,7 +235,7 @@ class TestPeriodFromUploadTimestamp: @pytest.mark.unit def test_anchors_on_the_day_the_timestamp_falls_on(self): assert period_from_upload_timestamp("2024-05-01T09:30:00.123456") == ValidityPeriod( - start_date="2024-05-01", end_date="2025-05-01" + start_date="2024-05-01", end_date=None ) @pytest.mark.unit @@ -284,10 +264,12 @@ def test_never_raises(self, value): assert period_from_upload_timestamp(value, clock=FROZEN) is not None @pytest.mark.unit - def test_a_timestamp_past_the_range_falls_back_to_today(self): + def test_a_timestamp_at_the_end_of_the_range_is_anchored_not_rejected(self): + # Used to fall back to today: deriving an end a year out overflowed + # past date.max. With no end to derive there is nothing to overflow. assert period_from_upload_timestamp( "9999-12-31T23:59:59", clock=FROZEN - ) == default_period(FROZEN) + ) == ValidityPeriod(start_date="9999-12-31", end_date=None) class TestParseDay: diff --git a/ui/.env.example b/ui/.env.example index 580a0cd..80e1c25 100644 --- a/ui/.env.example +++ b/ui/.env.example @@ -14,6 +14,13 @@ VITE_MARQO_PROXY_TARGET=http://localhost:8882 # local development without Keycloak. Set to true to require sign-in. VITE_AUTH_ENABLED=false +# --- Feature flags --------------------------------------------------------- +# Document validity: shows the "Valid from" / "Valid until" fields beside the +# document type, letting a reviewer set how long a document answers searches. +# The backend stamps and filters on a validity period regardless; this only +# exposes the entry fields. Turn on when the business adopts the feature. +VITE_DOCUMENT_VALIDITY_ENABLED=false + # DEV: platform realm with Google SSO + multi-state groups VITE_KEYCLOAK_URL=https://dev-auth-vistaar.da.gov.in/auth VITE_KEYCLOAK_REALM=bharat-vistaar diff --git a/ui/Dockerfile.prod b/ui/Dockerfile.prod index f269cb2..33a97c8 100644 --- a/ui/Dockerfile.prod +++ b/ui/Dockerfile.prod @@ -34,6 +34,9 @@ ARG VITE_KEYCLOAK_REALM=docs-pipeline ARG VITE_KEYCLOAK_CLIENT_ID=docs-pipeline-ui ARG VITE_KEYCLOAK_IDP_HINT= ARG VITE_AUTH_ENABLED=true +# Feature flag: shows the document validity date fields. Off by default — +# the backend applies validity regardless, this only exposes the entry fields. +ARG VITE_DOCUMENT_VALIDITY_ENABLED=false ARG VITE_BASE=/knowledge-provider/ ENV VITE_API_BASE=$VITE_API_BASE \ @@ -41,7 +44,8 @@ ENV VITE_API_BASE=$VITE_API_BASE \ VITE_KEYCLOAK_REALM=$VITE_KEYCLOAK_REALM \ VITE_KEYCLOAK_CLIENT_ID=$VITE_KEYCLOAK_CLIENT_ID \ VITE_KEYCLOAK_IDP_HINT=$VITE_KEYCLOAK_IDP_HINT \ - VITE_AUTH_ENABLED=$VITE_AUTH_ENABLED + VITE_AUTH_ENABLED=$VITE_AUTH_ENABLED \ + VITE_DOCUMENT_VALIDITY_ENABLED=$VITE_DOCUMENT_VALIDITY_ENABLED # Vite base path for assets under /knowledge-provider/ RUN npm run build -- --base=${VITE_BASE} diff --git a/ui/src/config.js b/ui/src/config.js index 7935b81..0b9173c 100644 --- a/ui/src/config.js +++ b/ui/src/config.js @@ -1,3 +1,18 @@ // Prefer VITE_API_BASE at build time (production path prefix). // Local Vite dev proxies /api → API container/host. export const API_BASE = (import.meta.env.VITE_API_BASE || '/api').replace(/\/$/, '') + +/** + * Document validity (start / end dates on the classification panel). + * + * Off by default: the backend already stamps and honours a validity period on + * every document, but the business has not asked reviewers to set one yet. + * The flag gates the *entry fields* only - search still filters on whatever + * is stored, so turning it on exposes an existing capability rather than + * switching one on. + * + * Build-time, matching VITE_AUTH_ENABLED: flipping it needs a UI rebuild, and + * it is a deployment-wide decision rather than a per-user one. + */ +export const DOCUMENT_VALIDITY_ENABLED = + String(import.meta.env.VITE_DOCUMENT_VALIDITY_ENABLED ?? 'false').toLowerCase() === 'true' diff --git a/ui/src/views/DocumentOpsView.jsx b/ui/src/views/DocumentOpsView.jsx index c2b3acb..892fabd 100644 --- a/ui/src/views/DocumentOpsView.jsx +++ b/ui/src/views/DocumentOpsView.jsx @@ -44,6 +44,7 @@ import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '. import { Skeleton } from '../components/ui/skeleton' import { Tabs, TabsContent, TabsList, TabsTrigger } from '../components/ui/tabs' import { Textarea } from '../components/ui/textarea' +import { DOCUMENT_VALIDITY_ENABLED } from '../config' import { fetchJson, formatCompactDateTime, @@ -110,25 +111,14 @@ function previewSchemeCode(title) { } /** - * Validity the document is searchable in. Prefilled rather than blank: the - * server already stamped the upload day and a year out on upload, so the - * reviewer is confirming or moving a real period, not inventing one. Mirrors - * pipeline/document_validity.py - the server revalidates whatever is sent. + * Validity the document is searchable in. The start is prefilled from what the + * server stamped at upload; the end is deliberately blank-able - an empty end + * means the document never expires, which is the default every upload gets. + * Mirrors pipeline/document_validity.py; the server revalidates whatever is sent. */ -function plusOneYearISODate(stamp) { - const [year, month, day] = (stamp || '').split('-') - if (!year || !month || !day) return '' - // Clamp 29 Feb to 28 Feb in a non-leap year, matching add_years() server-side. - const moved = new Date(Number(year) + 1, Number(month) - 1, Number(day)) - const movedMonth = String(moved.getMonth() + 1).padStart(2, '0') - const movedDay = String(moved.getDate()).padStart(2, '0') - if (moved.getMonth() !== Number(month) - 1) return `${Number(year) + 1}-${month}-28` - return `${moved.getFullYear()}-${movedMonth}-${movedDay}` -} - function validateValidity(validFrom, validTo) { - if (!validFrom || !validTo) return 'Set both a start and an end date.' - if (validTo < validFrom) return 'End date cannot be before the start date.' + if (!validFrom) return 'Set a start date.' + if (validTo && validTo < validFrom) return 'End date cannot be before the start date.' return '' } @@ -152,15 +142,17 @@ function DocumentClassificationPanel({ doc, workflowId, canClassify, onSaved }) const [customKind, setCustomKind] = useState(hasKind && !DOCUMENT_KIND_OPTIONS.some(o => o.value === doc.document_kind) ? doc.document_kind : '') const [schemeName, setSchemeName] = useState(doc.scheme_name || '') const [validFrom, setValidFrom] = useState(doc.valid_from || todayISODate()) - const [validTo, setValidTo] = useState( - doc.valid_to || plusOneYearISODate(doc.valid_from || todayISODate()) - ) + // Blank when the document has no end date, which is the normal case - an + // empty field here reads as "never expires", not as "not filled in yet". + const [validTo, setValidTo] = useState(doc.valid_to || '') const [saving, setSaving] = useState(false) const [error, setError] = useState('') const effectiveKind = kind === '__custom__' ? customKind.trim().toLowerCase() : kind const isScheme = effectiveKind === 'scheme' - const validityError = validateValidity(validFrom, validTo) + const validityError = DOCUMENT_VALIDITY_ENABLED + ? validateValidity(validFrom, validTo) + : '' const canSave = Boolean(effectiveKind) && (!isScheme || schemeName.trim().length > 0) && @@ -177,8 +169,11 @@ function DocumentClassificationPanel({ doc, workflowId, canClassify, onSaved }) body: JSON.stringify({ document_kind: effectiveKind, ...(isScheme ? { scheme_name: schemeName.trim() } : {}), - valid_from: validFrom, - valid_to: validTo, + // Omitted entirely when the feature is off, so classifying a + // document leaves whatever period the upload stamped untouched. + ...(DOCUMENT_VALIDITY_ENABLED + ? { valid_from: validFrom, valid_to: validTo } + : {}), }), }) await onSaved() @@ -192,13 +187,15 @@ function DocumentClassificationPanel({ doc, workflowId, canClassify, onSaved }) const alreadySaved = doc.document_kind === effectiveKind && (!isScheme || doc.scheme_name === schemeName.trim()) - && doc.valid_from === validFrom - && doc.valid_to === validTo + && (!DOCUMENT_VALIDITY_ENABLED + || (doc.valid_from === validFrom && (doc.valid_to || '') === validTo)) return (

- Document type and validity — used by the Master Catalog / AI layer + {DOCUMENT_VALIDITY_ENABLED + ? 'Document type and validity — used by the Master Catalog / AI layer' + : 'Document type — used by the Master Catalog / AI layer'}

@@ -214,27 +211,31 @@ function DocumentClassificationPanel({ doc, workflowId, canClassify, onSaved })
-
- Valid from - setValidFrom(e.target.value)} - /> -
-
- Valid until - setValidTo(e.target.value)} - /> -
+ {DOCUMENT_VALIDITY_ENABLED && ( + <> +
+ Valid from + setValidFrom(e.target.value)} + /> +
+
+ Valid until (optional) + setValidTo(e.target.value)} + /> +
+ + )} {kind === '__custom__' && (
Custom type @@ -273,13 +274,13 @@ function DocumentClassificationPanel({ doc, workflowId, canClassify, onSaved }) )}
- {canClassify && validityError ? ( + {DOCUMENT_VALIDITY_ENABLED && canClassify && validityError ? (

{validityError}

) : null} - {canClassify && !validityError ? ( + {DOCUMENT_VALIDITY_ENABLED && canClassify && !validityError ? (

- Search only answers from this document between these dates. Defaults to a - year from upload — move either end before publishing to dev. + Search only answers from this document from the start date onwards. Leave + the end date blank and it never expires.

) : null} {error ?

{error}

: null}