From bcd3dffd45e929c0d4b8b6fe366b2c8b1717c00d Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Tue, 13 Oct 2026 17:13:50 +0530 Subject: [PATCH 01/23] feat: publish-under-group-provider [#190] Documents filed under a state publish to the network under that Group's provider (NETWORK_GROUP_PROVIDER_IDS), with group-prefixed catalog and resource ids. Bharat Vistaar and default keep the shared provider. A Group with no provider is skipped with a warning shown on the document page; a malformed map stops the API and worker from booting. Chunks gain a group field beside instance/instance_name. See ADR 0006. Co-Authored-By: Claude Opus 5.5 (1M context) --- .env.example | 4 + CONTEXT.md | 14 ++- ENV.md | 4 + pipeline/activities.py | 32 +++++- pipeline/api.py | 5 + pipeline/catalog_builder.py | 53 +++++++-- pipeline/discovery_publish_service.py | 63 ++++++++-- pipeline/document_repository.py | 12 ++ pipeline/network_constants.py | 5 +- pipeline/network_group.py | 84 ++++++++++++++ pipeline/vector_store/qdrant_store.py | 4 + pipeline/worker.py | 5 + tests/test_activities.py | 145 +++++++++++++++++++++++- tests/test_catalog_builder.py | 57 ++++++++++ tests/test_discovery_publish_service.py | 103 +++++++++++++++++ tests/test_document_repository.py | 23 ++++ tests/test_network_group.py | 131 +++++++++++++++++++++ tests/test_qdrant_group.py | 38 +++++++ ui/src/lib/networkPublish.js | 29 +++++ ui/src/lib/networkPublish.test.js | 46 ++++++++ ui/src/views/DocumentOpsView.jsx | 6 + 21 files changed, 830 insertions(+), 33 deletions(-) create mode 100644 pipeline/network_group.py create mode 100644 tests/test_network_group.py create mode 100644 tests/test_qdrant_group.py create mode 100644 ui/src/lib/networkPublish.js create mode 100644 ui/src/lib/networkPublish.test.js diff --git a/.env.example b/.env.example index 0e27f27..a08df31 100644 --- a/.env.example +++ b/.env.example @@ -259,3 +259,7 @@ KC_SMTP_STARTTLS=true KC_SMTP_SSL=false KC_SMTP_USERNAME= KC_SMTP_PASSWORD= + +# Group Provider per state, set per deployment (ADR 0006). JSON object, keys case-insensitive. +# Leave empty to publish no Group; malformed JSON stops the API and worker from booting. +NETWORK_GROUP_PROVIDER_IDS={"mh":"mh.provider.oan.dev"} diff --git a/CONTEXT.md b/CONTEXT.md index 9cf0d9f..058c542 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -9,7 +9,7 @@ The operator action that approves a document out of `ready_for_ingestion` and le _Avoid_: "publish" alone — ambiguous with Publish to Network. **Publish to Network**: -The pipeline stage that runs only after a document's PROD promotion succeeds, and POSTs a Beckn-shaped `publish` envelope to an external Discovery Service. Skipped entirely when PROD is disabled (`DISABLE_PROD_SETTING=true`) — a document never reaches the network without first promoting to PROD. The envelope it sends is kind-level (see Knowledge Kind), not document-level: it does not make any single document searchable, only re-announces that this provider serves that kind at all. +The pipeline stage that runs only after a document's PROD promotion succeeds, and POSTs a Beckn-shaped `publish` envelope to an external Discovery Service. Skipped entirely when PROD is disabled (`DISABLE_PROD_SETTING=true`) — a document never reaches the network without first promoting to PROD. The envelope it sends is kind-level (see Knowledge Kind), not document-level: it does not make any single document searchable, only re-announces that the document's provider (its Group Provider, or the Shared Provider) serves that kind at all. _Avoid_: "publish" alone — ambiguous with Publish to Dev. **Master Catalog**: @@ -24,6 +24,18 @@ _Avoid_: "catalog" alone — ambiguous with Master Catalog and the Network Catal The Beckn wire-protocol `message.catalogs` object sent by Publish to Network to the Discovery Service. Structurally and semantically unrelated to the Master Catalog and Scheme Catalog tables — do not conflate the three. _Avoid_: "catalog" alone. +**Group**: +The state a document belongs to, named by its state code (`mh` for Maharashtra) and taken from the document's `instance`. Bharat Vistaar and `default` documents have no Group. New features say Group; existing screens, roles and data still say state or instance. +_Avoid_: "tenant"; "state" or "instance" in new features. Never call Bharat Vistaar a Group. + +**Group Provider**: +The network provider a Group's documents are announced under in Publish to Network, with its own catalog per Knowledge Kind, set per deployment. A Group without one configured publishes nothing. +_Avoid_: "state provider". + +**Shared Provider**: +The single network provider used by documents with no Group (Bharat Vistaar, `default`), keeping the original catalog ids. +_Avoid_: "national provider", "central provider". + **Knowledge Kind**: 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. diff --git a/ENV.md b/ENV.md index 3e517c3..734824c 100644 --- a/ENV.md +++ b/ENV.md @@ -249,10 +249,13 @@ Separate from the SQLite/Qdrant catalog above (which tracks PROD vector publicat First piece of the network-discovery push architecture: once a document's PROD promotion succeeds, the `publishing_to_network` pipeline stage POSTs a Beckn-shaped `publish` envelope to an external Discovery Service. It's a PROD-only stage (see `docs/ADR/0004-publishing-to-network-is-prod-only.md`) — skipped entirely when `DISABLE_PROD_SETTING=true`, since a document never reaches the network without first promoting to PROD. This is going to be the only way the experience layer discovers schemas going forward — the Master Scheme Catalog pull API above stays in place for backward compatibility. The catalog announces the *knowledge kind* the document was classified as — one catalog with exactly one `OnDemand` resource for `advisory`, another for `scheme`; any other `document_kind` publishes nothing. `DISCOVERY_SERVICE_ENDPOINT` and `NETWORK_SENDER_ID` are **required** whenever PROD promotion is enabled — there is no separate feature flag for this stage beyond `DISABLE_PROD_SETTING`. +A document filed under a state (its **Group**, taken from the document's `instance`) is announced under that Group's own provider, looked up in `NETWORK_GROUP_PROVIDER_IDS`: catalog and resource ids get the group code as a prefix (`mh-cat-oan-knowledge-provider-advisories`) and both catalog and provider are named ` - Knowledge Advisories|Schemes`. Documents under the portal instance (`PORTAL_INSTANCE`, `bv`) or the default instance (`DEFAULT_INSTANCE`) have no Group and keep the shared provider and unprefixed ids. A Group missing from the map is **skipped with a warning** (the document still completes, and the console shows the warning); a malformed map stops both the API and the worker from booting. See `docs/ADR/0006-publish-to-network-under-per-group-provider.md`. + | Variable | Default | Purpose | |----------|---------|---------| | `DISCOVERY_SERVICE_ENDPOINT` | *(empty — required)* | Base URL of the external Discovery Service; the pipeline POSTs to `{DISCOVERY_SERVICE_ENDPOINT}/publish` | | `NETWORK_SENDER_ID` | *(empty — required)* | This instance's Beckn `senderId`, also sent as `networkId` | +| `NETWORK_GROUP_PROVIDER_IDS` | *(empty — no Group has a provider)* | JSON object of group code → provider id, set per deployment, e.g. `{"mh":"mh.provider.oan.dev"}`. Keys are case-insensitive. Must be valid JSON of non-empty strings or the API and worker refuse to start | | `DISCOVERY_SERVICE_PUBLISH_INITIAL_INTERVAL_SECONDS` | `30` | Wait before the first retry of a failed publish | | `DISCOVERY_SERVICE_PUBLISH_BACKOFF_COEFFICIENT` | `2.0` | Multiplier applied to the wait after each retry | | `DISCOVERY_SERVICE_PUBLISH_MAX_INTERVAL_SECONDS` | `300` | Cap on the wait between retries | @@ -270,6 +273,7 @@ First piece of the network-discovery push architecture: once a document's PROD p | OCR / translation / chunking / domain tags | — | config / status | ✅ runs jobs | | Qdrant (`VECTOR_DB_*`) | — | ✅ | ✅ | | Publish to Network (`DISCOVERY_SERVICE_ENDPOINT`, `NETWORK_SENDER_ID`) | — | — | ✅ runs the publish activity | +| Group Provider map (`NETWORK_GROUP_PROVIDER_IDS`) | — | validated at boot | ✅ validated at boot, used by the publish activity | --- diff --git a/pipeline/activities.py b/pipeline/activities.py index b29c479..8ce7112 100644 --- a/pipeline/activities.py +++ b/pipeline/activities.py @@ -26,10 +26,15 @@ 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 +from .discovery_publish_service import ( + SKIP_GROUP_PROVIDER_NOT_CONFIGURED, + SKIP_UNMAPPED_KIND, + DiscoveryPublishService, +) from .document_repository import DocumentRepository from .instances import instance_display_name from .network_constants import RESULT_REJECTED +from .network_group import group_for_instance from .ocr import ocr_pdf as run_ocr_pdf from .ocr import ocr_pdf_in_segments as run_ocr_pdf_in_segments from .translation import load_translation_config, translate_pages @@ -697,6 +702,8 @@ def _prepare_records( "workflow_id": workflow_id or "", "instance": resolved_instance, "instance_name": instance_name, + # None for a chunk with no Group; dropped from the payload, not nulled. + "group": group_for_instance(resolved_instance), "type": "scheme" if is_scheme else "document", "source": "docs-pipeline", "filename": external_slug, @@ -817,6 +824,7 @@ def _passage_schema_definition(use_tensor_prefix_field: bool = True) -> dict: {"name": "workflow_id", "type": "text", "features": ["filter"]}, {"name": "instance", "type": "text", "features": ["filter"]}, {"name": "instance_name", "type": "text", "features": ["filter"]}, + {"name": "group", "type": "text", "features": ["filter"]}, {"name": "type", "type": "text", "features": ["filter"]}, {"name": "source", "type": "text", "features": ["filter"]}, {"name": "filename", "type": "text", "features": ["filter"]}, @@ -1456,27 +1464,39 @@ 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, read here rather than - passed in as an activity argument - that keeps every workflow call site, and - any workflow already in flight, untouched. + The catalog depends on the document's knowledge kind and Group, 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 repository = DocumentRepository() document_kind = normalize_document_kind(repository.get_document_kind(workflow_id)) + group = repository.get_document_group(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, group ) if result["skipped"]: artifact_type = "network_publish_skipped" filename = "network_publish_skipped.json" + skip_reason = result.get("skip_reason") or SKIP_UNMAPPED_KIND + if skip_reason == SKIP_GROUP_PROVIDER_NOT_CONFIGURED: + reason = ( + "Not published: no network provider is configured for " + f"{instance_display_name(group)} ({group})" + ) + activity.logger.warning("workflow_id=%s %s", workflow_id, reason) + else: + reason = "no network catalog is mapped to this document kind" exchange = { "skipped": True, "document_kind": document_kind, - "reason": "no network catalog is mapped to this document kind", + "group": group, + "reason_code": skip_reason, + "reason": reason, } else: artifact_type = "network_publish_payload" diff --git a/pipeline/api.py b/pipeline/api.py index 747704f..d37cf03 100644 --- a/pipeline/api.py +++ b/pipeline/api.py @@ -103,6 +103,7 @@ SearchSettingsUpdate, SettingsAuditResponse, ) +from .network_group import load_group_provider_ids from .workflows import ( ChunkingOnlyWorkflow, DocumentPipelineWorkflow, @@ -137,6 +138,10 @@ async def lifespan(app: FastAPI): """Initialize Temporal and MinIO clients on startup.""" global temporal_client, minio_client, MINIO_BUCKET + # Only the worker publishes, but a malformed Group Provider map is a bad + # deploy - fail it here too rather than leave half the stack up (ADR 0006). + load_group_provider_ids() + auth_cfg = load_auth_config() validate_auth_config(auth_cfg) if auth_cfg.disabled: diff --git a/pipeline/catalog_builder.py b/pipeline/catalog_builder.py index 7b7da27..9791abb 100644 --- a/pipeline/catalog_builder.py +++ b/pipeline/catalog_builder.py @@ -3,14 +3,18 @@ One catalog holding exactly one OnDemand resource per kind, identical for every 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. +actual question, so there is nothing per-document to put on the wire. A Group's +documents get that same catalog under the Group's own provider, with ids +prefixed by the group code; documents with no Group keep the Shared Provider. 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. +docs/ADR/0003-static-ondemand-network-catalog-per-knowledge-kind.md and +docs/ADR/0006-publish-to-network-under-per-group-provider.md. """ import copy +from dataclasses import dataclass from typing import Optional from .network_constants import ( @@ -57,6 +61,7 @@ _KNOWLEDGE_KIND_CATALOGS = { "advisory": { + "group_label": "Knowledge Advisories", "catalog_id": ADVISORY_CATALOG_ID, "catalog_name": ADVISORY_CATALOG_NAME, "resource_id": ADVISORY_RESOURCE_ID, @@ -64,6 +69,7 @@ "resource_attributes": _ADVISORY_RESOURCE_ATTRIBUTES, }, "scheme": { + "group_label": "Knowledge Schemes", "catalog_id": SCHEMES_CATALOG_ID, "catalog_name": SCHEMES_CATALOG_NAME, "resource_id": SCHEMES_RESOURCE_ID, @@ -75,14 +81,30 @@ PUBLISHABLE_DOCUMENT_KINDS = tuple(_KNOWLEDGE_KIND_CATALOGS) +@dataclass(frozen=True) +class GroupProvider: + """The Group a catalog is announced for, and the provider id it publishes as.""" + + code: str + name: str + provider_id: str + + def normalize_document_kind(document_kind: Optional[str]) -> str: """Coerce a stored `documents.document_kind` to its lookup form.""" 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], + group_provider: Optional[GroupProvider] = None, +) -> Optional[dict]: """Return the catalog to announce for `document_kind`, or None. + With a `group_provider`, the catalog and resource ids carry the group code + and both catalog and provider are named for the Group; without one, the + catalog is announced under the Shared Provider exactly as before. + 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`). @@ -91,14 +113,31 @@ def build_catalog(document_kind: Optional[str]) -> Optional[dict]: if spec is None: return None + catalog_id = spec["catalog_id"] + catalog_name = spec["catalog_name"] + resource_id = spec["resource_id"] + provider = copy.deepcopy(CATALOG_PROVIDER) + if group_provider is not None: + prefix = f"{group_provider.code}-" + catalog_id = prefix + catalog_id + resource_id = prefix + resource_id + catalog_name = f"{group_provider.name} - {spec['group_label']}" + provider = { + "id": group_provider.provider_id, + "descriptor": { + "code": prefix + CATALOG_PROVIDER["descriptor"]["code"], + "name": catalog_name, + }, + } + catalog = { - "id": spec["catalog_id"], - "descriptor": {"name": spec["catalog_name"]}, + "id": catalog_id, + "descriptor": {"name": catalog_name}, "isActive": True, - "provider": copy.deepcopy(CATALOG_PROVIDER), + "provider": provider, "resources": [ { - "id": spec["resource_id"], + "id": resource_id, "descriptor": {"name": spec["resource_name"]}, "resourceAttributes": copy.deepcopy(spec["resource_attributes"]), } diff --git a/pipeline/discovery_publish_service.py b/pipeline/discovery_publish_service.py index d34f2b8..0fb313d 100644 --- a/pipeline/discovery_publish_service.py +++ b/pipeline/discovery_publish_service.py @@ -16,11 +16,18 @@ import httpx -from .catalog_builder import build_catalog +from .catalog_builder import GroupProvider, build_catalog +from .instances import instance_display_name from .network_constants import BECKN_VERSION, RESULT_ACCEPTED +from .network_group import load_group_provider_ids logger = logging.getLogger(__name__) +# Why a publish made no HTTP call. An unmapped kind is the normal case; a Group +# with no provider configured is a deployment gap an operator must see. +SKIP_UNMAPPED_KIND = "unmapped_kind" +SKIP_GROUP_PROVIDER_NOT_CONFIGURED = "group_provider_not_configured" + class DiscoveryPublishService: """POSTs a Beckn-shaped catalog/publish envelope to the Discovery Service.""" @@ -41,15 +48,19 @@ def __init__( raise RuntimeError("NETWORK_SENDER_ID must be set to publish to the network.") self.timeout = timeout + # Raises on a malformed map; the API and worker also check it at boot. + self.group_provider_ids = load_group_provider_ids() def publish( self, transaction_id: str, document_kind: Optional[str], workflow_id: Optional[str] = None, + group: Optional[str] = None, ) -> dict: """ - POST a catalog/publish envelope for `document_kind`. + POST a catalog/publish envelope for `document_kind`, announced under + `group`'s provider, or under the Shared Provider when `group` is None. `transaction_id` is supplied by the caller (generated once per publish, reused across retries) - only `messageId` is generated fresh here, per @@ -58,22 +69,37 @@ def publish( 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. + So does a Group with no provider configured - publishing it under the + Shared Provider would put the state's knowledge under the wrong identity. """ - catalog = build_catalog(document_kind) - if catalog is None: + if build_catalog(document_kind) is None: logger.info( "workflow_id=%s document_kind=%s network_publish_skipped=True", workflow_id, document_kind, ) - return { - "skipped": True, - "envelope": None, - "status_code": None, - "response_body": None, - "result_status": None, - "errors": [], - } + return self._skipped(SKIP_UNMAPPED_KIND) + + group_provider = None + if group is not None: + provider_id = self.group_provider_ids.get(group) + if provider_id is None: + logger.warning( + "workflow_id=%s document_kind=%s group=%s network_publish_skipped=True " + "reason=%s", + workflow_id, + document_kind, + group, + SKIP_GROUP_PROVIDER_NOT_CONFIGURED, + ) + return self._skipped(SKIP_GROUP_PROVIDER_NOT_CONFIGURED) + group_provider = GroupProvider( + code=group, + name=instance_display_name(group), + provider_id=provider_id, + ) + + catalog = build_catalog(document_kind, group_provider) envelope = { "context": { @@ -148,6 +174,7 @@ def publish( result_status, errors = self._read_result(response, catalog["id"], workflow_id) return { "skipped": False, + "skip_reason": None, "envelope": envelope, "status_code": response.status_code, "response_body": response.text, @@ -155,6 +182,18 @@ def publish( "errors": errors, } + @staticmethod + def _skipped(reason: str) -> dict: + return { + "skipped": True, + "skip_reason": reason, + "envelope": None, + "status_code": None, + "response_body": None, + "result_status": None, + "errors": [], + } + def _read_result( self, response: httpx.Response, diff --git a/pipeline/document_repository.py b/pipeline/document_repository.py index 9f2e2c3..573d0f8 100644 --- a/pipeline/document_repository.py +++ b/pipeline/document_repository.py @@ -9,6 +9,7 @@ from typing import Optional from . import db +from .network_group import group_for_instance class DocumentRepository: @@ -28,3 +29,14 @@ def get_document_kind(self, workflow_id: str) -> Optional[str]: if not doc: return None return doc.get("document_kind") + + def get_document_group(self, workflow_id: str) -> Optional[str]: + """The document's group code, or None when it has no Group. + + Taken from the document's own `instance`, never the uploader's, so a + Super Admin uploading for a state files it under that state's Group. + """ + doc = self._db.get_document(workflow_id) + if not doc: + return None + return group_for_instance(doc.get("instance")) diff --git a/pipeline/network_constants.py b/pipeline/network_constants.py index 422a946..9df344f 100644 --- a/pipeline/network_constants.py +++ b/pipeline/network_constants.py @@ -33,8 +33,9 @@ DEFAULT_DOCUMENT_KIND = "document" -# Same provider on every catalog - this pipeline publishes as one provider -# regardless of knowledge kind. +# The Shared Provider: every catalog of a document with no Group. A Group's +# documents publish under that Group's provider instead (ADR 0006), whose +# descriptor code is this one prefixed with the group code. CATALOG_PROVIDER = { "id": "knowledge-provider", "descriptor": { diff --git a/pipeline/network_group.py b/pipeline/network_group.py new file mode 100644 index 0000000..e9770ae --- /dev/null +++ b/pipeline/network_group.py @@ -0,0 +1,84 @@ +""" +Group and Group Provider resolution for Publish to Network. + +A Group is the state a document belongs to, named by its state code and taken +from the document's `instance`. The portal instance (Bharat Vistaar) and the +default instance have no Group: their documents keep the Shared Provider. See +docs/ADR/0006-publish-to-network-under-per-group-provider.md. + +New features say Group; existing screens, roles and data still say state or +instance, and this module is the one place that maps between them. +""" + +from __future__ import annotations + +import json +import os +from typing import Optional + +from .instances import PORTAL_INSTANCE + +# JSON object of group code -> Group Provider id, e.g. {"mh": "mh.provider.oan.dev"}. +GROUP_PROVIDER_IDS_ENV = "NETWORK_GROUP_PROVIDER_IDS" + + +class GroupProviderConfigError(ValueError): + """The Group Provider map is malformed; the API and worker refuse to boot.""" + + +def _no_group_instances() -> set[str]: + # Follow the configured values, so a deployment that renames its portal or + # default instance never mistakes it for a state. + portal = (os.environ.get("PORTAL_INSTANCE") or PORTAL_INSTANCE).strip().lower() + default = (os.environ.get("DEFAULT_INSTANCE") or "default").strip().lower() + return {portal or PORTAL_INSTANCE, default or "default"} + + +def group_for_instance(instance: Optional[str]) -> Optional[str]: + """The document's group code, or None when its instance has no Group.""" + code = (instance or "").strip().lower() + if not code or code in _no_group_instances(): + return None + return code + + +def parse_group_provider_ids(raw: Optional[str]) -> dict[str, str]: + """Parse the Group Provider map; unset or blank means no Group has a provider. + + Keys are lowercased so `MH` and `mh` name the same Group. Anything other + than an object of non-empty strings raises, rather than silently skipping + every Group's publish. + """ + if raw is None or not raw.strip(): + return {} + + try: + parsed = json.loads(raw) + except json.JSONDecodeError as exc: + raise GroupProviderConfigError(f"{GROUP_PROVIDER_IDS_ENV} is not valid JSON: {exc}") from exc + + if not isinstance(parsed, dict): + raise GroupProviderConfigError( + f"{GROUP_PROVIDER_IDS_ENV} must be a JSON object of group code to provider id" + ) + + provider_ids: dict[str, str] = {} + for key, value in parsed.items(): + code = key.strip().lower() if isinstance(key, str) else "" + provider_id = value.strip() if isinstance(value, str) else "" + if not code or not provider_id: + raise GroupProviderConfigError( + f"{GROUP_PROVIDER_IDS_ENV} entry {key!r}: {value!r} must map a group code " + "to a non-empty provider id" + ) + if code in provider_ids: + raise GroupProviderConfigError( + f"{GROUP_PROVIDER_IDS_ENV} names group {code!r} more than once" + ) + provider_ids[code] = provider_id + return provider_ids + + +def load_group_provider_ids() -> dict[str, str]: + """The Group Provider map from the environment; raises if malformed.""" + return parse_group_provider_ids(os.environ.get(GROUP_PROVIDER_IDS_ENV)) diff --git a/pipeline/vector_store/qdrant_store.py b/pipeline/vector_store/qdrant_store.py index 844154c..1072749 100644 --- a/pipeline/vector_store/qdrant_store.py +++ b/pipeline/vector_store/qdrant_store.py @@ -23,6 +23,9 @@ "workflow_id", "instance", "instance_name", + # Group code (ADR 0006); absent on chunks with no Group and on chunks + # ingested before it existed. + "group", "type", "source", "filename", @@ -281,6 +284,7 @@ def today(self) -> str: "workflow_id": qmodels.PayloadSchemaType.KEYWORD, "filename": qmodels.PayloadSchemaType.KEYWORD, "instance": qmodels.PayloadSchemaType.KEYWORD, + "group": qmodels.PayloadSchemaType.KEYWORD, "chunk_num": qmodels.PayloadSchemaType.INTEGER, "is_reference": qmodels.PayloadSchemaType.BOOL, "type": qmodels.PayloadSchemaType.KEYWORD, diff --git a/pipeline/worker.py b/pipeline/worker.py index 0d39634..8a3721a 100644 --- a/pipeline/worker.py +++ b/pipeline/worker.py @@ -26,6 +26,7 @@ run_ocr_and_store, update_document_state, ) +from .network_group import load_group_provider_ids from .workflows import ( ChunkingOnlyWorkflow, DocumentPipelineWorkflow, @@ -53,6 +54,10 @@ async def main(): """Start the worker.""" + # Refuse to boot on a malformed Group Provider map rather than discover it + # on the first publish (ADR 0006). + load_group_provider_ids() + translation_provider = os.environ.get("TRANSLATION_PROVIDER", "gemma_vllm").strip().lower() if translation_provider in {"gemma_vllm", "gemma4", "gemma"}: if not os.environ.get("TRANSLATION_VLLM_BASE_URL", "").strip(): diff --git a/tests/test_activities.py b/tests/test_activities.py index 79904c0..370ec84 100644 --- a/tests/test_activities.py +++ b/tests/test_activities.py @@ -163,6 +163,25 @@ def test_prepare_records_carries_instance_name(self): assert records[0]["instance"] == code assert records[0]["instance_name"] == expected + @pytest.mark.unit + def test_prepare_records_carries_group_beside_instance(self, monkeypatch): + """A state chunk carries its Group; a platform chunk has no group field.""" + from pipeline.activities import _prepare_records + + monkeypatch.delenv("PORTAL_INSTANCE", raising=False) + monkeypatch.delenv("DEFAULT_INSTANCE", raising=False) + chunks = [{"chunk_number": 1, "original_text": "Test chunk", "token_count": 5}] + + for code, expected in (("mh", "mh"), ("bv", None), ("default", None)): + record = _prepare_records( + document_id="test-doc", + filename="test.pdf", + chunks=chunks, + instance=code, + )[0] + assert record["instance"] == code + assert record["group"] == expected + @pytest.mark.unit def test_prepare_records_fills_scheme_aliases(self): """A scheme must never index with scheme_aliases: [].""" @@ -526,7 +545,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, group=None): assert transaction_id == "txn-1" assert document_kind == "advisory" assert workflow_id == "wf-1" @@ -574,7 +593,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, group=None): raise RuntimeError("discovery service unreachable") monkeypatch.setattr(activities, "DiscoveryPublishService", FakeService) @@ -596,7 +615,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, group=None): return { "skipped": True, "envelope": None, @@ -641,7 +660,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, group=None): return { "skipped": False, "envelope": {"context": {"messageId": "msg-3"}}, @@ -675,7 +694,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, group=None): return { "skipped": False, "envelope": {"context": {"messageId": "msg-4"}}, @@ -706,6 +725,122 @@ def publish(self, transaction_id, document_kind, workflow_id=None): assert recorded["metadata"]["errors"] == errors +class TestPublishCatalogToNetworkGroups: + """The activity publishes under the document's Group (ADR 0006).""" + + @staticmethod + def _wire(monkeypatch, document, service_result, seen): + import pipeline.activities as activities + import pipeline.db as db + + class FakeService: + def publish(self, transaction_id, document_kind, workflow_id=None, group=None): + seen["group"] = group + return service_result + + monkeypatch.setattr(activities, "DiscoveryPublishService", FakeService) + monkeypatch.setattr( + activities, + "_upload_file_to_minio", + lambda *a, **k: ("minio://documents/x.json", 1, "application/json"), + ) + monkeypatch.setattr(db, "get_document", lambda workflow_id: document) + monkeypatch.setattr(db, "get_latest_document_job", lambda workflow_id: None) + recorded = {} + monkeypatch.setattr(db, "add_document_artifact", lambda **kwargs: recorded.update(kwargs)) + return recorded + + _PUBLISHED = { + "skipped": False, + "envelope": {"context": {"messageId": "msg-g"}}, + "status_code": 200, + "response_body": "{}", + "result_status": "ACCEPTED", + "errors": [], + } + + @pytest.mark.unit + @pytest.mark.asyncio + @pytest.mark.parametrize("instance,group", [("mh", "mh"), ("MH", "mh"), ("bv", None), ("default", None)]) + async def test_passes_the_documents_group(self, monkeypatch, instance, group): + import pipeline.activities as activities + + monkeypatch.delenv("PORTAL_INSTANCE", raising=False) + monkeypatch.delenv("DEFAULT_INSTANCE", raising=False) + seen = {} + self._wire( + monkeypatch, + {"document_kind": "advisory", "instance": instance}, + self._PUBLISHED, + seen, + ) + + await activities.publish_catalog_to_network("wf-g", "txn-g") + + assert seen["group"] == group + + @pytest.mark.unit + @pytest.mark.asyncio + async def test_unconfigured_group_records_a_warning_skip(self, monkeypatch, caplog): + import logging + + import pipeline.activities as activities + from pipeline.discovery_publish_service import SKIP_GROUP_PROVIDER_NOT_CONFIGURED + + skipped = { + "skipped": True, + "skip_reason": SKIP_GROUP_PROVIDER_NOT_CONFIGURED, + "envelope": None, + "status_code": None, + "response_body": None, + "result_status": None, + "errors": [], + } + recorded = self._wire( + monkeypatch, + {"document_kind": "advisory", "instance": "mh"}, + skipped, + {}, + ) + + with caplog.at_level(logging.WARNING): + result = await activities.publish_catalog_to_network("wf-s", "txn-s") + + assert result["status"] == "skipped" + assert recorded["artifact_type"] == "network_publish_skipped" + metadata = recorded["metadata"] + assert metadata["reason_code"] == SKIP_GROUP_PROVIDER_NOT_CONFIGURED + assert metadata["group"] == "mh" + assert "Maharashtra (mh)" in metadata["reason"] + assert any("Maharashtra (mh)" in r.getMessage() for r in caplog.records if r.levelno == logging.WARNING) + + @pytest.mark.unit + @pytest.mark.asyncio + async def test_unmapped_kind_skip_carries_its_reason_code(self, monkeypatch): + import pipeline.activities as activities + from pipeline.discovery_publish_service import SKIP_UNMAPPED_KIND + + skipped = { + "skipped": True, + "skip_reason": SKIP_UNMAPPED_KIND, + "envelope": None, + "status_code": None, + "response_body": None, + "result_status": None, + "errors": [], + } + recorded = self._wire( + monkeypatch, + {"document_kind": "document", "instance": "mh"}, + skipped, + {}, + ) + + await activities.publish_catalog_to_network("wf-u", "txn-u") + + assert recorded["metadata"]["reason_code"] == SKIP_UNMAPPED_KIND + + class TestWorkerActivityRegistration: """ Every @activity.defn in activities.py must be registered on the worker diff --git a/tests/test_catalog_builder.py b/tests/test_catalog_builder.py index 24fbfa4..8deea56 100644 --- a/tests/test_catalog_builder.py +++ b/tests/test_catalog_builder.py @@ -3,6 +3,7 @@ import pytest from pipeline.catalog_builder import ( + GroupProvider, build_catalog, normalize_document_kind, ) @@ -149,3 +150,59 @@ def test_mutating_a_built_catalogs_provider_does_not_leak_into_the_next(self): second = build_catalog("advisory") assert second["provider"] == CATALOG_PROVIDER + + +MAHARASHTRA = GroupProvider(code="mh", name="Maharashtra", provider_id="mh.provider.oan.dev") + + +class TestGroupProviderCatalog: + """A Group's documents are announced under its own provider (ADR 0006).""" + + @pytest.mark.unit + @pytest.mark.parametrize( + "kind,catalog_id,resource_id,name", + [ + ( + "advisory", + "mh-cat-oan-knowledge-provider-advisories", + "mh-res-oan-knowledge-provider-advisories", + "Maharashtra - Knowledge Advisories", + ), + ( + "scheme", + "mh-cat-oan-knowledge-provider-schemes", + "mh-res-oan-knowledge-provider-schemes", + "Maharashtra - Knowledge Schemes", + ), + ], + ) + def test_ids_and_names_carry_the_group(self, kind, catalog_id, resource_id, name): + catalog = build_catalog(kind, MAHARASHTRA) + + assert catalog["id"] == catalog_id + assert catalog["descriptor"] == {"name": name} + assert catalog["resources"][0]["id"] == resource_id + assert catalog["provider"] == { + "id": "mh.provider.oan.dev", + "descriptor": {"code": "mh-oan-knowledge-provider", "name": name}, + } + + @pytest.mark.unit + @pytest.mark.parametrize("kind", ["advisory", "scheme"]) + def test_resource_attributes_match_the_shared_provider(self, kind): + grouped = build_catalog(kind, MAHARASHTRA)["resources"][0] + shared = build_catalog(kind)["resources"][0] + + assert grouped["resourceAttributes"] == shared["resourceAttributes"] + assert grouped["descriptor"] == shared["descriptor"] + + @pytest.mark.unit + def test_unmapped_kind_still_publishes_nothing(self): + assert build_catalog("document", MAHARASHTRA) is None + + @pytest.mark.unit + def test_no_group_keeps_the_shared_provider(self): + catalog = build_catalog("advisory", None) + + assert catalog["id"] == ADVISORY_CATALOG_ID + assert catalog["provider"] == CATALOG_PROVIDER diff --git a/tests/test_discovery_publish_service.py b/tests/test_discovery_publish_service.py index 30052a9..7f66882 100644 --- a/tests/test_discovery_publish_service.py +++ b/tests/test_discovery_publish_service.py @@ -439,3 +439,106 @@ 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 TestDiscoveryPublishServiceGroupProvider: + """A Group's documents publish under its Group Provider (ADR 0006).""" + + @pytest.mark.unit + def test_configured_group_publishes_under_its_own_provider(self, monkeypatch): + from pipeline.discovery_publish_service import DiscoveryPublishService + + monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") + monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") + monkeypatch.setenv("NETWORK_GROUP_PROVIDER_IDS", '{"MH": "mh.provider.oan.dev"}') + service = DiscoveryPublishService() + client = _client_returning( + _on_publish_response("mh-cat-oan-knowledge-provider-advisories", "ACCEPTED") + ) + + with patch("pipeline.discovery_publish_service.httpx.Client", return_value=client): + result = service.publish(transaction_id="txn-mh", document_kind="advisory", group="mh") + + envelope = client.post.call_args.kwargs["json"] + catalog = envelope["message"]["catalogs"][0] + assert catalog["id"] == "mh-cat-oan-knowledge-provider-advisories" + assert catalog["resources"][0]["id"] == "mh-res-oan-knowledge-provider-advisories" + assert catalog["provider"]["id"] == "mh.provider.oan.dev" + assert catalog["provider"]["descriptor"]["name"] == "Maharashtra - Knowledge Advisories" + assert envelope["message"]["publishDirectives"][0]["catalogId"] == catalog["id"] + # One publish address for every Group. + assert client.post.call_args.args[0] == "https://discovery.example.com/publish" + assert result["skipped"] is False + assert result["result_status"] == "ACCEPTED" + + @pytest.mark.unit + def test_unconfigured_group_is_skipped_without_an_http_call(self, monkeypatch): + from pipeline.discovery_publish_service import ( + SKIP_GROUP_PROVIDER_NOT_CONFIGURED, + DiscoveryPublishService, + ) + + monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") + monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") + monkeypatch.setenv("NETWORK_GROUP_PROVIDER_IDS", '{"mh": "mh.provider.oan.dev"}') + service = DiscoveryPublishService() + client = MagicMock() + + with patch("pipeline.discovery_publish_service.httpx.Client", return_value=client): + result = service.publish(transaction_id="txn-ka", document_kind="advisory", group="ka") + + client.post.assert_not_called() + assert result["skipped"] is True + assert result["skip_reason"] == SKIP_GROUP_PROVIDER_NOT_CONFIGURED + assert result["envelope"] is None + + @pytest.mark.unit + def test_unmapped_kind_wins_over_an_unconfigured_group(self, monkeypatch): + # Not announcing a 'document' is normal - it must not raise the + # misconfiguration warning just because its Group has no provider. + from pipeline.discovery_publish_service import ( + SKIP_UNMAPPED_KIND, + DiscoveryPublishService, + ) + + monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") + monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") + monkeypatch.delenv("NETWORK_GROUP_PROVIDER_IDS", raising=False) + service = DiscoveryPublishService() + + result = service.publish(transaction_id="txn-doc", document_kind="document", group="ka") + + assert result["skipped"] is True + assert result["skip_reason"] == SKIP_UNMAPPED_KIND + + @pytest.mark.unit + def test_no_group_publishes_under_the_shared_provider(self, monkeypatch): + from pipeline.discovery_publish_service import DiscoveryPublishService + from pipeline.network_constants import CATALOG_PROVIDER + + monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") + monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") + monkeypatch.setenv("NETWORK_GROUP_PROVIDER_IDS", '{"mh": "mh.provider.oan.dev"}') + service = DiscoveryPublishService() + 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-bv", document_kind="advisory", group=None) + + catalog = client.post.call_args.kwargs["json"]["message"]["catalogs"][0] + assert catalog["id"] == "cat-oan-knowledge-provider-advisories" + assert catalog["provider"] == CATALOG_PROVIDER + + @pytest.mark.unit + def test_malformed_provider_map_fails_construction(self, monkeypatch): + from pipeline.discovery_publish_service import DiscoveryPublishService + from pipeline.network_group import GroupProviderConfigError + + monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") + monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") + monkeypatch.setenv("NETWORK_GROUP_PROVIDER_IDS", "{broken") + + with pytest.raises(GroupProviderConfigError): + DiscoveryPublishService() diff --git a/tests/test_document_repository.py b/tests/test_document_repository.py index dbf6c4f..ae6dba4 100644 --- a/tests/test_document_repository.py +++ b/tests/test_document_repository.py @@ -76,3 +76,26 @@ 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 TestGetDocumentGroup: + @pytest.fixture(autouse=True) + def _default_no_group_instances(self, monkeypatch): + monkeypatch.delenv("PORTAL_INSTANCE", raising=False) + monkeypatch.delenv("DEFAULT_INSTANCE", raising=False) + + @pytest.mark.unit + @pytest.mark.parametrize( + "instance,group", + [("mh", "mh"), ("MH", "mh"), ("bv", None), ("default", None), (None, None)], + ) + def test_group_comes_from_the_documents_instance(self, instance, group): + repo = DocumentRepository(FakeDb(document={"instance": instance})) + + assert repo.get_document_group("wf-1") == group + + @pytest.mark.unit + def test_returns_none_for_a_missing_document(self): + repo = DocumentRepository(FakeDb(document=None)) + + assert repo.get_document_group("wf-nope") is None diff --git a/tests/test_network_group.py b/tests/test_network_group.py new file mode 100644 index 0000000..fd3c17d --- /dev/null +++ b/tests/test_network_group.py @@ -0,0 +1,131 @@ +"""Unit tests for Group resolution and the Group Provider map (ADR 0006).""" + +import pytest + +from pipeline.network_group import ( + GROUP_PROVIDER_IDS_ENV, + GroupProviderConfigError, + group_for_instance, + load_group_provider_ids, + parse_group_provider_ids, +) + + +@pytest.fixture(autouse=True) +def _default_no_group_instances(monkeypatch): + monkeypatch.delenv("PORTAL_INSTANCE", raising=False) + monkeypatch.delenv("DEFAULT_INSTANCE", raising=False) + + +class TestGroupForInstance: + @pytest.mark.unit + @pytest.mark.parametrize( + "instance,expected", + [ + ("mh", "mh"), + (" MH ", "mh"), + ("ka", "ka"), + ("bv", None), + ("BV", None), + ("default", None), + ("", None), + (None, None), + ], + ) + def test_states_are_groups_and_platform_instances_are_not(self, instance, expected): + assert group_for_instance(instance) == expected + + @pytest.mark.unit + def test_follows_a_configured_portal_instance(self, monkeypatch): + monkeypatch.setenv("PORTAL_INSTANCE", "portal") + + assert group_for_instance("portal") is None + assert group_for_instance("mh") == "mh" + + @pytest.mark.unit + def test_follows_a_configured_default_instance(self, monkeypatch): + monkeypatch.setenv("DEFAULT_INSTANCE", "local") + + assert group_for_instance("local") is None + assert group_for_instance("mh") == "mh" + + +class TestParseGroupProviderIds: + @pytest.mark.unit + @pytest.mark.parametrize("raw", [None, "", " "]) + def test_unset_means_no_group_has_a_provider(self, raw): + assert parse_group_provider_ids(raw) == {} + + @pytest.mark.unit + def test_keys_are_case_insensitive_and_values_trimmed(self): + parsed = parse_group_provider_ids('{"MH": " mh.provider.oan.dev ", "ka": "ka.provider.oan.dev"}') + + assert parsed == {"mh": "mh.provider.oan.dev", "ka": "ka.provider.oan.dev"} + + @pytest.mark.unit + @pytest.mark.parametrize( + "raw", + [ + "not json", + '["mh.provider.oan.dev"]', + '"mh.provider.oan.dev"', + '{"mh": 42}', + '{"mh": ""}', + '{"": "mh.provider.oan.dev"}', + '{"MH": "a", "mh": "b"}', + ], + ) + def test_malformed_map_is_rejected(self, raw): + with pytest.raises(GroupProviderConfigError, match=GROUP_PROVIDER_IDS_ENV): + parse_group_provider_ids(raw) + + +class TestLoadGroupProviderIds: + @pytest.mark.unit + def test_reads_the_environment(self, monkeypatch): + monkeypatch.setenv(GROUP_PROVIDER_IDS_ENV, '{"mh": "mh.provider.oan.dev"}') + + assert load_group_provider_ids() == {"mh": "mh.provider.oan.dev"} + + @pytest.mark.unit + def test_malformed_environment_raises(self, monkeypatch): + monkeypatch.setenv(GROUP_PROVIDER_IDS_ENV, "{broken") + + with pytest.raises(GroupProviderConfigError): + load_group_provider_ids() + + +class TestBootRefusesMalformedProviderMap: + """Both the API and the worker refuse to start on a malformed map.""" + + @pytest.fixture + def broken_map(self, monkeypatch): + monkeypatch.setenv(GROUP_PROVIDER_IDS_ENV, "{broken") + monkeypatch.setenv("TRANSLATION_PROVIDER", "none") + + @pytest.mark.unit + @pytest.mark.asyncio + async def test_worker_does_not_start(self, broken_map, monkeypatch): + import pipeline.worker as worker + + async def never_connect(*args, **kwargs): + raise AssertionError("worker connected to Temporal despite a bad provider map") + + monkeypatch.setattr(worker.Client, "connect", never_connect) + + with pytest.raises(GroupProviderConfigError): + await worker.main() + + @pytest.mark.unit + @pytest.mark.asyncio + async def test_api_does_not_start(self, broken_map, monkeypatch): + import pipeline.api as api + + async def never_connect(*args, **kwargs): + raise AssertionError("API connected to Temporal despite a bad provider map") + + monkeypatch.setattr(api.Client, "connect", never_connect) + + with pytest.raises(GroupProviderConfigError): + async with api.lifespan(api.app): + pass diff --git a/tests/test_qdrant_group.py b/tests/test_qdrant_group.py new file mode 100644 index 0000000..444bccd --- /dev/null +++ b/tests/test_qdrant_group.py @@ -0,0 +1,38 @@ +"""Unit tests for the Group field on Qdrant chunk payloads (ADR 0006).""" + +import pytest +from qdrant_client.http import models as qmodels + +from pipeline.vector_store.qdrant_store import ( + PAYLOAD_FIELDS, + QdrantVectorStore, + _record_payload, +) + + +class TestGroupPayload: + @pytest.mark.unit + def test_carries_the_group_beside_the_instance(self): + payload = _record_payload( + {"text": "x", "instance": "mh", "instance_name": "Maharashtra", "group": "mh"} + ) + + assert payload["group"] == "mh" + assert payload["instance"] == "mh" + assert payload["instance_name"] == "Maharashtra" + + @pytest.mark.unit + def test_a_chunk_with_no_group_has_no_group_field(self): + # Absent, not null: Bharat Vistaar and 'default' chunks have no Group. + payload = _record_payload({"text": "x", "instance": "bv", "group": None}) + + assert "group" not in payload + assert payload["instance"] == "bv" + + @pytest.mark.unit + def test_group_is_a_declared_payload_field(self): + assert "group" in PAYLOAD_FIELDS + + @pytest.mark.unit + def test_group_is_indexed_as_a_keyword_like_instance(self): + assert QdrantVectorStore.PAYLOAD_INDEXES["group"] == qmodels.PayloadSchemaType.KEYWORD diff --git a/ui/src/lib/networkPublish.js b/ui/src/lib/networkPublish.js new file mode 100644 index 0000000..0d8287d --- /dev/null +++ b/ui/src/lib/networkPublish.js @@ -0,0 +1,29 @@ +// Publish to Network skip reasons, mirrored from +// pipeline/discovery_publish_service.py. +const GROUP_PROVIDER_NOT_CONFIGURED = 'group_provider_not_configured' + +function parseMetadata(artifact) { + try { + return JSON.parse(artifact.metadata_json || 'null') + } catch { + return null + } +} + +/** + * The warning to show when the document's latest Publish to Network was + * skipped because its Group has no network provider configured, else null. + * + * Only the latest attempt counts, so the notice clears once a re-run publishes. + * A skip for an unmapped knowledge kind is normal and never warns. + */ +export function groupProviderSkipReason(artifacts) { + const latest = (artifacts || []) + .filter(artifact => artifact.stage === 'publishing_to_network') + .reduce((newest, artifact) => (!newest || artifact.id > newest.id ? artifact : newest), null) + + if (latest?.artifact_type !== 'network_publish_skipped') return null + const metadata = parseMetadata(latest) + if (metadata?.reason_code !== GROUP_PROVIDER_NOT_CONFIGURED) return null + return metadata.reason || 'Not published: no network provider is configured for this group' +} diff --git a/ui/src/lib/networkPublish.test.js b/ui/src/lib/networkPublish.test.js new file mode 100644 index 0000000..324d729 --- /dev/null +++ b/ui/src/lib/networkPublish.test.js @@ -0,0 +1,46 @@ +import { describe, expect, it } from 'vitest' +import { groupProviderSkipReason } from './networkPublish' + +const artifact = (id, artifactType, metadata) => ({ + id, + artifact_type: artifactType, + stage: 'publishing_to_network', + metadata_json: metadata ? JSON.stringify(metadata) : null, +}) + +const GROUP_SKIP = { + skipped: true, + reason_code: 'group_provider_not_configured', + group: 'mh', + reason: 'Not published: no network provider is configured for Maharashtra (mh)', +} + +describe('groupProviderSkipReason', () => { + it('returns the reason when the latest publish skipped for a missing Group Provider', () => { + expect(groupProviderSkipReason([artifact(3, 'network_publish_skipped', GROUP_SKIP)])).toBe(GROUP_SKIP.reason) + }) + + it('ignores a skip for an unmapped knowledge kind', () => { + const kindSkip = { skipped: true, reason_code: 'unmapped_kind', reason: 'no network catalog is mapped' } + expect(groupProviderSkipReason([artifact(3, 'network_publish_skipped', kindSkip)])).toBeNull() + }) + + it('clears once a later publish went through', () => { + const artifacts = [ + artifact(2, 'network_publish_skipped', GROUP_SKIP), + artifact(5, 'network_publish_payload', { result_status: 'ACCEPTED' }), + ] + expect(groupProviderSkipReason(artifacts)).toBeNull() + }) + + it('ignores artifacts from other stages', () => { + const other = { ...artifact(9, 'chunk_export', { ok: true }), stage: 'chunking' } + expect(groupProviderSkipReason([artifact(3, 'network_publish_skipped', GROUP_SKIP), other])).toBe(GROUP_SKIP.reason) + }) + + it('tolerates missing or unreadable metadata', () => { + expect(groupProviderSkipReason(undefined)).toBeNull() + expect(groupProviderSkipReason([])).toBeNull() + expect(groupProviderSkipReason([{ ...artifact(1, 'network_publish_skipped'), metadata_json: '{bad' }])).toBeNull() + }) +}) diff --git a/ui/src/views/DocumentOpsView.jsx b/ui/src/views/DocumentOpsView.jsx index 892fabd..96462e2 100644 --- a/ui/src/views/DocumentOpsView.jsx +++ b/ui/src/views/DocumentOpsView.jsx @@ -54,6 +54,7 @@ import { summarizeAuditAction, summarizeAvailableAction, } from '../lib/pipelineUi' +import { groupProviderSkipReason } from '../lib/networkPublish' function getChunkEmptyMessage(doc) { const stage = doc?.stage @@ -851,6 +852,7 @@ 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 networkPublishWarning = groupProviderSkipReason(doc?.artifacts) 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]) @@ -1000,6 +1002,10 @@ export default function DocumentOpsView() { ) : null} + {networkPublishWarning ? ( + + ) : null} +
Date: Tue, 13 Oct 2026 17:13:59 +0530 Subject: [PATCH 02/23] fix: add-adr-0006 [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- ...ish-to-network-under-per-group-provider.md | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 docs/ADR/0006-publish-to-network-under-per-group-provider.md diff --git a/docs/ADR/0006-publish-to-network-under-per-group-provider.md b/docs/ADR/0006-publish-to-network-under-per-group-provider.md new file mode 100644 index 0000000..624515f --- /dev/null +++ b/docs/ADR/0006-publish-to-network-under-per-group-provider.md @@ -0,0 +1,23 @@ +--- +status: accepted +--- + +# Publish to Network under a per-Group provider + +Context: ADR 0003 announced every document under one provider, with one catalog per Knowledge Kind. A state's experience (for example Maharashtra's, through DSS) needs to consume only its own state's knowledge from the network, and one shared provider gives it no way to do that (engineering-tracker#190). + +Decision: +- Documents with a Group publish under that Group's provider. Its id is set per deployment, for example `mh.provider.oan.dev`. Catalog and resource ids are today's ids prefixed with the group code (`mh-cat-oan-knowledge-provider-advisories`, `mh-res-oan-knowledge-provider-advisories`, and the same for schemes). Both catalog and provider are named ` - Knowledge Advisories|Schemes`. +- The group code is derived from the document's `instance`, not from the uploader's group, so a Super Admin uploading for a state publishes under that state. +- Documents with no Group (Bharat Vistaar `bv`, `default`) keep the Shared Provider and ADR 0003's unprefixed ids, unchanged. +- A Group with no provider configured skips the publish with a warning, like an unmapped Knowledge Kind. It is not a failure. A real publish failure stays fatal, per ADR 0004. +- Only the provider id is configured per Group, as one JSON map in `NETWORK_GROUP_PROVIDER_IDS` (keys case-insensitive). The catalog and resource ids are derived, so onboarding a Group is one map entry. A malformed map stops both the API and the worker from booting, so a bad deploy can't quietly skip every Group. +- The provider descriptor code is the shared one prefixed with the group code (`mh-oan-knowledge-provider`). +- A Group skip is recorded with `reason_code: group_provider_not_configured` and shown as a warning on the document page, but only while it is the document's latest publish attempt. A Knowledge Kind skip shows nothing. +- "No Group" follows the configured `PORTAL_INSTANCE` and `DEFAULT_INSTANCE` values rather than hardcoding `bv` and `default`. +- The publish endpoint stays the same for every Group. + +Consequences: +- Ids are permanent under `MERGE` (ADR 0003). Documents already published under the Shared Provider are not moved, so state knowledge published before this change stays under the Shared Provider. A mistyped Group provider id leaves stranded entries on the network. +- Depends on the network adapter layer accepting the per-Group provider, catalog and resource ids. +- Chunks gain a `group` field (absent for Bharat Vistaar and `default`) alongside the unchanged `instance` and `instance_name`. Existing chunks are not backfilled, which #202's group filter must account for. From bfe8918457588fb9a539aba71eff361e0f074112 Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Tue, 13 Oct 2026 17:20:04 +0530 Subject: [PATCH 03/23] fix: remove-code-comments [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- pipeline/activities.py | 7 +++---- pipeline/api.py | 2 -- pipeline/catalog_builder.py | 13 ++----------- pipeline/discovery_publish_service.py | 8 +------- pipeline/document_repository.py | 5 ----- pipeline/network_constants.py | 5 ++--- pipeline/network_group.py | 25 +------------------------ pipeline/vector_store/qdrant_store.py | 2 -- pipeline/worker.py | 2 -- tests/test_activities.py | 3 --- tests/test_catalog_builder.py | 2 -- tests/test_discovery_publish_service.py | 5 ----- tests/test_network_group.py | 4 ---- tests/test_qdrant_group.py | 3 --- ui/src/lib/networkPublish.js | 9 --------- 15 files changed, 9 insertions(+), 86 deletions(-) diff --git a/pipeline/activities.py b/pipeline/activities.py index 8ce7112..cb1bef9 100644 --- a/pipeline/activities.py +++ b/pipeline/activities.py @@ -702,7 +702,6 @@ def _prepare_records( "workflow_id": workflow_id or "", "instance": resolved_instance, "instance_name": instance_name, - # None for a chunk with no Group; dropped from the payload, not nulled. "group": group_for_instance(resolved_instance), "type": "scheme" if is_scheme else "document", "source": "docs-pipeline", @@ -1464,9 +1463,9 @@ 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 Group, 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 diff --git a/pipeline/api.py b/pipeline/api.py index d37cf03..531364f 100644 --- a/pipeline/api.py +++ b/pipeline/api.py @@ -138,8 +138,6 @@ async def lifespan(app: FastAPI): """Initialize Temporal and MinIO clients on startup.""" global temporal_client, minio_client, MINIO_BUCKET - # Only the worker publishes, but a malformed Group Provider map is a bad - # deploy - fail it here too rather than leave half the stack up (ADR 0006). load_group_provider_ids() auth_cfg = load_auth_config() diff --git a/pipeline/catalog_builder.py b/pipeline/catalog_builder.py index 9791abb..6b50f5d 100644 --- a/pipeline/catalog_builder.py +++ b/pipeline/catalog_builder.py @@ -3,14 +3,11 @@ One catalog holding exactly one OnDemand resource per kind, identical for every 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. A Group's -documents get that same catalog under the Group's own provider, with ids -prefixed by the group code; documents with no Group keep the Shared Provider. +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 and the transport in discovery_publish_service.py. See -docs/ADR/0003-static-ondemand-network-catalog-per-knowledge-kind.md and -docs/ADR/0006-publish-to-network-under-per-group-provider.md. +docs/ADR/0003-static-ondemand-network-catalog-per-knowledge-kind.md. """ import copy @@ -83,8 +80,6 @@ @dataclass(frozen=True) class GroupProvider: - """The Group a catalog is announced for, and the provider id it publishes as.""" - code: str name: str provider_id: str @@ -101,10 +96,6 @@ def build_catalog( ) -> Optional[dict]: """Return the catalog to announce for `document_kind`, or None. - With a `group_provider`, the catalog and resource ids carry the group code - and both catalog and provider are named for the Group; without one, the - catalog is announced under the Shared Provider exactly as before. - 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`). diff --git a/pipeline/discovery_publish_service.py b/pipeline/discovery_publish_service.py index 0fb313d..fa3d3da 100644 --- a/pipeline/discovery_publish_service.py +++ b/pipeline/discovery_publish_service.py @@ -23,8 +23,6 @@ logger = logging.getLogger(__name__) -# Why a publish made no HTTP call. An unmapped kind is the normal case; a Group -# with no provider configured is a deployment gap an operator must see. SKIP_UNMAPPED_KIND = "unmapped_kind" SKIP_GROUP_PROVIDER_NOT_CONFIGURED = "group_provider_not_configured" @@ -48,7 +46,6 @@ def __init__( raise RuntimeError("NETWORK_SENDER_ID must be set to publish to the network.") self.timeout = timeout - # Raises on a malformed map; the API and worker also check it at boot. self.group_provider_ids = load_group_provider_ids() def publish( @@ -59,8 +56,7 @@ def publish( group: Optional[str] = None, ) -> dict: """ - POST a catalog/publish envelope for `document_kind`, announced under - `group`'s provider, or under the Shared Provider when `group` is None. + POST a catalog/publish envelope for `document_kind`. `transaction_id` is supplied by the caller (generated once per publish, reused across retries) - only `messageId` is generated fresh here, per @@ -69,8 +65,6 @@ def publish( 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. - So does a Group with no provider configured - publishing it under the - Shared Provider would put the state's knowledge under the wrong identity. """ if build_catalog(document_kind) is None: logger.info( diff --git a/pipeline/document_repository.py b/pipeline/document_repository.py index 573d0f8..cf83033 100644 --- a/pipeline/document_repository.py +++ b/pipeline/document_repository.py @@ -31,11 +31,6 @@ def get_document_kind(self, workflow_id: str) -> Optional[str]: return doc.get("document_kind") def get_document_group(self, workflow_id: str) -> Optional[str]: - """The document's group code, or None when it has no Group. - - Taken from the document's own `instance`, never the uploader's, so a - Super Admin uploading for a state files it under that state's Group. - """ doc = self._db.get_document(workflow_id) if not doc: return None diff --git a/pipeline/network_constants.py b/pipeline/network_constants.py index 9df344f..422a946 100644 --- a/pipeline/network_constants.py +++ b/pipeline/network_constants.py @@ -33,9 +33,8 @@ DEFAULT_DOCUMENT_KIND = "document" -# The Shared Provider: every catalog of a document with no Group. A Group's -# documents publish under that Group's provider instead (ADR 0006), whose -# descriptor code is this one prefixed with the group code. +# Same provider on every catalog - this pipeline publishes as one provider +# regardless of knowledge kind. CATALOG_PROVIDER = { "id": "knowledge-provider", "descriptor": { diff --git a/pipeline/network_group.py b/pipeline/network_group.py index e9770ae..c39f2e4 100644 --- a/pipeline/network_group.py +++ b/pipeline/network_group.py @@ -1,15 +1,3 @@ -""" -Group and Group Provider resolution for Publish to Network. - -A Group is the state a document belongs to, named by its state code and taken -from the document's `instance`. The portal instance (Bharat Vistaar) and the -default instance have no Group: their documents keep the Shared Provider. See -docs/ADR/0006-publish-to-network-under-per-group-provider.md. - -New features say Group; existing screens, roles and data still say state or -instance, and this module is the one place that maps between them. -""" - from __future__ import annotations import json @@ -18,24 +6,20 @@ from .instances import PORTAL_INSTANCE -# JSON object of group code -> Group Provider id, e.g. {"mh": "mh.provider.oan.dev"}. GROUP_PROVIDER_IDS_ENV = "NETWORK_GROUP_PROVIDER_IDS" class GroupProviderConfigError(ValueError): - """The Group Provider map is malformed; the API and worker refuse to boot.""" + pass def _no_group_instances() -> set[str]: - # Follow the configured values, so a deployment that renames its portal or - # default instance never mistakes it for a state. portal = (os.environ.get("PORTAL_INSTANCE") or PORTAL_INSTANCE).strip().lower() default = (os.environ.get("DEFAULT_INSTANCE") or "default").strip().lower() return {portal or PORTAL_INSTANCE, default or "default"} def group_for_instance(instance: Optional[str]) -> Optional[str]: - """The document's group code, or None when its instance has no Group.""" code = (instance or "").strip().lower() if not code or code in _no_group_instances(): return None @@ -43,12 +27,6 @@ def group_for_instance(instance: Optional[str]) -> Optional[str]: def parse_group_provider_ids(raw: Optional[str]) -> dict[str, str]: - """Parse the Group Provider map; unset or blank means no Group has a provider. - - Keys are lowercased so `MH` and `mh` name the same Group. Anything other - than an object of non-empty strings raises, rather than silently skipping - every Group's publish. - """ if raw is None or not raw.strip(): return {} @@ -80,5 +58,4 @@ def parse_group_provider_ids(raw: Optional[str]) -> dict[str, str]: def load_group_provider_ids() -> dict[str, str]: - """The Group Provider map from the environment; raises if malformed.""" return parse_group_provider_ids(os.environ.get(GROUP_PROVIDER_IDS_ENV)) diff --git a/pipeline/vector_store/qdrant_store.py b/pipeline/vector_store/qdrant_store.py index 1072749..5cc6d1a 100644 --- a/pipeline/vector_store/qdrant_store.py +++ b/pipeline/vector_store/qdrant_store.py @@ -23,8 +23,6 @@ "workflow_id", "instance", "instance_name", - # Group code (ADR 0006); absent on chunks with no Group and on chunks - # ingested before it existed. "group", "type", "source", diff --git a/pipeline/worker.py b/pipeline/worker.py index 8a3721a..0c7f2d7 100644 --- a/pipeline/worker.py +++ b/pipeline/worker.py @@ -54,8 +54,6 @@ async def main(): """Start the worker.""" - # Refuse to boot on a malformed Group Provider map rather than discover it - # on the first publish (ADR 0006). load_group_provider_ids() translation_provider = os.environ.get("TRANSLATION_PROVIDER", "gemma_vllm").strip().lower() diff --git a/tests/test_activities.py b/tests/test_activities.py index 370ec84..6c93948 100644 --- a/tests/test_activities.py +++ b/tests/test_activities.py @@ -165,7 +165,6 @@ def test_prepare_records_carries_instance_name(self): @pytest.mark.unit def test_prepare_records_carries_group_beside_instance(self, monkeypatch): - """A state chunk carries its Group; a platform chunk has no group field.""" from pipeline.activities import _prepare_records monkeypatch.delenv("PORTAL_INSTANCE", raising=False) @@ -726,8 +725,6 @@ def publish(self, transaction_id, document_kind, workflow_id=None, group=None): class TestPublishCatalogToNetworkGroups: - """The activity publishes under the document's Group (ADR 0006).""" - @staticmethod def _wire(monkeypatch, document, service_result, seen): import pipeline.activities as activities diff --git a/tests/test_catalog_builder.py b/tests/test_catalog_builder.py index 8deea56..be9df39 100644 --- a/tests/test_catalog_builder.py +++ b/tests/test_catalog_builder.py @@ -156,8 +156,6 @@ def test_mutating_a_built_catalogs_provider_does_not_leak_into_the_next(self): class TestGroupProviderCatalog: - """A Group's documents are announced under its own provider (ADR 0006).""" - @pytest.mark.unit @pytest.mark.parametrize( "kind,catalog_id,resource_id,name", diff --git a/tests/test_discovery_publish_service.py b/tests/test_discovery_publish_service.py index 7f66882..95ed359 100644 --- a/tests/test_discovery_publish_service.py +++ b/tests/test_discovery_publish_service.py @@ -442,8 +442,6 @@ def test_skipped_publish_logs_no_request(self, monkeypatch, caplog): class TestDiscoveryPublishServiceGroupProvider: - """A Group's documents publish under its Group Provider (ADR 0006).""" - @pytest.mark.unit def test_configured_group_publishes_under_its_own_provider(self, monkeypatch): from pipeline.discovery_publish_service import DiscoveryPublishService @@ -466,7 +464,6 @@ def test_configured_group_publishes_under_its_own_provider(self, monkeypatch): assert catalog["provider"]["id"] == "mh.provider.oan.dev" assert catalog["provider"]["descriptor"]["name"] == "Maharashtra - Knowledge Advisories" assert envelope["message"]["publishDirectives"][0]["catalogId"] == catalog["id"] - # One publish address for every Group. assert client.post.call_args.args[0] == "https://discovery.example.com/publish" assert result["skipped"] is False assert result["result_status"] == "ACCEPTED" @@ -494,8 +491,6 @@ def test_unconfigured_group_is_skipped_without_an_http_call(self, monkeypatch): @pytest.mark.unit def test_unmapped_kind_wins_over_an_unconfigured_group(self, monkeypatch): - # Not announcing a 'document' is normal - it must not raise the - # misconfiguration warning just because its Group has no provider. from pipeline.discovery_publish_service import ( SKIP_UNMAPPED_KIND, DiscoveryPublishService, diff --git a/tests/test_network_group.py b/tests/test_network_group.py index fd3c17d..96c56c5 100644 --- a/tests/test_network_group.py +++ b/tests/test_network_group.py @@ -1,5 +1,3 @@ -"""Unit tests for Group resolution and the Group Provider map (ADR 0006).""" - import pytest from pipeline.network_group import ( @@ -96,8 +94,6 @@ def test_malformed_environment_raises(self, monkeypatch): class TestBootRefusesMalformedProviderMap: - """Both the API and the worker refuse to start on a malformed map.""" - @pytest.fixture def broken_map(self, monkeypatch): monkeypatch.setenv(GROUP_PROVIDER_IDS_ENV, "{broken") diff --git a/tests/test_qdrant_group.py b/tests/test_qdrant_group.py index 444bccd..f6fb166 100644 --- a/tests/test_qdrant_group.py +++ b/tests/test_qdrant_group.py @@ -1,5 +1,3 @@ -"""Unit tests for the Group field on Qdrant chunk payloads (ADR 0006).""" - import pytest from qdrant_client.http import models as qmodels @@ -23,7 +21,6 @@ def test_carries_the_group_beside_the_instance(self): @pytest.mark.unit def test_a_chunk_with_no_group_has_no_group_field(self): - # Absent, not null: Bharat Vistaar and 'default' chunks have no Group. payload = _record_payload({"text": "x", "instance": "bv", "group": None}) assert "group" not in payload diff --git a/ui/src/lib/networkPublish.js b/ui/src/lib/networkPublish.js index 0d8287d..068b689 100644 --- a/ui/src/lib/networkPublish.js +++ b/ui/src/lib/networkPublish.js @@ -1,5 +1,3 @@ -// Publish to Network skip reasons, mirrored from -// pipeline/discovery_publish_service.py. const GROUP_PROVIDER_NOT_CONFIGURED = 'group_provider_not_configured' function parseMetadata(artifact) { @@ -10,13 +8,6 @@ function parseMetadata(artifact) { } } -/** - * The warning to show when the document's latest Publish to Network was - * skipped because its Group has no network provider configured, else null. - * - * Only the latest attempt counts, so the notice clears once a re-run publishes. - * A skip for an unmapped knowledge kind is normal and never warns. - */ export function groupProviderSkipReason(artifacts) { const latest = (artifacts || []) .filter(artifact => artifact.stage === 'publishing_to_network') From 2439803462fc35d218872e1cc9acf393f246f7c4 Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Tue, 13 Oct 2026 17:27:05 +0530 Subject: [PATCH 04/23] fix: parse-metadata-as-const [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- ui/src/lib/networkPublish.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/src/lib/networkPublish.js b/ui/src/lib/networkPublish.js index 068b689..cd0361a 100644 --- a/ui/src/lib/networkPublish.js +++ b/ui/src/lib/networkPublish.js @@ -1,6 +1,6 @@ const GROUP_PROVIDER_NOT_CONFIGURED = 'group_provider_not_configured' -function parseMetadata(artifact) { +const parseMetadata = artifact => { try { return JSON.parse(artifact.metadata_json || 'null') } catch { From 8db40a6cddac9455f9aa12f8d5729af6165b2d4e Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Tue, 13 Oct 2026 17:56:09 +0530 Subject: [PATCH 05/23] fix: skip-reason-as-const [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- ui/src/lib/networkPublish.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/src/lib/networkPublish.js b/ui/src/lib/networkPublish.js index cd0361a..9c989e8 100644 --- a/ui/src/lib/networkPublish.js +++ b/ui/src/lib/networkPublish.js @@ -8,7 +8,7 @@ const parseMetadata = artifact => { } } -export function groupProviderSkipReason(artifacts) { +export const groupProviderSkipReason = artifacts => { const latest = (artifacts || []) .filter(artifact => artifact.stage === 'publishing_to_network') .reduce((newest, artifact) => (!newest || artifact.id > newest.id ? artifact : newest), null) From 2b44605e713d2ea81a564fcc39bfc85c0746a9ba Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Wed, 14 Oct 2026 11:10:36 +0530 Subject: [PATCH 06/23] feat: load group providers from a config file [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- .env.example | 6 +- ENV.md | 6 +- deploy/network_group_providers.example.json | 3 + ...ish-to-network-under-per-group-provider.md | 2 +- pipeline/network_group.py | 26 ++++++--- pipeline/network_group_providers.json | 1 + tests/test_discovery_publish_service.py | 26 +++++---- tests/test_network_group.py | 57 +++++++++++++++---- 8 files changed, 91 insertions(+), 36 deletions(-) create mode 100644 deploy/network_group_providers.example.json create mode 100644 pipeline/network_group_providers.json diff --git a/.env.example b/.env.example index a08df31..e5e0dd3 100644 --- a/.env.example +++ b/.env.example @@ -260,6 +260,6 @@ KC_SMTP_SSL=false KC_SMTP_USERNAME= KC_SMTP_PASSWORD= -# Group Provider per state, set per deployment (ADR 0006). JSON object, keys case-insensitive. -# Leave empty to publish no Group; malformed JSON stops the API and worker from booting. -NETWORK_GROUP_PROVIDER_IDS={"mh":"mh.provider.oan.dev"} +# Group Provider config file (ADR 0006): JSON of group code -> provider id, set per deployment. +# Unset uses the bundled empty map; a missing or malformed file stops the API and worker from booting. +NETWORK_GROUP_PROVIDERS_CONFIG_PATH=./deploy/network_group_providers.example.json diff --git a/ENV.md b/ENV.md index 734824c..dc30b54 100644 --- a/ENV.md +++ b/ENV.md @@ -249,13 +249,13 @@ Separate from the SQLite/Qdrant catalog above (which tracks PROD vector publicat First piece of the network-discovery push architecture: once a document's PROD promotion succeeds, the `publishing_to_network` pipeline stage POSTs a Beckn-shaped `publish` envelope to an external Discovery Service. It's a PROD-only stage (see `docs/ADR/0004-publishing-to-network-is-prod-only.md`) — skipped entirely when `DISABLE_PROD_SETTING=true`, since a document never reaches the network without first promoting to PROD. This is going to be the only way the experience layer discovers schemas going forward — the Master Scheme Catalog pull API above stays in place for backward compatibility. The catalog announces the *knowledge kind* the document was classified as — one catalog with exactly one `OnDemand` resource for `advisory`, another for `scheme`; any other `document_kind` publishes nothing. `DISCOVERY_SERVICE_ENDPOINT` and `NETWORK_SENDER_ID` are **required** whenever PROD promotion is enabled — there is no separate feature flag for this stage beyond `DISABLE_PROD_SETTING`. -A document filed under a state (its **Group**, taken from the document's `instance`) is announced under that Group's own provider, looked up in `NETWORK_GROUP_PROVIDER_IDS`: catalog and resource ids get the group code as a prefix (`mh-cat-oan-knowledge-provider-advisories`) and both catalog and provider are named ` - Knowledge Advisories|Schemes`. Documents under the portal instance (`PORTAL_INSTANCE`, `bv`) or the default instance (`DEFAULT_INSTANCE`) have no Group and keep the shared provider and unprefixed ids. A Group missing from the map is **skipped with a warning** (the document still completes, and the console shows the warning); a malformed map stops both the API and the worker from booting. See `docs/ADR/0006-publish-to-network-under-per-group-provider.md`. +A document filed under a state (its **Group**, taken from the document's `instance`) is announced under that Group's own provider, looked up in the Group Provider config file (`NETWORK_GROUP_PROVIDERS_CONFIG_PATH`): catalog and resource ids get the group code as a prefix (`mh-cat-oan-knowledge-provider-advisories`) and both catalog and provider are named ` - Knowledge Advisories|Schemes`. Documents under the portal instance (`PORTAL_INSTANCE`, `bv`) or the default instance (`DEFAULT_INSTANCE`) have no Group and keep the shared provider and unprefixed ids. A Group missing from the file is **skipped with a warning** (the document still completes, and the console shows the warning); a missing or malformed file stops both the API and the worker from booting. The file is not mounted by `docker-compose.yml` — mount your own into the `api` and `worker` containers and point the variable at it; `deploy/network_group_providers.example.json` shows the shape. See `docs/ADR/0006-publish-to-network-under-per-group-provider.md`. | Variable | Default | Purpose | |----------|---------|---------| | `DISCOVERY_SERVICE_ENDPOINT` | *(empty — required)* | Base URL of the external Discovery Service; the pipeline POSTs to `{DISCOVERY_SERVICE_ENDPOINT}/publish` | | `NETWORK_SENDER_ID` | *(empty — required)* | This instance's Beckn `senderId`, also sent as `networkId` | -| `NETWORK_GROUP_PROVIDER_IDS` | *(empty — no Group has a provider)* | JSON object of group code → provider id, set per deployment, e.g. `{"mh":"mh.provider.oan.dev"}`. Keys are case-insensitive. Must be valid JSON of non-empty strings or the API and worker refuse to start | +| `NETWORK_GROUP_PROVIDERS_CONFIG_PATH` | unset (bundled `pipeline/network_group_providers.json`, `{}` — no Group has a provider) | Path to a JSON file mapping group code → provider id, set per deployment, e.g. `{"mh": "mh.provider.oan.dev"}`. Keys are case-insensitive. A missing file, invalid JSON or empty values stop the API and worker from starting | | `DISCOVERY_SERVICE_PUBLISH_INITIAL_INTERVAL_SECONDS` | `30` | Wait before the first retry of a failed publish | | `DISCOVERY_SERVICE_PUBLISH_BACKOFF_COEFFICIENT` | `2.0` | Multiplier applied to the wait after each retry | | `DISCOVERY_SERVICE_PUBLISH_MAX_INTERVAL_SECONDS` | `300` | Cap on the wait between retries | @@ -273,7 +273,7 @@ A document filed under a state (its **Group**, taken from the document's `instan | OCR / translation / chunking / domain tags | — | config / status | ✅ runs jobs | | Qdrant (`VECTOR_DB_*`) | — | ✅ | ✅ | | Publish to Network (`DISCOVERY_SERVICE_ENDPOINT`, `NETWORK_SENDER_ID`) | — | — | ✅ runs the publish activity | -| Group Provider map (`NETWORK_GROUP_PROVIDER_IDS`) | — | validated at boot | ✅ validated at boot, used by the publish activity | +| Group Provider config (`NETWORK_GROUP_PROVIDERS_CONFIG_PATH`) | — | validated at boot | ✅ validated at boot, used by the publish activity | --- diff --git a/deploy/network_group_providers.example.json b/deploy/network_group_providers.example.json new file mode 100644 index 0000000..080f07c --- /dev/null +++ b/deploy/network_group_providers.example.json @@ -0,0 +1,3 @@ +{ + "mh": "mh.provider.oan.dev" +} diff --git a/docs/ADR/0006-publish-to-network-under-per-group-provider.md b/docs/ADR/0006-publish-to-network-under-per-group-provider.md index 624515f..5246380 100644 --- a/docs/ADR/0006-publish-to-network-under-per-group-provider.md +++ b/docs/ADR/0006-publish-to-network-under-per-group-provider.md @@ -11,7 +11,7 @@ Decision: - The group code is derived from the document's `instance`, not from the uploader's group, so a Super Admin uploading for a state publishes under that state. - Documents with no Group (Bharat Vistaar `bv`, `default`) keep the Shared Provider and ADR 0003's unprefixed ids, unchanged. - A Group with no provider configured skips the publish with a warning, like an unmapped Knowledge Kind. It is not a failure. A real publish failure stays fatal, per ADR 0004. -- Only the provider id is configured per Group, as one JSON map in `NETWORK_GROUP_PROVIDER_IDS` (keys case-insensitive). The catalog and resource ids are derived, so onboarding a Group is one map entry. A malformed map stops both the API and the worker from booting, so a bad deploy can't quietly skip every Group. +- Only the provider id is configured per Group, in a JSON file whose path is `NETWORK_GROUP_PROVIDERS_CONFIG_PATH` (keys case-insensitive), defaulting to a bundled empty `pipeline/network_group_providers.json` so no provider id is baked into the image. The catalog and resource ids are derived, so onboarding a Group is one entry in that file. A missing or malformed file stops both the API and the worker from booting, so a bad deploy can't quietly skip every Group. - The provider descriptor code is the shared one prefixed with the group code (`mh-oan-knowledge-provider`). - A Group skip is recorded with `reason_code: group_provider_not_configured` and shown as a warning on the document page, but only while it is the document's latest publish attempt. A Knowledge Kind skip shows nothing. - "No Group" follows the configured `PORTAL_INSTANCE` and `DEFAULT_INSTANCE` values rather than hardcoding `bv` and `default`. diff --git a/pipeline/network_group.py b/pipeline/network_group.py index c39f2e4..73386ee 100644 --- a/pipeline/network_group.py +++ b/pipeline/network_group.py @@ -2,11 +2,13 @@ import json import os +from pathlib import Path from typing import Optional from .instances import PORTAL_INSTANCE -GROUP_PROVIDER_IDS_ENV = "NETWORK_GROUP_PROVIDER_IDS" +GROUP_PROVIDERS_CONFIG_PATH_ENV = "NETWORK_GROUP_PROVIDERS_CONFIG_PATH" +DEFAULT_GROUP_PROVIDERS_CONFIG_PATH = Path(__file__).with_name("network_group_providers.json") class GroupProviderConfigError(ValueError): @@ -26,18 +28,18 @@ def group_for_instance(instance: Optional[str]) -> Optional[str]: return code -def parse_group_provider_ids(raw: Optional[str]) -> dict[str, str]: +def parse_group_provider_ids(raw: Optional[str], source: str) -> dict[str, str]: if raw is None or not raw.strip(): return {} try: parsed = json.loads(raw) except json.JSONDecodeError as exc: - raise GroupProviderConfigError(f"{GROUP_PROVIDER_IDS_ENV} is not valid JSON: {exc}") from exc + raise GroupProviderConfigError(f"{source} is not valid JSON: {exc}") from exc if not isinstance(parsed, dict): raise GroupProviderConfigError( - f"{GROUP_PROVIDER_IDS_ENV} must be a JSON object of group code to provider id" + f"{source} must be a JSON object of group code to provider id" ) provider_ids: dict[str, str] = {} @@ -46,16 +48,26 @@ def parse_group_provider_ids(raw: Optional[str]) -> dict[str, str]: provider_id = value.strip() if isinstance(value, str) else "" if not code or not provider_id: raise GroupProviderConfigError( - f"{GROUP_PROVIDER_IDS_ENV} entry {key!r}: {value!r} must map a group code " + f"{source} entry {key!r}: {value!r} must map a group code " "to a non-empty provider id" ) if code in provider_ids: raise GroupProviderConfigError( - f"{GROUP_PROVIDER_IDS_ENV} names group {code!r} more than once" + f"{source} names group {code!r} more than once" ) provider_ids[code] = provider_id return provider_ids +def group_providers_config_path() -> Path: + configured = os.environ.get(GROUP_PROVIDERS_CONFIG_PATH_ENV, "").strip() + return Path(configured) if configured else DEFAULT_GROUP_PROVIDERS_CONFIG_PATH + + def load_group_provider_ids() -> dict[str, str]: - return parse_group_provider_ids(os.environ.get(GROUP_PROVIDER_IDS_ENV)) + path = group_providers_config_path() + try: + raw = path.read_text(encoding="utf-8") + except OSError as exc: + raise GroupProviderConfigError(f"Group provider config {path} cannot be read: {exc}") from exc + return parse_group_provider_ids(raw, str(path)) diff --git a/pipeline/network_group_providers.json b/pipeline/network_group_providers.json new file mode 100644 index 0000000..0967ef4 --- /dev/null +++ b/pipeline/network_group_providers.json @@ -0,0 +1 @@ +{} diff --git a/tests/test_discovery_publish_service.py b/tests/test_discovery_publish_service.py index 95ed359..1d9bfb2 100644 --- a/tests/test_discovery_publish_service.py +++ b/tests/test_discovery_publish_service.py @@ -441,14 +441,20 @@ def test_skipped_publish_logs_no_request(self, monkeypatch, caplog): assert not any("network_publish_url=" in m for m in messages) +def _use_group_providers(monkeypatch, tmp_path, content): + path = tmp_path / "network_group_providers.json" + path.write_text(content) + monkeypatch.setenv("NETWORK_GROUP_PROVIDERS_CONFIG_PATH", str(path)) + + class TestDiscoveryPublishServiceGroupProvider: @pytest.mark.unit - def test_configured_group_publishes_under_its_own_provider(self, monkeypatch): + def test_configured_group_publishes_under_its_own_provider(self, monkeypatch, tmp_path): from pipeline.discovery_publish_service import DiscoveryPublishService monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") - monkeypatch.setenv("NETWORK_GROUP_PROVIDER_IDS", '{"MH": "mh.provider.oan.dev"}') + _use_group_providers(monkeypatch, tmp_path, '{"MH": "mh.provider.oan.dev"}') service = DiscoveryPublishService() client = _client_returning( _on_publish_response("mh-cat-oan-knowledge-provider-advisories", "ACCEPTED") @@ -469,7 +475,7 @@ def test_configured_group_publishes_under_its_own_provider(self, monkeypatch): assert result["result_status"] == "ACCEPTED" @pytest.mark.unit - def test_unconfigured_group_is_skipped_without_an_http_call(self, monkeypatch): + def test_unconfigured_group_is_skipped_without_an_http_call(self, monkeypatch, tmp_path): from pipeline.discovery_publish_service import ( SKIP_GROUP_PROVIDER_NOT_CONFIGURED, DiscoveryPublishService, @@ -477,7 +483,7 @@ def test_unconfigured_group_is_skipped_without_an_http_call(self, monkeypatch): monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") - monkeypatch.setenv("NETWORK_GROUP_PROVIDER_IDS", '{"mh": "mh.provider.oan.dev"}') + _use_group_providers(monkeypatch, tmp_path, '{"mh": "mh.provider.oan.dev"}') service = DiscoveryPublishService() client = MagicMock() @@ -490,7 +496,7 @@ def test_unconfigured_group_is_skipped_without_an_http_call(self, monkeypatch): assert result["envelope"] is None @pytest.mark.unit - def test_unmapped_kind_wins_over_an_unconfigured_group(self, monkeypatch): + def test_unmapped_kind_wins_over_an_unconfigured_group(self, monkeypatch, tmp_path): from pipeline.discovery_publish_service import ( SKIP_UNMAPPED_KIND, DiscoveryPublishService, @@ -498,7 +504,7 @@ def test_unmapped_kind_wins_over_an_unconfigured_group(self, monkeypatch): monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") - monkeypatch.delenv("NETWORK_GROUP_PROVIDER_IDS", raising=False) + monkeypatch.delenv("NETWORK_GROUP_PROVIDERS_CONFIG_PATH", raising=False) service = DiscoveryPublishService() result = service.publish(transaction_id="txn-doc", document_kind="document", group="ka") @@ -507,13 +513,13 @@ def test_unmapped_kind_wins_over_an_unconfigured_group(self, monkeypatch): assert result["skip_reason"] == SKIP_UNMAPPED_KIND @pytest.mark.unit - def test_no_group_publishes_under_the_shared_provider(self, monkeypatch): + def test_no_group_publishes_under_the_shared_provider(self, monkeypatch, tmp_path): from pipeline.discovery_publish_service import DiscoveryPublishService from pipeline.network_constants import CATALOG_PROVIDER monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") - monkeypatch.setenv("NETWORK_GROUP_PROVIDER_IDS", '{"mh": "mh.provider.oan.dev"}') + _use_group_providers(monkeypatch, tmp_path, '{"mh": "mh.provider.oan.dev"}') service = DiscoveryPublishService() client = _client_returning( _on_publish_response("cat-oan-knowledge-provider-advisories", "ACCEPTED") @@ -527,13 +533,13 @@ def test_no_group_publishes_under_the_shared_provider(self, monkeypatch): assert catalog["provider"] == CATALOG_PROVIDER @pytest.mark.unit - def test_malformed_provider_map_fails_construction(self, monkeypatch): + def test_malformed_provider_map_fails_construction(self, monkeypatch, tmp_path): from pipeline.discovery_publish_service import DiscoveryPublishService from pipeline.network_group import GroupProviderConfigError monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") - monkeypatch.setenv("NETWORK_GROUP_PROVIDER_IDS", "{broken") + _use_group_providers(monkeypatch, tmp_path, "{broken") with pytest.raises(GroupProviderConfigError): DiscoveryPublishService() diff --git a/tests/test_network_group.py b/tests/test_network_group.py index 96c56c5..5983240 100644 --- a/tests/test_network_group.py +++ b/tests/test_network_group.py @@ -1,7 +1,10 @@ +from pathlib import Path + import pytest from pipeline.network_group import ( - GROUP_PROVIDER_IDS_ENV, + DEFAULT_GROUP_PROVIDERS_CONFIG_PATH, + GROUP_PROVIDERS_CONFIG_PATH_ENV, GroupProviderConfigError, group_for_instance, load_group_provider_ids, @@ -52,11 +55,13 @@ class TestParseGroupProviderIds: @pytest.mark.unit @pytest.mark.parametrize("raw", [None, "", " "]) def test_unset_means_no_group_has_a_provider(self, raw): - assert parse_group_provider_ids(raw) == {} + assert parse_group_provider_ids(raw, "providers.json") == {} @pytest.mark.unit def test_keys_are_case_insensitive_and_values_trimmed(self): - parsed = parse_group_provider_ids('{"MH": " mh.provider.oan.dev ", "ka": "ka.provider.oan.dev"}') + parsed = parse_group_provider_ids( + '{"MH": " mh.provider.oan.dev ", "ka": "ka.provider.oan.dev"}', "providers.json" + ) assert parsed == {"mh": "mh.provider.oan.dev", "ka": "ka.provider.oan.dev"} @@ -74,29 +79,57 @@ def test_keys_are_case_insensitive_and_values_trimmed(self): ], ) def test_malformed_map_is_rejected(self, raw): - with pytest.raises(GroupProviderConfigError, match=GROUP_PROVIDER_IDS_ENV): - parse_group_provider_ids(raw) + with pytest.raises(GroupProviderConfigError, match="providers.json"): + parse_group_provider_ids(raw, "providers.json") + + +def _providers_file(monkeypatch, tmp_path, content): + path = tmp_path / "network_group_providers.json" + path.write_text(content) + monkeypatch.setenv(GROUP_PROVIDERS_CONFIG_PATH_ENV, str(path)) + return path class TestLoadGroupProviderIds: @pytest.mark.unit - def test_reads_the_environment(self, monkeypatch): - monkeypatch.setenv(GROUP_PROVIDER_IDS_ENV, '{"mh": "mh.provider.oan.dev"}') + def test_reads_the_configured_file(self, monkeypatch, tmp_path): + _providers_file(monkeypatch, tmp_path, '{"mh": "mh.provider.oan.dev"}') assert load_group_provider_ids() == {"mh": "mh.provider.oan.dev"} @pytest.mark.unit - def test_malformed_environment_raises(self, monkeypatch): - monkeypatch.setenv(GROUP_PROVIDER_IDS_ENV, "{broken") + def test_unset_path_uses_the_bundled_empty_default(self, monkeypatch): + monkeypatch.delenv(GROUP_PROVIDERS_CONFIG_PATH_ENV, raising=False) - with pytest.raises(GroupProviderConfigError): + assert DEFAULT_GROUP_PROVIDERS_CONFIG_PATH.is_file() + assert load_group_provider_ids() == {} + + @pytest.mark.unit + def test_missing_file_raises(self, monkeypatch, tmp_path): + missing = tmp_path / "absent.json" + monkeypatch.setenv(GROUP_PROVIDERS_CONFIG_PATH_ENV, str(missing)) + + with pytest.raises(GroupProviderConfigError, match="absent.json"): load_group_provider_ids() + @pytest.mark.unit + def test_malformed_file_raises(self, monkeypatch, tmp_path): + _providers_file(monkeypatch, tmp_path, "{broken") + + with pytest.raises(GroupProviderConfigError, match="network_group_providers.json"): + load_group_provider_ids() + + @pytest.mark.unit + def test_example_file_is_valid(self): + example = Path(__file__).parents[1] / "deploy" / "network_group_providers.example.json" + + assert parse_group_provider_ids(example.read_text(), str(example))["mh"] == "mh.provider.oan.dev" + class TestBootRefusesMalformedProviderMap: @pytest.fixture - def broken_map(self, monkeypatch): - monkeypatch.setenv(GROUP_PROVIDER_IDS_ENV, "{broken") + def broken_map(self, monkeypatch, tmp_path): + _providers_file(monkeypatch, tmp_path, "{broken") monkeypatch.setenv("TRANSLATION_PROVIDER", "none") @pytest.mark.unit From fdbb0a71e9523c6cc80b667e28a4297696496ec4 Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Wed, 14 Oct 2026 17:46:09 +0530 Subject: [PATCH 07/23] refactor: keep group out of chunk payloads [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- ...ish-to-network-under-per-group-provider.md | 2 +- pipeline/activities.py | 3 -- pipeline/vector_store/qdrant_store.py | 2 -- tests/test_activities.py | 18 ---------- tests/test_qdrant_group.py | 35 ------------------- 5 files changed, 1 insertion(+), 59 deletions(-) delete mode 100644 tests/test_qdrant_group.py diff --git a/docs/ADR/0006-publish-to-network-under-per-group-provider.md b/docs/ADR/0006-publish-to-network-under-per-group-provider.md index 5246380..1e8427c 100644 --- a/docs/ADR/0006-publish-to-network-under-per-group-provider.md +++ b/docs/ADR/0006-publish-to-network-under-per-group-provider.md @@ -20,4 +20,4 @@ Decision: Consequences: - Ids are permanent under `MERGE` (ADR 0003). Documents already published under the Shared Provider are not moved, so state knowledge published before this change stays under the Shared Provider. A mistyped Group provider id leaves stranded entries on the network. - Depends on the network adapter layer accepting the per-Group provider, catalog and resource ids. -- Chunks gain a `group` field (absent for Bharat Vistaar and `default`) alongside the unchanged `instance` and `instance_name`. Existing chunks are not backfilled, which #202's group filter must account for. +- Only the Publish to Network payload changes. Chunks and the search indexes gain no `group` field; they keep `instance` and `instance_name`, where Bharat Vistaar and `default` documents carry `bv`/`default` and state documents carry the state code, so #202 can filter on `instance`. diff --git a/pipeline/activities.py b/pipeline/activities.py index cb1bef9..a9ab952 100644 --- a/pipeline/activities.py +++ b/pipeline/activities.py @@ -34,7 +34,6 @@ from .document_repository import DocumentRepository from .instances import instance_display_name from .network_constants import RESULT_REJECTED -from .network_group import group_for_instance from .ocr import ocr_pdf as run_ocr_pdf from .ocr import ocr_pdf_in_segments as run_ocr_pdf_in_segments from .translation import load_translation_config, translate_pages @@ -702,7 +701,6 @@ def _prepare_records( "workflow_id": workflow_id or "", "instance": resolved_instance, "instance_name": instance_name, - "group": group_for_instance(resolved_instance), "type": "scheme" if is_scheme else "document", "source": "docs-pipeline", "filename": external_slug, @@ -823,7 +821,6 @@ def _passage_schema_definition(use_tensor_prefix_field: bool = True) -> dict: {"name": "workflow_id", "type": "text", "features": ["filter"]}, {"name": "instance", "type": "text", "features": ["filter"]}, {"name": "instance_name", "type": "text", "features": ["filter"]}, - {"name": "group", "type": "text", "features": ["filter"]}, {"name": "type", "type": "text", "features": ["filter"]}, {"name": "source", "type": "text", "features": ["filter"]}, {"name": "filename", "type": "text", "features": ["filter"]}, diff --git a/pipeline/vector_store/qdrant_store.py b/pipeline/vector_store/qdrant_store.py index 5cc6d1a..844154c 100644 --- a/pipeline/vector_store/qdrant_store.py +++ b/pipeline/vector_store/qdrant_store.py @@ -23,7 +23,6 @@ "workflow_id", "instance", "instance_name", - "group", "type", "source", "filename", @@ -282,7 +281,6 @@ def today(self) -> str: "workflow_id": qmodels.PayloadSchemaType.KEYWORD, "filename": qmodels.PayloadSchemaType.KEYWORD, "instance": qmodels.PayloadSchemaType.KEYWORD, - "group": qmodels.PayloadSchemaType.KEYWORD, "chunk_num": qmodels.PayloadSchemaType.INTEGER, "is_reference": qmodels.PayloadSchemaType.BOOL, "type": qmodels.PayloadSchemaType.KEYWORD, diff --git a/tests/test_activities.py b/tests/test_activities.py index 6c93948..e383c26 100644 --- a/tests/test_activities.py +++ b/tests/test_activities.py @@ -163,24 +163,6 @@ def test_prepare_records_carries_instance_name(self): assert records[0]["instance"] == code assert records[0]["instance_name"] == expected - @pytest.mark.unit - def test_prepare_records_carries_group_beside_instance(self, monkeypatch): - from pipeline.activities import _prepare_records - - monkeypatch.delenv("PORTAL_INSTANCE", raising=False) - monkeypatch.delenv("DEFAULT_INSTANCE", raising=False) - chunks = [{"chunk_number": 1, "original_text": "Test chunk", "token_count": 5}] - - for code, expected in (("mh", "mh"), ("bv", None), ("default", None)): - record = _prepare_records( - document_id="test-doc", - filename="test.pdf", - chunks=chunks, - instance=code, - )[0] - assert record["instance"] == code - assert record["group"] == expected - @pytest.mark.unit def test_prepare_records_fills_scheme_aliases(self): """A scheme must never index with scheme_aliases: [].""" diff --git a/tests/test_qdrant_group.py b/tests/test_qdrant_group.py deleted file mode 100644 index f6fb166..0000000 --- a/tests/test_qdrant_group.py +++ /dev/null @@ -1,35 +0,0 @@ -import pytest -from qdrant_client.http import models as qmodels - -from pipeline.vector_store.qdrant_store import ( - PAYLOAD_FIELDS, - QdrantVectorStore, - _record_payload, -) - - -class TestGroupPayload: - @pytest.mark.unit - def test_carries_the_group_beside_the_instance(self): - payload = _record_payload( - {"text": "x", "instance": "mh", "instance_name": "Maharashtra", "group": "mh"} - ) - - assert payload["group"] == "mh" - assert payload["instance"] == "mh" - assert payload["instance_name"] == "Maharashtra" - - @pytest.mark.unit - def test_a_chunk_with_no_group_has_no_group_field(self): - payload = _record_payload({"text": "x", "instance": "bv", "group": None}) - - assert "group" not in payload - assert payload["instance"] == "bv" - - @pytest.mark.unit - def test_group_is_a_declared_payload_field(self): - assert "group" in PAYLOAD_FIELDS - - @pytest.mark.unit - def test_group_is_indexed_as_a_keyword_like_instance(self): - assert QdrantVectorStore.PAYLOAD_INDEXES["group"] == qmodels.PayloadSchemaType.KEYWORD From d1a679122e9a0ef0639275f53268462a3dfb3826 Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Thu, 15 Oct 2026 16:47:48 +0530 Subject: [PATCH 08/23] feat: name network providers per kind and instance [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- ENV.md | 10 +- deploy/network_group_providers.example.json | 6 +- ...ish-to-network-under-per-group-provider.md | 13 +- pipeline/api.py | 4 +- pipeline/catalog_builder.py | 60 ++++++--- pipeline/discovery_publish_service.py | 22 ++-- pipeline/network_constants.py | 14 +-- pipeline/network_group.py | 108 +++++++++++----- pipeline/network_group_providers.json | 2 +- pipeline/worker.py | 4 +- tests/test_catalog_builder.py | 75 +++++++++--- tests/test_discovery_publish_service.py | 79 ++++++++++-- tests/test_network_group.py | 115 +++++++++++++----- 13 files changed, 367 insertions(+), 145 deletions(-) diff --git a/ENV.md b/ENV.md index dc30b54..36277aa 100644 --- a/ENV.md +++ b/ENV.md @@ -249,13 +249,17 @@ Separate from the SQLite/Qdrant catalog above (which tracks PROD vector publicat First piece of the network-discovery push architecture: once a document's PROD promotion succeeds, the `publishing_to_network` pipeline stage POSTs a Beckn-shaped `publish` envelope to an external Discovery Service. It's a PROD-only stage (see `docs/ADR/0004-publishing-to-network-is-prod-only.md`) — skipped entirely when `DISABLE_PROD_SETTING=true`, since a document never reaches the network without first promoting to PROD. This is going to be the only way the experience layer discovers schemas going forward — the Master Scheme Catalog pull API above stays in place for backward compatibility. The catalog announces the *knowledge kind* the document was classified as — one catalog with exactly one `OnDemand` resource for `advisory`, another for `scheme`; any other `document_kind` publishes nothing. `DISCOVERY_SERVICE_ENDPOINT` and `NETWORK_SENDER_ID` are **required** whenever PROD promotion is enabled — there is no separate feature flag for this stage beyond `DISABLE_PROD_SETTING`. -A document filed under a state (its **Group**, taken from the document's `instance`) is announced under that Group's own provider, looked up in the Group Provider config file (`NETWORK_GROUP_PROVIDERS_CONFIG_PATH`): catalog and resource ids get the group code as a prefix (`mh-cat-oan-knowledge-provider-advisories`) and both catalog and provider are named ` - Knowledge Advisories|Schemes`. Documents under the portal instance (`PORTAL_INSTANCE`, `bv`) or the default instance (`DEFAULT_INSTANCE`) have no Group and keep the shared provider and unprefixed ids. A Group missing from the file is **skipped with a warning** (the document still completes, and the console shows the warning); a missing or malformed file stops both the API and the worker from booting. The file is not mounted by `docker-compose.yml` — mount your own into the `api` and `worker` containers and point the variable at it; `deploy/network_group_providers.example.json` shows the shape. See `docs/ADR/0006-publish-to-network-under-per-group-provider.md`. +Each catalog's provider is named per Knowledge Kind and instance: id and descriptor code `{kind}.{instance}.provider.oan.dev` (`advisory.bv.provider.oan.dev`, `scheme.mh.provider.oan.dev`), name `Advisories|Schemes from documents`. A document filed under a state (its **Group**, taken from the document's `instance`) uses the group code as the instance, prefixes catalog and resource ids with it (`mh-cat-oan-knowledge-provider-advisories`) and names catalog and provider ` ` (`Maharashtra Advisories from documents`). Documents under the portal instance (`PORTAL_INSTANCE`, `bv`) or the default instance (`DEFAULT_INSTANCE`) have no Group: they use the portal instance in the provider id and keep the unprefixed ids. Only Groups listed in the Group Provider config file (`NETWORK_GROUP_PROVIDERS_CONFIG_PATH`) publish; any other Group is **skipped with a warning** (the document still completes, and the console shows the warning). A missing or malformed file, or a template with an unknown placeholder, stops both the API and the worker from booting. The file is not mounted by `docker-compose.yml` — mount your own into the `api` and `worker` containers and point the variable at it; `deploy/network_group_providers.example.json` shows the shape. See `docs/ADR/0006-publish-to-network-under-per-group-provider.md`. | Variable | Default | Purpose | |----------|---------|---------| | `DISCOVERY_SERVICE_ENDPOINT` | *(empty — required)* | Base URL of the external Discovery Service; the pipeline POSTs to `{DISCOVERY_SERVICE_ENDPOINT}/publish` | | `NETWORK_SENDER_ID` | *(empty — required)* | This instance's Beckn `senderId`, also sent as `networkId` | -| `NETWORK_GROUP_PROVIDERS_CONFIG_PATH` | unset (bundled `pipeline/network_group_providers.json`, `{}` — no Group has a provider) | Path to a JSON file mapping group code → provider id, set per deployment, e.g. `{"mh": "mh.provider.oan.dev"}`. Keys are case-insensitive. A missing file, invalid JSON or empty values stop the API and worker from starting | +| `NETWORK_GROUP_PROVIDERS_CONFIG_PATH` | unset (bundled `pipeline/network_group_providers.json`, `[]` — no Group publishes) | Path to a JSON array of the group codes that publish, set per deployment, e.g. `["mh"]`. Codes are case-insensitive. A missing file, invalid JSON or empty codes stop the API and worker from starting | +| `NETWORK_PROVIDER_ID_TEMPLATE` | `{kind}.{instance}.provider.oan.dev` | Provider id and descriptor code. `{kind}` is `advisory` or `scheme`; `{instance}` is the group code, or `PORTAL_INSTANCE` for documents with no Group | +| `NETWORK_ADVISORY_PROVIDER_NAME` | `Advisories from documents` | Provider name for advisory catalogs | +| `NETWORK_SCHEMES_PROVIDER_NAME` | `Schemes from documents` | Provider name for scheme catalogs | +| `NETWORK_GROUP_PROVIDER_NAME_TEMPLATE` | `{group_name} {provider_name}` | Provider and catalog name for a Group, e.g. `Maharashtra Advisories from documents` | | `DISCOVERY_SERVICE_PUBLISH_INITIAL_INTERVAL_SECONDS` | `30` | Wait before the first retry of a failed publish | | `DISCOVERY_SERVICE_PUBLISH_BACKOFF_COEFFICIENT` | `2.0` | Multiplier applied to the wait after each retry | | `DISCOVERY_SERVICE_PUBLISH_MAX_INTERVAL_SECONDS` | `300` | Cap on the wait between retries | @@ -273,7 +277,7 @@ A document filed under a state (its **Group**, taken from the document's `instan | OCR / translation / chunking / domain tags | — | config / status | ✅ runs jobs | | Qdrant (`VECTOR_DB_*`) | — | ✅ | ✅ | | Publish to Network (`DISCOVERY_SERVICE_ENDPOINT`, `NETWORK_SENDER_ID`) | — | — | ✅ runs the publish activity | -| Group Provider config (`NETWORK_GROUP_PROVIDERS_CONFIG_PATH`) | — | validated at boot | ✅ validated at boot, used by the publish activity | +| Group Provider config (`NETWORK_GROUP_PROVIDERS_CONFIG_PATH`, `NETWORK_*_PROVIDER_*`) | — | validated at boot | ✅ validated at boot, used by the publish activity | --- diff --git a/deploy/network_group_providers.example.json b/deploy/network_group_providers.example.json index 080f07c..4a7b3ea 100644 --- a/deploy/network_group_providers.example.json +++ b/deploy/network_group_providers.example.json @@ -1,3 +1,3 @@ -{ - "mh": "mh.provider.oan.dev" -} +[ + "mh" +] diff --git a/docs/ADR/0006-publish-to-network-under-per-group-provider.md b/docs/ADR/0006-publish-to-network-under-per-group-provider.md index 1e8427c..cc156d5 100644 --- a/docs/ADR/0006-publish-to-network-under-per-group-provider.md +++ b/docs/ADR/0006-publish-to-network-under-per-group-provider.md @@ -7,17 +7,18 @@ status: accepted Context: ADR 0003 announced every document under one provider, with one catalog per Knowledge Kind. A state's experience (for example Maharashtra's, through DSS) needs to consume only its own state's knowledge from the network, and one shared provider gives it no way to do that (engineering-tracker#190). Decision: -- Documents with a Group publish under that Group's provider. Its id is set per deployment, for example `mh.provider.oan.dev`. Catalog and resource ids are today's ids prefixed with the group code (`mh-cat-oan-knowledge-provider-advisories`, `mh-res-oan-knowledge-provider-advisories`, and the same for schemes). Both catalog and provider are named ` - Knowledge Advisories|Schemes`. +- Every catalog has a provider per Knowledge Kind and instance. Its id and descriptor code are `{kind}.{instance}.provider.oan.dev` (`advisory.mh.provider.oan.dev`, `scheme.bv.provider.oan.dev`). It is named `Advisories|Schemes from documents`, prefixed with the Group name for a Group (`Maharashtra Advisories from documents`). +- Documents with a Group use the group code as the instance. Catalog and resource ids are today's ids prefixed with the group code (`mh-cat-oan-knowledge-provider-advisories`, `mh-res-oan-knowledge-provider-advisories`, and the same for schemes), and the catalog takes the provider's name. - The group code is derived from the document's `instance`, not from the uploader's group, so a Super Admin uploading for a state publishes under that state. -- Documents with no Group (Bharat Vistaar `bv`, `default`) keep the Shared Provider and ADR 0003's unprefixed ids, unchanged. -- A Group with no provider configured skips the publish with a warning, like an unmapped Knowledge Kind. It is not a failure. A real publish failure stays fatal, per ADR 0004. -- Only the provider id is configured per Group, in a JSON file whose path is `NETWORK_GROUP_PROVIDERS_CONFIG_PATH` (keys case-insensitive), defaulting to a bundled empty `pipeline/network_group_providers.json` so no provider id is baked into the image. The catalog and resource ids are derived, so onboarding a Group is one entry in that file. A missing or malformed file stops both the API and the worker from booting, so a bad deploy can't quietly skip every Group. -- The provider descriptor code is the shared one prefixed with the group code (`mh-oan-knowledge-provider`). +- Documents with no Group (Bharat Vistaar `bv`, `default`) use the portal instance `bv` in the provider id and keep ADR 0003's unprefixed catalog and resource ids. +- A Group not listed in that file skips the publish with a warning, like an unmapped Knowledge Kind. It is not a failure. A real publish failure stays fatal, per ADR 0004. +- The provider id template, the per-kind provider names and the Group name template are environment settings, so a deployment can change them without a code change. A template with an unknown placeholder stops the API and the worker from booting. +- The Groups that publish are listed in a JSON array whose path is `NETWORK_GROUP_PROVIDERS_CONFIG_PATH` (codes case-insensitive), defaulting to a bundled empty `pipeline/network_group_providers.json`. Every id is derived, so onboarding a Group is one entry in that file. A missing or malformed file stops both the API and the worker from booting, so a bad deploy can't quietly skip every Group. - A Group skip is recorded with `reason_code: group_provider_not_configured` and shown as a warning on the document page, but only while it is the document's latest publish attempt. A Knowledge Kind skip shows nothing. - "No Group" follows the configured `PORTAL_INSTANCE` and `DEFAULT_INSTANCE` values rather than hardcoding `bv` and `default`. - The publish endpoint stays the same for every Group. Consequences: -- Ids are permanent under `MERGE` (ADR 0003). Documents already published under the Shared Provider are not moved, so state knowledge published before this change stays under the Shared Provider. A mistyped Group provider id leaves stranded entries on the network. +- Ids are permanent under `MERGE` (ADR 0003). Documents already published under the old shared provider `knowledge-provider` are not moved, so state knowledge published before this change stays there; the next `bv` publish re-announces the unprefixed catalogs under `advisory.bv`/`scheme.bv`. Changing the id template later strands entries the same way. - Depends on the network adapter layer accepting the per-Group provider, catalog and resource ids. - Only the Publish to Network payload changes. Chunks and the search indexes gain no `group` field; they keep `instance` and `instance_name`, where Bharat Vistaar and `default` documents carry `bv`/`default` and state documents carry the state code, so #202 can filter on `instance`. diff --git a/pipeline/api.py b/pipeline/api.py index 531364f..9506987 100644 --- a/pipeline/api.py +++ b/pipeline/api.py @@ -103,7 +103,7 @@ SearchSettingsUpdate, SettingsAuditResponse, ) -from .network_group import load_group_provider_ids +from .network_group import validate_network_provider_config from .workflows import ( ChunkingOnlyWorkflow, DocumentPipelineWorkflow, @@ -138,7 +138,7 @@ async def lifespan(app: FastAPI): """Initialize Temporal and MinIO clients on startup.""" global temporal_client, minio_client, MINIO_BUCKET - load_group_provider_ids() + validate_network_provider_config() auth_cfg = load_auth_config() validate_auth_config(auth_cfg) diff --git a/pipeline/catalog_builder.py b/pipeline/catalog_builder.py index 6b50f5d..ef1020d 100644 --- a/pipeline/catalog_builder.py +++ b/pipeline/catalog_builder.py @@ -11,24 +11,27 @@ """ import copy -from dataclasses import dataclass -from typing import Optional +from dataclasses import dataclass, field +from typing import Mapping, Optional from .network_constants import ( ADVISORY_CATALOG_ID, ADVISORY_CATALOG_NAME, + ADVISORY_PROVIDER_NAME, ADVISORY_RESOURCE_ID, ADVISORY_RESOURCE_NAME, ADVISORY_SUBJECT_CATEGORIES, ADVISORY_TOPICS, - CATALOG_PROVIDER, DEFAULT_DOCUMENT_KIND, + GROUP_PROVIDER_NAME_TEMPLATE, INFORMATION_MODE, + PROVIDER_ID_TEMPLATE, SCHEMA_CONTEXT_BASE, SCHEME_SUBJECT_CATEGORIES, SCHEME_TOPICS, SCHEMES_CATALOG_ID, SCHEMES_CATALOG_NAME, + SCHEMES_PROVIDER_NAME, SCHEMES_RESOURCE_ID, SCHEMES_RESOURCE_NAME, SERVED_LANGUAGES, @@ -58,7 +61,6 @@ _KNOWLEDGE_KIND_CATALOGS = { "advisory": { - "group_label": "Knowledge Advisories", "catalog_id": ADVISORY_CATALOG_ID, "catalog_name": ADVISORY_CATALOG_NAME, "resource_id": ADVISORY_RESOURCE_ID, @@ -66,7 +68,6 @@ "resource_attributes": _ADVISORY_RESOURCE_ATTRIBUTES, }, "scheme": { - "group_label": "Knowledge Schemes", "catalog_id": SCHEMES_CATALOG_ID, "catalog_name": SCHEMES_CATALOG_NAME, "resource_id": SCHEMES_RESOURCE_ID, @@ -79,10 +80,30 @@ @dataclass(frozen=True) -class GroupProvider: +class Group: code: str name: str - provider_id: str + + +@dataclass(frozen=True) +class ProviderNaming: + shared_instance: str = "bv" + id_template: str = PROVIDER_ID_TEMPLATE + group_name_template: str = GROUP_PROVIDER_NAME_TEMPLATE + provider_names: Mapping[str, str] = field( + default_factory=lambda: { + "advisory": ADVISORY_PROVIDER_NAME, + "scheme": SCHEMES_PROVIDER_NAME, + } + ) + + def provider(self, kind: str, group: Optional[Group]) -> dict: + instance = group.code if group is not None else self.shared_instance + provider_id = self.id_template.format(kind=kind, instance=instance) + name = self.provider_names[kind] + if group is not None: + name = self.group_name_template.format(group_name=group.name, provider_name=name) + return {"id": provider_id, "descriptor": {"code": provider_id, "name": name}} def normalize_document_kind(document_kind: Optional[str]) -> str: @@ -90,9 +111,14 @@ def normalize_document_kind(document_kind: Optional[str]) -> str: return (document_kind or DEFAULT_DOCUMENT_KIND).strip().lower() +def is_publishable_kind(document_kind: Optional[str]) -> bool: + return normalize_document_kind(document_kind) in _KNOWLEDGE_KIND_CATALOGS + + def build_catalog( document_kind: Optional[str], - group_provider: Optional[GroupProvider] = None, + group: Optional[Group] = None, + naming: ProviderNaming = ProviderNaming(), ) -> Optional[dict]: """Return the catalog to announce for `document_kind`, or None. @@ -100,26 +126,20 @@ def build_catalog( operator-entered custom slug. Callers must skip the publish rather than send an empty `catalogs` array, which the spec rejects (`minItems: 1`). """ - spec = _KNOWLEDGE_KIND_CATALOGS.get(normalize_document_kind(document_kind)) + kind = normalize_document_kind(document_kind) + spec = _KNOWLEDGE_KIND_CATALOGS.get(kind) if spec is None: return None catalog_id = spec["catalog_id"] catalog_name = spec["catalog_name"] resource_id = spec["resource_id"] - provider = copy.deepcopy(CATALOG_PROVIDER) - if group_provider is not None: - prefix = f"{group_provider.code}-" + provider = naming.provider(kind, group) + if group is not None: + prefix = f"{group.code}-" catalog_id = prefix + catalog_id resource_id = prefix + resource_id - catalog_name = f"{group_provider.name} - {spec['group_label']}" - provider = { - "id": group_provider.provider_id, - "descriptor": { - "code": prefix + CATALOG_PROVIDER["descriptor"]["code"], - "name": catalog_name, - }, - } + catalog_name = provider["descriptor"]["name"] catalog = { "id": catalog_id, diff --git a/pipeline/discovery_publish_service.py b/pipeline/discovery_publish_service.py index fa3d3da..8337155 100644 --- a/pipeline/discovery_publish_service.py +++ b/pipeline/discovery_publish_service.py @@ -16,10 +16,10 @@ import httpx -from .catalog_builder import GroupProvider, build_catalog +from .catalog_builder import Group, build_catalog, is_publishable_kind from .instances import instance_display_name from .network_constants import BECKN_VERSION, RESULT_ACCEPTED -from .network_group import load_group_provider_ids +from .network_group import load_group_codes, load_provider_naming logger = logging.getLogger(__name__) @@ -46,7 +46,8 @@ def __init__( raise RuntimeError("NETWORK_SENDER_ID must be set to publish to the network.") self.timeout = timeout - self.group_provider_ids = load_group_provider_ids() + self.group_codes = load_group_codes() + self.provider_naming = load_provider_naming() def publish( self, @@ -66,7 +67,7 @@ def publish( with `skipped=True`: the spec declares `message.catalogs` as `minItems: 1`, so there is no valid "publish nothing" request to send. """ - if build_catalog(document_kind) is None: + if not is_publishable_kind(document_kind): logger.info( "workflow_id=%s document_kind=%s network_publish_skipped=True", workflow_id, @@ -74,10 +75,9 @@ def publish( ) return self._skipped(SKIP_UNMAPPED_KIND) - group_provider = None + catalog_group = None if group is not None: - provider_id = self.group_provider_ids.get(group) - if provider_id is None: + if group not in self.group_codes: logger.warning( "workflow_id=%s document_kind=%s group=%s network_publish_skipped=True " "reason=%s", @@ -87,13 +87,9 @@ def publish( SKIP_GROUP_PROVIDER_NOT_CONFIGURED, ) return self._skipped(SKIP_GROUP_PROVIDER_NOT_CONFIGURED) - group_provider = GroupProvider( - code=group, - name=instance_display_name(group), - provider_id=provider_id, - ) + catalog_group = Group(code=group, name=instance_display_name(group)) - catalog = build_catalog(document_kind, group_provider) + catalog = build_catalog(document_kind, catalog_group, self.provider_naming) envelope = { "context": { diff --git a/pipeline/network_constants.py b/pipeline/network_constants.py index 422a946..267575e 100644 --- a/pipeline/network_constants.py +++ b/pipeline/network_constants.py @@ -33,12 +33,8 @@ DEFAULT_DOCUMENT_KIND = "document" -# Same provider on every catalog - this pipeline publishes as one provider -# regardless of knowledge kind. -CATALOG_PROVIDER = { - "id": "knowledge-provider", - "descriptor": { - "code": "oan-knowledge-provider", - "name": "OAN Knowledge Provider", - }, -} +ADVISORY_PROVIDER_NAME = "Advisories from documents" +SCHEMES_PROVIDER_NAME = "Schemes from documents" + +PROVIDER_ID_TEMPLATE = "{kind}.{instance}.provider.oan.dev" +GROUP_PROVIDER_NAME_TEMPLATE = "{group_name} {provider_name}" diff --git a/pipeline/network_group.py b/pipeline/network_group.py index 73386ee..2d389eb 100644 --- a/pipeline/network_group.py +++ b/pipeline/network_group.py @@ -5,20 +5,37 @@ from pathlib import Path from typing import Optional +from .catalog_builder import ProviderNaming from .instances import PORTAL_INSTANCE +from .network_constants import ( + ADVISORY_PROVIDER_NAME, + GROUP_PROVIDER_NAME_TEMPLATE, + PROVIDER_ID_TEMPLATE, + SCHEMES_PROVIDER_NAME, +) GROUP_PROVIDERS_CONFIG_PATH_ENV = "NETWORK_GROUP_PROVIDERS_CONFIG_PATH" DEFAULT_GROUP_PROVIDERS_CONFIG_PATH = Path(__file__).with_name("network_group_providers.json") +PROVIDER_ID_TEMPLATE_ENV = "NETWORK_PROVIDER_ID_TEMPLATE" +GROUP_PROVIDER_NAME_TEMPLATE_ENV = "NETWORK_GROUP_PROVIDER_NAME_TEMPLATE" +ADVISORY_PROVIDER_NAME_ENV = "NETWORK_ADVISORY_PROVIDER_NAME" +SCHEMES_PROVIDER_NAME_ENV = "NETWORK_SCHEMES_PROVIDER_NAME" -class GroupProviderConfigError(ValueError): +class NetworkProviderConfigError(ValueError): pass +def _env(name: str, default: str) -> str: + return os.environ.get(name, "").strip() or default + + +def portal_instance() -> str: + return _env("PORTAL_INSTANCE", PORTAL_INSTANCE).lower() + + def _no_group_instances() -> set[str]: - portal = (os.environ.get("PORTAL_INSTANCE") or PORTAL_INSTANCE).strip().lower() - default = (os.environ.get("DEFAULT_INSTANCE") or "default").strip().lower() - return {portal or PORTAL_INSTANCE, default or "default"} + return {portal_instance(), _env("DEFAULT_INSTANCE", "default").lower()} def group_for_instance(instance: Optional[str]) -> Optional[str]: @@ -28,35 +45,27 @@ def group_for_instance(instance: Optional[str]) -> Optional[str]: return code -def parse_group_provider_ids(raw: Optional[str], source: str) -> dict[str, str]: +def parse_group_codes(raw: Optional[str], source: str) -> frozenset[str]: if raw is None or not raw.strip(): - return {} + return frozenset() try: parsed = json.loads(raw) except json.JSONDecodeError as exc: - raise GroupProviderConfigError(f"{source} is not valid JSON: {exc}") from exc - - if not isinstance(parsed, dict): - raise GroupProviderConfigError( - f"{source} must be a JSON object of group code to provider id" - ) - - provider_ids: dict[str, str] = {} - for key, value in parsed.items(): - code = key.strip().lower() if isinstance(key, str) else "" - provider_id = value.strip() if isinstance(value, str) else "" - if not code or not provider_id: - raise GroupProviderConfigError( - f"{source} entry {key!r}: {value!r} must map a group code " - "to a non-empty provider id" - ) - if code in provider_ids: - raise GroupProviderConfigError( - f"{source} names group {code!r} more than once" - ) - provider_ids[code] = provider_id - return provider_ids + raise NetworkProviderConfigError(f"{source} is not valid JSON: {exc}") from exc + + if not isinstance(parsed, list): + raise NetworkProviderConfigError(f"{source} must be a JSON array of group codes") + + codes: set[str] = set() + for entry in parsed: + code = entry.strip().lower() if isinstance(entry, str) else "" + if not code: + raise NetworkProviderConfigError(f"{source} entry {entry!r} must be a non-empty group code") + if code in codes: + raise NetworkProviderConfigError(f"{source} names group {code!r} more than once") + codes.add(code) + return frozenset(codes) def group_providers_config_path() -> Path: @@ -64,10 +73,47 @@ def group_providers_config_path() -> Path: return Path(configured) if configured else DEFAULT_GROUP_PROVIDERS_CONFIG_PATH -def load_group_provider_ids() -> dict[str, str]: +def load_group_codes() -> frozenset[str]: path = group_providers_config_path() try: raw = path.read_text(encoding="utf-8") except OSError as exc: - raise GroupProviderConfigError(f"Group provider config {path} cannot be read: {exc}") from exc - return parse_group_provider_ids(raw, str(path)) + raise NetworkProviderConfigError(f"Group provider config {path} cannot be read: {exc}") from exc + return parse_group_codes(raw, str(path)) + + +def _checked_template(env_name: str, default: str, **placeholders: str) -> str: + template = _env(env_name, default) + try: + rendered = template.format(**placeholders) + except (KeyError, IndexError, ValueError) as exc: + raise NetworkProviderConfigError( + f"{env_name}={template!r} may only use {sorted(placeholders)}: {exc}" + ) from exc + if not rendered.strip(): + raise NetworkProviderConfigError(f"{env_name} renders an empty value") + return template + + +def load_provider_naming() -> ProviderNaming: + return ProviderNaming( + shared_instance=portal_instance(), + id_template=_checked_template( + PROVIDER_ID_TEMPLATE_ENV, PROVIDER_ID_TEMPLATE, kind="advisory", instance="bv" + ), + group_name_template=_checked_template( + GROUP_PROVIDER_NAME_TEMPLATE_ENV, + GROUP_PROVIDER_NAME_TEMPLATE, + group_name="Maharashtra", + provider_name="Advisories from documents", + ), + provider_names={ + "advisory": _env(ADVISORY_PROVIDER_NAME_ENV, ADVISORY_PROVIDER_NAME), + "scheme": _env(SCHEMES_PROVIDER_NAME_ENV, SCHEMES_PROVIDER_NAME), + }, + ) + + +def validate_network_provider_config() -> None: + load_group_codes() + load_provider_naming() diff --git a/pipeline/network_group_providers.json b/pipeline/network_group_providers.json index 0967ef4..fe51488 100644 --- a/pipeline/network_group_providers.json +++ b/pipeline/network_group_providers.json @@ -1 +1 @@ -{} +[] diff --git a/pipeline/worker.py b/pipeline/worker.py index 0c7f2d7..6cd996b 100644 --- a/pipeline/worker.py +++ b/pipeline/worker.py @@ -26,7 +26,7 @@ run_ocr_and_store, update_document_state, ) -from .network_group import load_group_provider_ids +from .network_group import validate_network_provider_config from .workflows import ( ChunkingOnlyWorkflow, DocumentPipelineWorkflow, @@ -54,7 +54,7 @@ async def main(): """Start the worker.""" - load_group_provider_ids() + validate_network_provider_config() translation_provider = os.environ.get("TRANSLATION_PROVIDER", "gemma_vllm").strip().lower() if translation_provider in {"gemma_vllm", "gemma4", "gemma"}: diff --git a/tests/test_catalog_builder.py b/tests/test_catalog_builder.py index be9df39..533afed 100644 --- a/tests/test_catalog_builder.py +++ b/tests/test_catalog_builder.py @@ -3,18 +3,28 @@ import pytest from pipeline.catalog_builder import ( - GroupProvider, + Group, + ProviderNaming, build_catalog, + is_publishable_kind, normalize_document_kind, ) from pipeline.network_constants import ( ADVISORY_CATALOG_ID, ADVISORY_RESOURCE_ID, - CATALOG_PROVIDER, SCHEMES_CATALOG_ID, SCHEMES_RESOURCE_ID, ) +BV_ADVISORY_PROVIDER = { + "id": "advisory.bv.provider.oan.dev", + "descriptor": {"code": "advisory.bv.provider.oan.dev", "name": "Advisories from documents"}, +} +BV_SCHEMES_PROVIDER = { + "id": "scheme.bv.provider.oan.dev", + "descriptor": {"code": "scheme.bv.provider.oan.dev", "name": "Schemes from documents"}, +} + # Forbidden by the KnowledgeAdvisory schema under informationMode OnDemand. # Both advisory and scheme resources use this schema. FORBIDDEN_ADVISORY_ATTRIBUTES = ( @@ -57,7 +67,7 @@ def test_catalog_envelope_fields(self): assert "bppUri" not in catalog assert catalog["descriptor"] == {"name": "Agricultural advisory from documents"} assert catalog["isActive"] is True - assert catalog["provider"] == CATALOG_PROVIDER + assert catalog["provider"] == BV_ADVISORY_PROVIDER @pytest.mark.unit def test_exactly_one_resource_with_a_stable_id(self): @@ -95,7 +105,7 @@ def test_catalog_envelope_fields(self): assert "bppUri" not in catalog assert catalog["descriptor"] == {"name": "Schemes from documents"} assert catalog["isActive"] is True - assert catalog["provider"] == CATALOG_PROVIDER + assert catalog["provider"] == BV_SCHEMES_PROVIDER @pytest.mark.unit def test_exactly_one_resource_with_a_stable_id(self): @@ -130,6 +140,12 @@ class TestUnmappedKinds: @pytest.mark.parametrize("kind", ["document", "video", "how_to_faq", "", None]) def test_returns_none_so_the_caller_skips_publishing(self, kind): assert build_catalog(kind) is None + assert is_publishable_kind(kind) is False + + @pytest.mark.unit + @pytest.mark.parametrize("kind", ["advisory", " Scheme "]) + def test_knowledge_kinds_are_publishable(self, kind): + assert is_publishable_kind(kind) is True class TestBuilderIsolation: @@ -149,40 +165,42 @@ def test_mutating_a_built_catalogs_provider_does_not_leak_into_the_next(self): second = build_catalog("advisory") - assert second["provider"] == CATALOG_PROVIDER + assert second["provider"] == BV_ADVISORY_PROVIDER -MAHARASHTRA = GroupProvider(code="mh", name="Maharashtra", provider_id="mh.provider.oan.dev") +MAHARASHTRA = Group(code="mh", name="Maharashtra") class TestGroupProviderCatalog: @pytest.mark.unit @pytest.mark.parametrize( - "kind,catalog_id,resource_id,name", + "kind,catalog_id,resource_id,provider_id,name", [ ( "advisory", "mh-cat-oan-knowledge-provider-advisories", "mh-res-oan-knowledge-provider-advisories", - "Maharashtra - Knowledge Advisories", + "advisory.mh.provider.oan.dev", + "Maharashtra Advisories from documents", ), ( "scheme", "mh-cat-oan-knowledge-provider-schemes", "mh-res-oan-knowledge-provider-schemes", - "Maharashtra - Knowledge Schemes", + "scheme.mh.provider.oan.dev", + "Maharashtra Schemes from documents", ), ], ) - def test_ids_and_names_carry_the_group(self, kind, catalog_id, resource_id, name): + def test_ids_and_names_carry_the_group(self, kind, catalog_id, resource_id, provider_id, name): catalog = build_catalog(kind, MAHARASHTRA) assert catalog["id"] == catalog_id assert catalog["descriptor"] == {"name": name} assert catalog["resources"][0]["id"] == resource_id assert catalog["provider"] == { - "id": "mh.provider.oan.dev", - "descriptor": {"code": "mh-oan-knowledge-provider", "name": name}, + "id": provider_id, + "descriptor": {"code": provider_id, "name": name}, } @pytest.mark.unit @@ -199,8 +217,37 @@ def test_unmapped_kind_still_publishes_nothing(self): assert build_catalog("document", MAHARASHTRA) is None @pytest.mark.unit - def test_no_group_keeps_the_shared_provider(self): + def test_no_group_uses_the_shared_instance(self): catalog = build_catalog("advisory", None) assert catalog["id"] == ADVISORY_CATALOG_ID - assert catalog["provider"] == CATALOG_PROVIDER + assert catalog["provider"] == BV_ADVISORY_PROVIDER + + +class TestProviderNaming: + NAMING = ProviderNaming( + shared_instance="portal", + id_template="{instance}-{kind}.example.net", + group_name_template="{provider_name} ({group_name})", + provider_names={"advisory": "Advice", "scheme": "Schemes"}, + ) + + @pytest.mark.unit + def test_shared_provider_follows_the_configured_naming(self): + catalog = build_catalog("scheme", None, self.NAMING) + + assert catalog["id"] == SCHEMES_CATALOG_ID + assert catalog["provider"] == { + "id": "portal-scheme.example.net", + "descriptor": {"code": "portal-scheme.example.net", "name": "Schemes"}, + } + + @pytest.mark.unit + def test_group_provider_follows_the_configured_naming(self): + catalog = build_catalog("advisory", MAHARASHTRA, self.NAMING) + + assert catalog["descriptor"] == {"name": "Advice (Maharashtra)"} + assert catalog["provider"] == { + "id": "mh-advisory.example.net", + "descriptor": {"code": "mh-advisory.example.net", "name": "Advice (Maharashtra)"}, + } diff --git a/tests/test_discovery_publish_service.py b/tests/test_discovery_publish_service.py index 1d9bfb2..1543bd7 100644 --- a/tests/test_discovery_publish_service.py +++ b/tests/test_discovery_publish_service.py @@ -447,6 +447,18 @@ def _use_group_providers(monkeypatch, tmp_path, content): monkeypatch.setenv("NETWORK_GROUP_PROVIDERS_CONFIG_PATH", str(path)) +@pytest.fixture(autouse=True) +def _default_provider_naming(monkeypatch): + for name in ( + "PORTAL_INSTANCE", + "NETWORK_PROVIDER_ID_TEMPLATE", + "NETWORK_GROUP_PROVIDER_NAME_TEMPLATE", + "NETWORK_ADVISORY_PROVIDER_NAME", + "NETWORK_SCHEMES_PROVIDER_NAME", + ): + monkeypatch.delenv(name, raising=False) + + class TestDiscoveryPublishServiceGroupProvider: @pytest.mark.unit def test_configured_group_publishes_under_its_own_provider(self, monkeypatch, tmp_path): @@ -454,7 +466,7 @@ def test_configured_group_publishes_under_its_own_provider(self, monkeypatch, tm monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") - _use_group_providers(monkeypatch, tmp_path, '{"MH": "mh.provider.oan.dev"}') + _use_group_providers(monkeypatch, tmp_path, '["MH"]') service = DiscoveryPublishService() client = _client_returning( _on_publish_response("mh-cat-oan-knowledge-provider-advisories", "ACCEPTED") @@ -467,8 +479,13 @@ def test_configured_group_publishes_under_its_own_provider(self, monkeypatch, tm catalog = envelope["message"]["catalogs"][0] assert catalog["id"] == "mh-cat-oan-knowledge-provider-advisories" assert catalog["resources"][0]["id"] == "mh-res-oan-knowledge-provider-advisories" - assert catalog["provider"]["id"] == "mh.provider.oan.dev" - assert catalog["provider"]["descriptor"]["name"] == "Maharashtra - Knowledge Advisories" + assert catalog["provider"] == { + "id": "advisory.mh.provider.oan.dev", + "descriptor": { + "code": "advisory.mh.provider.oan.dev", + "name": "Maharashtra Advisories from documents", + }, + } assert envelope["message"]["publishDirectives"][0]["catalogId"] == catalog["id"] assert client.post.call_args.args[0] == "https://discovery.example.com/publish" assert result["skipped"] is False @@ -483,7 +500,7 @@ def test_unconfigured_group_is_skipped_without_an_http_call(self, monkeypatch, t monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") - _use_group_providers(monkeypatch, tmp_path, '{"mh": "mh.provider.oan.dev"}') + _use_group_providers(monkeypatch, tmp_path, '["mh"]') service = DiscoveryPublishService() client = MagicMock() @@ -515,11 +532,10 @@ def test_unmapped_kind_wins_over_an_unconfigured_group(self, monkeypatch, tmp_pa @pytest.mark.unit def test_no_group_publishes_under_the_shared_provider(self, monkeypatch, tmp_path): from pipeline.discovery_publish_service import DiscoveryPublishService - from pipeline.network_constants import CATALOG_PROVIDER monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") - _use_group_providers(monkeypatch, tmp_path, '{"mh": "mh.provider.oan.dev"}') + _use_group_providers(monkeypatch, tmp_path, '["mh"]') service = DiscoveryPublishService() client = _client_returning( _on_publish_response("cat-oan-knowledge-provider-advisories", "ACCEPTED") @@ -530,16 +546,61 @@ def test_no_group_publishes_under_the_shared_provider(self, monkeypatch, tmp_pat catalog = client.post.call_args.kwargs["json"]["message"]["catalogs"][0] assert catalog["id"] == "cat-oan-knowledge-provider-advisories" - assert catalog["provider"] == CATALOG_PROVIDER + assert catalog["provider"] == { + "id": "advisory.bv.provider.oan.dev", + "descriptor": { + "code": "advisory.bv.provider.oan.dev", + "name": "Advisories from documents", + }, + } + + @pytest.mark.unit + def test_provider_naming_is_read_from_the_environment(self, monkeypatch, tmp_path): + from pipeline.discovery_publish_service import DiscoveryPublishService + + monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") + monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") + monkeypatch.setenv("NETWORK_PROVIDER_ID_TEMPLATE", "{kind}.{instance}.provider.oan.prod") + monkeypatch.setenv("NETWORK_GROUP_PROVIDER_NAME_TEMPLATE", "{group_name} - {provider_name}") + monkeypatch.setenv("NETWORK_SCHEMES_PROVIDER_NAME", "Government schemes") + _use_group_providers(monkeypatch, tmp_path, '["mh"]') + service = DiscoveryPublishService() + client = _client_returning( + _on_publish_response("mh-cat-oan-knowledge-provider-schemes", "ACCEPTED") + ) + + with patch("pipeline.discovery_publish_service.httpx.Client", return_value=client): + service.publish(transaction_id="txn-mh", document_kind="scheme", group="mh") + + provider = client.post.call_args.kwargs["json"]["message"]["catalogs"][0]["provider"] + assert provider == { + "id": "scheme.mh.provider.oan.prod", + "descriptor": { + "code": "scheme.mh.provider.oan.prod", + "name": "Maharashtra - Government schemes", + }, + } @pytest.mark.unit def test_malformed_provider_map_fails_construction(self, monkeypatch, tmp_path): from pipeline.discovery_publish_service import DiscoveryPublishService - from pipeline.network_group import GroupProviderConfigError + from pipeline.network_group import NetworkProviderConfigError monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") _use_group_providers(monkeypatch, tmp_path, "{broken") - with pytest.raises(GroupProviderConfigError): + with pytest.raises(NetworkProviderConfigError): + DiscoveryPublishService() + + @pytest.mark.unit + def test_bad_provider_id_template_fails_construction(self, monkeypatch): + from pipeline.discovery_publish_service import DiscoveryPublishService + from pipeline.network_group import NetworkProviderConfigError + + monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") + monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") + monkeypatch.setenv("NETWORK_PROVIDER_ID_TEMPLATE", "{state}.provider.oan.dev") + + with pytest.raises(NetworkProviderConfigError, match="NETWORK_PROVIDER_ID_TEMPLATE"): DiscoveryPublishService() diff --git a/tests/test_network_group.py b/tests/test_network_group.py index 5983240..664a5d7 100644 --- a/tests/test_network_group.py +++ b/tests/test_network_group.py @@ -5,10 +5,11 @@ from pipeline.network_group import ( DEFAULT_GROUP_PROVIDERS_CONFIG_PATH, GROUP_PROVIDERS_CONFIG_PATH_ENV, - GroupProviderConfigError, + NetworkProviderConfigError, group_for_instance, - load_group_provider_ids, - parse_group_provider_ids, + load_group_codes, + load_provider_naming, + parse_group_codes, ) @@ -51,36 +52,31 @@ def test_follows_a_configured_default_instance(self, monkeypatch): assert group_for_instance("mh") == "mh" -class TestParseGroupProviderIds: +class TestParseGroupCodes: @pytest.mark.unit @pytest.mark.parametrize("raw", [None, "", " "]) - def test_unset_means_no_group_has_a_provider(self, raw): - assert parse_group_provider_ids(raw, "providers.json") == {} + def test_unset_means_no_group_is_onboarded(self, raw): + assert parse_group_codes(raw, "providers.json") == frozenset() @pytest.mark.unit - def test_keys_are_case_insensitive_and_values_trimmed(self): - parsed = parse_group_provider_ids( - '{"MH": " mh.provider.oan.dev ", "ka": "ka.provider.oan.dev"}', "providers.json" - ) - - assert parsed == {"mh": "mh.provider.oan.dev", "ka": "ka.provider.oan.dev"} + def test_codes_are_case_insensitive_and_trimmed(self): + assert parse_group_codes('[" MH ", "ka"]', "providers.json") == {"mh", "ka"} @pytest.mark.unit @pytest.mark.parametrize( "raw", [ "not json", - '["mh.provider.oan.dev"]', - '"mh.provider.oan.dev"', - '{"mh": 42}', - '{"mh": ""}', - '{"": "mh.provider.oan.dev"}', - '{"MH": "a", "mh": "b"}', + '{"mh": "mh.provider.oan.dev"}', + '"mh"', + "[42]", + '[""]', + '["MH", "mh"]', ], ) - def test_malformed_map_is_rejected(self, raw): - with pytest.raises(GroupProviderConfigError, match="providers.json"): - parse_group_provider_ids(raw, "providers.json") + def test_malformed_list_is_rejected(self, raw): + with pytest.raises(NetworkProviderConfigError, match="providers.json"): + parse_group_codes(raw, "providers.json") def _providers_file(monkeypatch, tmp_path, content): @@ -90,40 +86,95 @@ def _providers_file(monkeypatch, tmp_path, content): return path -class TestLoadGroupProviderIds: +class TestLoadGroupCodes: @pytest.mark.unit def test_reads_the_configured_file(self, monkeypatch, tmp_path): - _providers_file(monkeypatch, tmp_path, '{"mh": "mh.provider.oan.dev"}') + _providers_file(monkeypatch, tmp_path, '["mh"]') - assert load_group_provider_ids() == {"mh": "mh.provider.oan.dev"} + assert load_group_codes() == {"mh"} @pytest.mark.unit def test_unset_path_uses_the_bundled_empty_default(self, monkeypatch): monkeypatch.delenv(GROUP_PROVIDERS_CONFIG_PATH_ENV, raising=False) assert DEFAULT_GROUP_PROVIDERS_CONFIG_PATH.is_file() - assert load_group_provider_ids() == {} + assert load_group_codes() == frozenset() @pytest.mark.unit def test_missing_file_raises(self, monkeypatch, tmp_path): missing = tmp_path / "absent.json" monkeypatch.setenv(GROUP_PROVIDERS_CONFIG_PATH_ENV, str(missing)) - with pytest.raises(GroupProviderConfigError, match="absent.json"): - load_group_provider_ids() + with pytest.raises(NetworkProviderConfigError, match="absent.json"): + load_group_codes() @pytest.mark.unit def test_malformed_file_raises(self, monkeypatch, tmp_path): _providers_file(monkeypatch, tmp_path, "{broken") - with pytest.raises(GroupProviderConfigError, match="network_group_providers.json"): - load_group_provider_ids() + with pytest.raises(NetworkProviderConfigError, match="network_group_providers.json"): + load_group_codes() @pytest.mark.unit def test_example_file_is_valid(self): example = Path(__file__).parents[1] / "deploy" / "network_group_providers.example.json" - assert parse_group_provider_ids(example.read_text(), str(example))["mh"] == "mh.provider.oan.dev" + assert "mh" in parse_group_codes(example.read_text(), str(example)) + + +class TestLoadProviderNaming: + @pytest.fixture(autouse=True) + def _clear_naming_env(self, monkeypatch): + for name in ( + "NETWORK_PROVIDER_ID_TEMPLATE", + "NETWORK_GROUP_PROVIDER_NAME_TEMPLATE", + "NETWORK_ADVISORY_PROVIDER_NAME", + "NETWORK_SCHEMES_PROVIDER_NAME", + ): + monkeypatch.delenv(name, raising=False) + + @pytest.mark.unit + def test_defaults(self): + naming = load_provider_naming() + + assert naming.shared_instance == "bv" + assert naming.id_template == "{kind}.{instance}.provider.oan.dev" + assert naming.group_name_template == "{group_name} {provider_name}" + assert naming.provider_names == { + "advisory": "Advisories from documents", + "scheme": "Schemes from documents", + } + + @pytest.mark.unit + def test_reads_overrides(self, monkeypatch): + monkeypatch.setenv("PORTAL_INSTANCE", "Portal") + monkeypatch.setenv("NETWORK_PROVIDER_ID_TEMPLATE", "{instance}.{kind}.example.net") + monkeypatch.setenv("NETWORK_GROUP_PROVIDER_NAME_TEMPLATE", "{provider_name} - {group_name}") + monkeypatch.setenv("NETWORK_ADVISORY_PROVIDER_NAME", "Advice") + monkeypatch.setenv("NETWORK_SCHEMES_PROVIDER_NAME", "Schemes") + + naming = load_provider_naming() + + assert naming.shared_instance == "portal" + assert naming.id_template == "{instance}.{kind}.example.net" + assert naming.group_name_template == "{provider_name} - {group_name}" + assert naming.provider_names == {"advisory": "Advice", "scheme": "Schemes"} + + @pytest.mark.unit + @pytest.mark.parametrize( + "env_name,template", + [ + ("NETWORK_PROVIDER_ID_TEMPLATE", "{state}.provider.oan.dev"), + ("NETWORK_PROVIDER_ID_TEMPLATE", "{0}.provider.oan.dev"), + ("NETWORK_PROVIDER_ID_TEMPLATE", "{kind.provider.oan.dev"), + ("NETWORK_GROUP_PROVIDER_NAME_TEMPLATE", "{group} {provider_name}"), + ], + ) + def test_unknown_placeholders_are_rejected(self, monkeypatch, env_name, template): + monkeypatch.setenv(env_name, template) + + with pytest.raises(NetworkProviderConfigError, match=env_name): + load_provider_naming() class TestBootRefusesMalformedProviderMap: @@ -142,7 +193,7 @@ async def never_connect(*args, **kwargs): monkeypatch.setattr(worker.Client, "connect", never_connect) - with pytest.raises(GroupProviderConfigError): + with pytest.raises(NetworkProviderConfigError): await worker.main() @pytest.mark.unit @@ -155,6 +206,6 @@ async def never_connect(*args, **kwargs): monkeypatch.setattr(api.Client, "connect", never_connect) - with pytest.raises(GroupProviderConfigError): + with pytest.raises(NetworkProviderConfigError): async with api.lifespan(api.app): pass From 8a53865e2b50cabb04380490675d24e041a38711 Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Thu, 15 Oct 2026 17:16:39 +0530 Subject: [PATCH 09/23] feat: override provider ids per instance from a config file [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- ENV.md | 8 +-- deploy/network_group_providers.example.json | 9 ++- ...ish-to-network-under-per-group-provider.md | 7 +- pipeline/activities.py | 19 +----- pipeline/catalog_builder.py | 5 +- pipeline/discovery_publish_service.py | 22 +----- pipeline/network_group.py | 53 ++++++++++----- pipeline/network_group_providers.json | 2 +- tests/test_activities.py | 43 ++---------- tests/test_catalog_builder.py | 33 +++++++++ tests/test_discovery_publish_service.py | 57 ++++++++++------ tests/test_network_group.py | 67 +++++++++++++------ ui/src/lib/networkPublish.js | 20 ------ ui/src/lib/networkPublish.test.js | 46 ------------- ui/src/views/DocumentOpsView.jsx | 6 -- 15 files changed, 178 insertions(+), 219 deletions(-) delete mode 100644 ui/src/lib/networkPublish.js delete mode 100644 ui/src/lib/networkPublish.test.js diff --git a/ENV.md b/ENV.md index 36277aa..c7726bb 100644 --- a/ENV.md +++ b/ENV.md @@ -249,14 +249,14 @@ Separate from the SQLite/Qdrant catalog above (which tracks PROD vector publicat First piece of the network-discovery push architecture: once a document's PROD promotion succeeds, the `publishing_to_network` pipeline stage POSTs a Beckn-shaped `publish` envelope to an external Discovery Service. It's a PROD-only stage (see `docs/ADR/0004-publishing-to-network-is-prod-only.md`) — skipped entirely when `DISABLE_PROD_SETTING=true`, since a document never reaches the network without first promoting to PROD. This is going to be the only way the experience layer discovers schemas going forward — the Master Scheme Catalog pull API above stays in place for backward compatibility. The catalog announces the *knowledge kind* the document was classified as — one catalog with exactly one `OnDemand` resource for `advisory`, another for `scheme`; any other `document_kind` publishes nothing. `DISCOVERY_SERVICE_ENDPOINT` and `NETWORK_SENDER_ID` are **required** whenever PROD promotion is enabled — there is no separate feature flag for this stage beyond `DISABLE_PROD_SETTING`. -Each catalog's provider is named per Knowledge Kind and instance: id and descriptor code `{kind}.{instance}.provider.oan.dev` (`advisory.bv.provider.oan.dev`, `scheme.mh.provider.oan.dev`), name `Advisories|Schemes from documents`. A document filed under a state (its **Group**, taken from the document's `instance`) uses the group code as the instance, prefixes catalog and resource ids with it (`mh-cat-oan-knowledge-provider-advisories`) and names catalog and provider ` ` (`Maharashtra Advisories from documents`). Documents under the portal instance (`PORTAL_INSTANCE`, `bv`) or the default instance (`DEFAULT_INSTANCE`) have no Group: they use the portal instance in the provider id and keep the unprefixed ids. Only Groups listed in the Group Provider config file (`NETWORK_GROUP_PROVIDERS_CONFIG_PATH`) publish; any other Group is **skipped with a warning** (the document still completes, and the console shows the warning). A missing or malformed file, or a template with an unknown placeholder, stops both the API and the worker from booting. The file is not mounted by `docker-compose.yml` — mount your own into the `api` and `worker` containers and point the variable at it; `deploy/network_group_providers.example.json` shows the shape. See `docs/ADR/0006-publish-to-network-under-per-group-provider.md`. +Each catalog's provider is named per Knowledge Kind and instance: id and descriptor code `{kind}.{instance}.provider.oan.dev` (`advisory.bv.provider.oan.dev`, `scheme.mh.provider.oan.dev`), name `Advisories|Schemes from documents`. A document filed under a state (its **Group**, taken from the document's `instance`) uses the group code as the instance, prefixes catalog and resource ids with it (`mh-cat-oan-knowledge-provider-advisories`) and names catalog and provider ` ` (`Maharashtra Advisories from documents`). Documents under the portal instance (`PORTAL_INSTANCE`, `bv`) or the default instance (`DEFAULT_INSTANCE`) have no Group: they use the portal instance in the provider id and keep the unprefixed ids. Every Group publishes. An instance can replace the default provider id per Knowledge Kind in the provider id overrides file (`NETWORK_GROUP_PROVIDERS_CONFIG_PATH`); a kind it leaves out keeps the default. A missing or malformed file, or a template with an unknown placeholder, stops both the API and the worker from booting. The file is not mounted by `docker-compose.yml` — mount your own into the `api` and `worker` containers and point the variable at it; `deploy/network_group_providers.example.json` shows the shape. See `docs/ADR/0006-publish-to-network-under-per-group-provider.md`. | Variable | Default | Purpose | |----------|---------|---------| | `DISCOVERY_SERVICE_ENDPOINT` | *(empty — required)* | Base URL of the external Discovery Service; the pipeline POSTs to `{DISCOVERY_SERVICE_ENDPOINT}/publish` | | `NETWORK_SENDER_ID` | *(empty — required)* | This instance's Beckn `senderId`, also sent as `networkId` | -| `NETWORK_GROUP_PROVIDERS_CONFIG_PATH` | unset (bundled `pipeline/network_group_providers.json`, `[]` — no Group publishes) | Path to a JSON array of the group codes that publish, set per deployment, e.g. `["mh"]`. Codes are case-insensitive. A missing file, invalid JSON or empty codes stop the API and worker from starting | -| `NETWORK_PROVIDER_ID_TEMPLATE` | `{kind}.{instance}.provider.oan.dev` | Provider id and descriptor code. `{kind}` is `advisory` or `scheme`; `{instance}` is the group code, or `PORTAL_INSTANCE` for documents with no Group | +| `NETWORK_GROUP_PROVIDERS_CONFIG_PATH` | unset (bundled `pipeline/network_group_providers.json`, `{}` — no overrides) | Path to a JSON object of instance code → `{kind: provider id}`, set per deployment, e.g. `{"ka": {"advisory": "advisory.ka.com", "scheme": "scheme.ka.com"}}`. Codes and kinds are case-insensitive; kinds are `advisory` or `scheme`. A missing file, invalid JSON, an unknown kind or an empty id stop the API and worker from starting | +| `NETWORK_PROVIDER_ID_TEMPLATE` | `{kind}.{instance}.provider.oan.dev` | Default provider id and descriptor code, unless overridden in the file above. `{kind}` is `advisory` or `scheme`; `{instance}` is the group code, or `PORTAL_INSTANCE` for documents with no Group | | `NETWORK_ADVISORY_PROVIDER_NAME` | `Advisories from documents` | Provider name for advisory catalogs | | `NETWORK_SCHEMES_PROVIDER_NAME` | `Schemes from documents` | Provider name for scheme catalogs | | `NETWORK_GROUP_PROVIDER_NAME_TEMPLATE` | `{group_name} {provider_name}` | Provider and catalog name for a Group, e.g. `Maharashtra Advisories from documents` | @@ -277,7 +277,7 @@ Each catalog's provider is named per Knowledge Kind and instance: id and descrip | OCR / translation / chunking / domain tags | — | config / status | ✅ runs jobs | | Qdrant (`VECTOR_DB_*`) | — | ✅ | ✅ | | Publish to Network (`DISCOVERY_SERVICE_ENDPOINT`, `NETWORK_SENDER_ID`) | — | — | ✅ runs the publish activity | -| Group Provider config (`NETWORK_GROUP_PROVIDERS_CONFIG_PATH`, `NETWORK_*_PROVIDER_*`) | — | validated at boot | ✅ validated at boot, used by the publish activity | +| Network provider config (`NETWORK_GROUP_PROVIDERS_CONFIG_PATH`, `NETWORK_*_PROVIDER_*`) | — | validated at boot | ✅ validated at boot, used by the publish activity | --- diff --git a/deploy/network_group_providers.example.json b/deploy/network_group_providers.example.json index 4a7b3ea..23f63eb 100644 --- a/deploy/network_group_providers.example.json +++ b/deploy/network_group_providers.example.json @@ -1,3 +1,6 @@ -[ - "mh" -] +{ + "ka": { + "advisory": "advisory.ka.com", + "scheme": "scheme.ka.com" + } +} diff --git a/docs/ADR/0006-publish-to-network-under-per-group-provider.md b/docs/ADR/0006-publish-to-network-under-per-group-provider.md index cc156d5..a96ebda 100644 --- a/docs/ADR/0006-publish-to-network-under-per-group-provider.md +++ b/docs/ADR/0006-publish-to-network-under-per-group-provider.md @@ -11,14 +11,13 @@ Decision: - Documents with a Group use the group code as the instance. Catalog and resource ids are today's ids prefixed with the group code (`mh-cat-oan-knowledge-provider-advisories`, `mh-res-oan-knowledge-provider-advisories`, and the same for schemes), and the catalog takes the provider's name. - The group code is derived from the document's `instance`, not from the uploader's group, so a Super Admin uploading for a state publishes under that state. - Documents with no Group (Bharat Vistaar `bv`, `default`) use the portal instance `bv` in the provider id and keep ADR 0003's unprefixed catalog and resource ids. -- A Group not listed in that file skips the publish with a warning, like an unmapped Knowledge Kind. It is not a failure. A real publish failure stays fatal, per ADR 0004. +- Every Group publishes; only an unmapped Knowledge Kind skips. A real publish failure stays fatal, per ADR 0004. - The provider id template, the per-kind provider names and the Group name template are environment settings, so a deployment can change them without a code change. A template with an unknown placeholder stops the API and the worker from booting. -- The Groups that publish are listed in a JSON array whose path is `NETWORK_GROUP_PROVIDERS_CONFIG_PATH` (codes case-insensitive), defaulting to a bundled empty `pipeline/network_group_providers.json`. Every id is derived, so onboarding a Group is one entry in that file. A missing or malformed file stops both the API and the worker from booting, so a bad deploy can't quietly skip every Group. -- A Group skip is recorded with `reason_code: group_provider_not_configured` and shown as a warning on the document page, but only while it is the document's latest publish attempt. A Knowledge Kind skip shows nothing. +- An instance that wants its own provider id overrides it per Knowledge Kind in a JSON file whose path is `NETWORK_GROUP_PROVIDERS_CONFIG_PATH`, e.g. `{"ka": {"advisory": "advisory.ka.com", "scheme": "scheme.ka.com"}}`. Instances and kinds it leaves out keep the template id. It defaults to a bundled empty `pipeline/network_group_providers.json`, so onboarding a Group needs no config. A missing or malformed file stops both the API and the worker from booting, so a bad deploy can't quietly publish under the wrong id. - "No Group" follows the configured `PORTAL_INSTANCE` and `DEFAULT_INSTANCE` values rather than hardcoding `bv` and `default`. - The publish endpoint stays the same for every Group. Consequences: -- Ids are permanent under `MERGE` (ADR 0003). Documents already published under the old shared provider `knowledge-provider` are not moved, so state knowledge published before this change stays there; the next `bv` publish re-announces the unprefixed catalogs under `advisory.bv`/`scheme.bv`. Changing the id template later strands entries the same way. +- Ids are permanent under `MERGE` (ADR 0003). Documents already published under the old shared provider `knowledge-provider` are not moved, so state knowledge published before this change stays there; the next `bv` publish re-announces the unprefixed catalogs under `advisory.bv`/`scheme.bv`. Changing the id template or an override later strands entries the same way. - Depends on the network adapter layer accepting the per-Group provider, catalog and resource ids. - Only the Publish to Network payload changes. Chunks and the search indexes gain no `group` field; they keep `instance` and `instance_name`, where Bharat Vistaar and `default` documents carry `bv`/`default` and state documents carry the state code, so #202 can filter on `instance`. diff --git a/pipeline/activities.py b/pipeline/activities.py index a9ab952..1ef09c1 100644 --- a/pipeline/activities.py +++ b/pipeline/activities.py @@ -26,11 +26,7 @@ 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 ( - SKIP_GROUP_PROVIDER_NOT_CONFIGURED, - SKIP_UNMAPPED_KIND, - DiscoveryPublishService, -) +from .discovery_publish_service import DiscoveryPublishService from .document_repository import DocumentRepository from .instances import instance_display_name from .network_constants import RESULT_REJECTED @@ -1478,21 +1474,10 @@ async def publish_catalog_to_network(workflow_id: str, transaction_id: str) -> d if result["skipped"]: artifact_type = "network_publish_skipped" filename = "network_publish_skipped.json" - skip_reason = result.get("skip_reason") or SKIP_UNMAPPED_KIND - if skip_reason == SKIP_GROUP_PROVIDER_NOT_CONFIGURED: - reason = ( - "Not published: no network provider is configured for " - f"{instance_display_name(group)} ({group})" - ) - activity.logger.warning("workflow_id=%s %s", workflow_id, reason) - else: - reason = "no network catalog is mapped to this document kind" exchange = { "skipped": True, "document_kind": document_kind, - "group": group, - "reason_code": skip_reason, - "reason": reason, + "reason": "no network catalog is mapped to this document kind", } else: artifact_type = "network_publish_payload" diff --git a/pipeline/catalog_builder.py b/pipeline/catalog_builder.py index ef1020d..05c1025 100644 --- a/pipeline/catalog_builder.py +++ b/pipeline/catalog_builder.py @@ -96,10 +96,13 @@ class ProviderNaming: "scheme": SCHEMES_PROVIDER_NAME, } ) + provider_id_overrides: Mapping[str, Mapping[str, str]] = field(default_factory=dict) def provider(self, kind: str, group: Optional[Group]) -> dict: instance = group.code if group is not None else self.shared_instance - provider_id = self.id_template.format(kind=kind, instance=instance) + provider_id = self.provider_id_overrides.get(instance, {}).get(kind) or self.id_template.format( + kind=kind, instance=instance + ) name = self.provider_names[kind] if group is not None: name = self.group_name_template.format(group_name=group.name, provider_name=name) diff --git a/pipeline/discovery_publish_service.py b/pipeline/discovery_publish_service.py index 8337155..338b9c5 100644 --- a/pipeline/discovery_publish_service.py +++ b/pipeline/discovery_publish_service.py @@ -19,13 +19,10 @@ from .catalog_builder import Group, build_catalog, is_publishable_kind from .instances import instance_display_name from .network_constants import BECKN_VERSION, RESULT_ACCEPTED -from .network_group import load_group_codes, load_provider_naming +from .network_group import load_provider_naming logger = logging.getLogger(__name__) -SKIP_UNMAPPED_KIND = "unmapped_kind" -SKIP_GROUP_PROVIDER_NOT_CONFIGURED = "group_provider_not_configured" - class DiscoveryPublishService: """POSTs a Beckn-shaped catalog/publish envelope to the Discovery Service.""" @@ -46,7 +43,6 @@ def __init__( raise RuntimeError("NETWORK_SENDER_ID must be set to publish to the network.") self.timeout = timeout - self.group_codes = load_group_codes() self.provider_naming = load_provider_naming() def publish( @@ -73,20 +69,10 @@ def publish( workflow_id, document_kind, ) - return self._skipped(SKIP_UNMAPPED_KIND) + return self._skipped() catalog_group = None if group is not None: - if group not in self.group_codes: - logger.warning( - "workflow_id=%s document_kind=%s group=%s network_publish_skipped=True " - "reason=%s", - workflow_id, - document_kind, - group, - SKIP_GROUP_PROVIDER_NOT_CONFIGURED, - ) - return self._skipped(SKIP_GROUP_PROVIDER_NOT_CONFIGURED) catalog_group = Group(code=group, name=instance_display_name(group)) catalog = build_catalog(document_kind, catalog_group, self.provider_naming) @@ -164,7 +150,6 @@ def publish( result_status, errors = self._read_result(response, catalog["id"], workflow_id) return { "skipped": False, - "skip_reason": None, "envelope": envelope, "status_code": response.status_code, "response_body": response.text, @@ -173,10 +158,9 @@ def publish( } @staticmethod - def _skipped(reason: str) -> dict: + def _skipped() -> dict: return { "skipped": True, - "skip_reason": reason, "envelope": None, "status_code": None, "response_body": None, diff --git a/pipeline/network_group.py b/pipeline/network_group.py index 2d389eb..82dcf3e 100644 --- a/pipeline/network_group.py +++ b/pipeline/network_group.py @@ -5,7 +5,7 @@ from pathlib import Path from typing import Optional -from .catalog_builder import ProviderNaming +from .catalog_builder import PUBLISHABLE_DOCUMENT_KINDS, ProviderNaming from .instances import PORTAL_INSTANCE from .network_constants import ( ADVISORY_PROVIDER_NAME, @@ -45,27 +45,46 @@ def group_for_instance(instance: Optional[str]) -> Optional[str]: return code -def parse_group_codes(raw: Optional[str], source: str) -> frozenset[str]: +def parse_provider_id_overrides(raw: Optional[str], source: str) -> dict[str, dict[str, str]]: if raw is None or not raw.strip(): - return frozenset() + return {} try: parsed = json.loads(raw) except json.JSONDecodeError as exc: raise NetworkProviderConfigError(f"{source} is not valid JSON: {exc}") from exc - if not isinstance(parsed, list): - raise NetworkProviderConfigError(f"{source} must be a JSON array of group codes") + if not isinstance(parsed, dict): + raise NetworkProviderConfigError( + f"{source} must be a JSON object of instance code to {{kind: provider id}}" + ) - codes: set[str] = set() - for entry in parsed: - code = entry.strip().lower() if isinstance(entry, str) else "" + overrides: dict[str, dict[str, str]] = {} + for key, kinds in parsed.items(): + code = key.strip().lower() if isinstance(key, str) else "" if not code: - raise NetworkProviderConfigError(f"{source} entry {entry!r} must be a non-empty group code") - if code in codes: - raise NetworkProviderConfigError(f"{source} names group {code!r} more than once") - codes.add(code) - return frozenset(codes) + raise NetworkProviderConfigError(f"{source} entry {key!r} must be a non-empty instance code") + if code in overrides: + raise NetworkProviderConfigError(f"{source} names instance {code!r} more than once") + if not isinstance(kinds, dict) or not kinds: + raise NetworkProviderConfigError( + f"{source} entry {key!r} must map knowledge kinds to provider ids" + ) + provider_ids: dict[str, str] = {} + for kind, value in kinds.items(): + normalized_kind = kind.strip().lower() if isinstance(kind, str) else "" + provider_id = value.strip() if isinstance(value, str) else "" + if normalized_kind not in PUBLISHABLE_DOCUMENT_KINDS: + raise NetworkProviderConfigError( + f"{source} entry {key!r} kind {kind!r} must be one of {list(PUBLISHABLE_DOCUMENT_KINDS)}" + ) + if not provider_id: + raise NetworkProviderConfigError( + f"{source} entry {key!r} kind {kind!r} must have a non-empty provider id" + ) + provider_ids[normalized_kind] = provider_id + overrides[code] = provider_ids + return overrides def group_providers_config_path() -> Path: @@ -73,13 +92,13 @@ def group_providers_config_path() -> Path: return Path(configured) if configured else DEFAULT_GROUP_PROVIDERS_CONFIG_PATH -def load_group_codes() -> frozenset[str]: +def load_provider_id_overrides() -> dict[str, dict[str, str]]: path = group_providers_config_path() try: raw = path.read_text(encoding="utf-8") except OSError as exc: - raise NetworkProviderConfigError(f"Group provider config {path} cannot be read: {exc}") from exc - return parse_group_codes(raw, str(path)) + raise NetworkProviderConfigError(f"Provider id overrides {path} cannot be read: {exc}") from exc + return parse_provider_id_overrides(raw, str(path)) def _checked_template(env_name: str, default: str, **placeholders: str) -> str: @@ -111,9 +130,9 @@ def load_provider_naming() -> ProviderNaming: "advisory": _env(ADVISORY_PROVIDER_NAME_ENV, ADVISORY_PROVIDER_NAME), "scheme": _env(SCHEMES_PROVIDER_NAME_ENV, SCHEMES_PROVIDER_NAME), }, + provider_id_overrides=load_provider_id_overrides(), ) def validate_network_provider_config() -> None: - load_group_codes() load_provider_naming() diff --git a/pipeline/network_group_providers.json b/pipeline/network_group_providers.json index fe51488..0967ef4 100644 --- a/pipeline/network_group_providers.json +++ b/pipeline/network_group_providers.json @@ -1 +1 @@ -[] +{} diff --git a/tests/test_activities.py b/tests/test_activities.py index e383c26..37de1c6 100644 --- a/tests/test_activities.py +++ b/tests/test_activities.py @@ -760,15 +760,11 @@ async def test_passes_the_documents_group(self, monkeypatch, instance, group): @pytest.mark.unit @pytest.mark.asyncio - async def test_unconfigured_group_records_a_warning_skip(self, monkeypatch, caplog): - import logging - + async def test_unmapped_kind_records_a_skip(self, monkeypatch): import pipeline.activities as activities - from pipeline.discovery_publish_service import SKIP_GROUP_PROVIDER_NOT_CONFIGURED skipped = { "skipped": True, - "skip_reason": SKIP_GROUP_PROVIDER_NOT_CONFIGURED, "envelope": None, "status_code": None, "response_body": None, @@ -777,47 +773,16 @@ async def test_unconfigured_group_records_a_warning_skip(self, monkeypatch, capl } recorded = self._wire( monkeypatch, - {"document_kind": "advisory", "instance": "mh"}, + {"document_kind": "document", "instance": "mh"}, skipped, {}, ) - with caplog.at_level(logging.WARNING): - result = await activities.publish_catalog_to_network("wf-s", "txn-s") + result = await activities.publish_catalog_to_network("wf-u", "txn-u") assert result["status"] == "skipped" assert recorded["artifact_type"] == "network_publish_skipped" - metadata = recorded["metadata"] - assert metadata["reason_code"] == SKIP_GROUP_PROVIDER_NOT_CONFIGURED - assert metadata["group"] == "mh" - assert "Maharashtra (mh)" in metadata["reason"] - assert any("Maharashtra (mh)" in r.getMessage() for r in caplog.records if r.levelno == logging.WARNING) - - @pytest.mark.unit - @pytest.mark.asyncio - async def test_unmapped_kind_skip_carries_its_reason_code(self, monkeypatch): - import pipeline.activities as activities - from pipeline.discovery_publish_service import SKIP_UNMAPPED_KIND - - skipped = { - "skipped": True, - "skip_reason": SKIP_UNMAPPED_KIND, - "envelope": None, - "status_code": None, - "response_body": None, - "result_status": None, - "errors": [], - } - recorded = self._wire( - monkeypatch, - {"document_kind": "document", "instance": "mh"}, - skipped, - {}, - ) - - await activities.publish_catalog_to_network("wf-u", "txn-u") - - assert recorded["metadata"]["reason_code"] == SKIP_UNMAPPED_KIND + assert recorded["metadata"]["reason"] == "no network catalog is mapped to this document kind" class TestWorkerActivityRegistration: diff --git a/tests/test_catalog_builder.py b/tests/test_catalog_builder.py index 533afed..e5e5adb 100644 --- a/tests/test_catalog_builder.py +++ b/tests/test_catalog_builder.py @@ -251,3 +251,36 @@ def test_group_provider_follows_the_configured_naming(self): "id": "mh-advisory.example.net", "descriptor": {"code": "mh-advisory.example.net", "name": "Advice (Maharashtra)"}, } + + +class TestProviderIdOverrides: + NAMING = ProviderNaming( + provider_id_overrides={ + "ka": {"advisory": "advisory.ka.com", "scheme": "scheme.ka.com"}, + "tn": {"scheme": "scheme.tn.com"}, + } + ) + + @pytest.mark.unit + @pytest.mark.parametrize("kind,provider_id", [("advisory", "advisory.ka.com"), ("scheme", "scheme.ka.com")]) + def test_overridden_group_uses_its_own_provider_id(self, kind, provider_id): + catalog = build_catalog(kind, Group(code="ka", name="Karnataka"), self.NAMING) + + assert catalog["provider"]["id"] == provider_id + assert catalog["provider"]["descriptor"]["code"] == provider_id + assert catalog["id"].startswith("ka-") + + @pytest.mark.unit + def test_a_kind_without_an_override_falls_back_to_the_template(self): + catalog = build_catalog("advisory", Group(code="tn", name="Tamil Nadu"), self.NAMING) + + assert catalog["provider"]["id"] == "advisory.tn.provider.oan.dev" + + @pytest.mark.unit + def test_group_without_overrides_uses_the_defaults(self): + catalog = build_catalog("scheme", MAHARASHTRA, self.NAMING) + + assert catalog["provider"] == { + "id": "scheme.mh.provider.oan.dev", + "descriptor": {"code": "scheme.mh.provider.oan.dev", "name": "Maharashtra Schemes from documents"}, + } diff --git a/tests/test_discovery_publish_service.py b/tests/test_discovery_publish_service.py index 1543bd7..e3ac25c 100644 --- a/tests/test_discovery_publish_service.py +++ b/tests/test_discovery_publish_service.py @@ -466,7 +466,7 @@ def test_configured_group_publishes_under_its_own_provider(self, monkeypatch, tm monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") - _use_group_providers(monkeypatch, tmp_path, '["MH"]') + _use_group_providers(monkeypatch, tmp_path, '{}') service = DiscoveryPublishService() client = _client_returning( _on_publish_response("mh-cat-oan-knowledge-provider-advisories", "ACCEPTED") @@ -492,33 +492,50 @@ def test_configured_group_publishes_under_its_own_provider(self, monkeypatch, tm assert result["result_status"] == "ACCEPTED" @pytest.mark.unit - def test_unconfigured_group_is_skipped_without_an_http_call(self, monkeypatch, tmp_path): - from pipeline.discovery_publish_service import ( - SKIP_GROUP_PROVIDER_NOT_CONFIGURED, - DiscoveryPublishService, - ) + def test_group_without_an_override_publishes_under_the_default_provider(self, monkeypatch, tmp_path): + from pipeline.discovery_publish_service import DiscoveryPublishService monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") - _use_group_providers(monkeypatch, tmp_path, '["mh"]') + _use_group_providers(monkeypatch, tmp_path, '{"ka": {"scheme": "scheme.ka.com"}}') service = DiscoveryPublishService() - client = MagicMock() + client = _client_returning( + _on_publish_response("mh-cat-oan-knowledge-provider-schemes", "ACCEPTED") + ) with patch("pipeline.discovery_publish_service.httpx.Client", return_value=client): - result = service.publish(transaction_id="txn-ka", document_kind="advisory", group="ka") + result = service.publish(transaction_id="txn-mh", document_kind="scheme", group="mh") - client.post.assert_not_called() - assert result["skipped"] is True - assert result["skip_reason"] == SKIP_GROUP_PROVIDER_NOT_CONFIGURED - assert result["envelope"] is None + catalog = client.post.call_args.kwargs["json"]["message"]["catalogs"][0] + assert catalog["provider"]["id"] == "scheme.mh.provider.oan.dev" + assert result["skipped"] is False @pytest.mark.unit - def test_unmapped_kind_wins_over_an_unconfigured_group(self, monkeypatch, tmp_path): - from pipeline.discovery_publish_service import ( - SKIP_UNMAPPED_KIND, - DiscoveryPublishService, + def test_overridden_group_publishes_under_its_configured_provider(self, monkeypatch, tmp_path): + from pipeline.discovery_publish_service import DiscoveryPublishService + + monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") + monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") + _use_group_providers(monkeypatch, tmp_path, '{"KA": {"scheme": "scheme.ka.com"}}') + service = DiscoveryPublishService() + client = _client_returning( + _on_publish_response("ka-cat-oan-knowledge-provider-schemes", "ACCEPTED") ) + with patch("pipeline.discovery_publish_service.httpx.Client", return_value=client): + service.publish(transaction_id="txn-ka", document_kind="scheme", group="ka") + + catalog = client.post.call_args.kwargs["json"]["message"]["catalogs"][0] + assert catalog["id"] == "ka-cat-oan-knowledge-provider-schemes" + assert catalog["provider"] == { + "id": "scheme.ka.com", + "descriptor": {"code": "scheme.ka.com", "name": "Karnataka Schemes from documents"}, + } + + @pytest.mark.unit + def test_unmapped_kind_is_skipped_for_a_group(self, monkeypatch): + from pipeline.discovery_publish_service import DiscoveryPublishService + monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") monkeypatch.delenv("NETWORK_GROUP_PROVIDERS_CONFIG_PATH", raising=False) @@ -527,7 +544,7 @@ def test_unmapped_kind_wins_over_an_unconfigured_group(self, monkeypatch, tmp_pa result = service.publish(transaction_id="txn-doc", document_kind="document", group="ka") assert result["skipped"] is True - assert result["skip_reason"] == SKIP_UNMAPPED_KIND + assert result["envelope"] is None @pytest.mark.unit def test_no_group_publishes_under_the_shared_provider(self, monkeypatch, tmp_path): @@ -535,7 +552,7 @@ def test_no_group_publishes_under_the_shared_provider(self, monkeypatch, tmp_pat monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") - _use_group_providers(monkeypatch, tmp_path, '["mh"]') + _use_group_providers(monkeypatch, tmp_path, '{}') service = DiscoveryPublishService() client = _client_returning( _on_publish_response("cat-oan-knowledge-provider-advisories", "ACCEPTED") @@ -563,7 +580,7 @@ def test_provider_naming_is_read_from_the_environment(self, monkeypatch, tmp_pat monkeypatch.setenv("NETWORK_PROVIDER_ID_TEMPLATE", "{kind}.{instance}.provider.oan.prod") monkeypatch.setenv("NETWORK_GROUP_PROVIDER_NAME_TEMPLATE", "{group_name} - {provider_name}") monkeypatch.setenv("NETWORK_SCHEMES_PROVIDER_NAME", "Government schemes") - _use_group_providers(monkeypatch, tmp_path, '["mh"]') + _use_group_providers(monkeypatch, tmp_path, '{}') service = DiscoveryPublishService() client = _client_returning( _on_publish_response("mh-cat-oan-knowledge-provider-schemes", "ACCEPTED") diff --git a/tests/test_network_group.py b/tests/test_network_group.py index 664a5d7..6a3b4ba 100644 --- a/tests/test_network_group.py +++ b/tests/test_network_group.py @@ -7,9 +7,9 @@ GROUP_PROVIDERS_CONFIG_PATH_ENV, NetworkProviderConfigError, group_for_instance, - load_group_codes, + load_provider_id_overrides, load_provider_naming, - parse_group_codes, + parse_provider_id_overrides, ) @@ -52,31 +52,42 @@ def test_follows_a_configured_default_instance(self, monkeypatch): assert group_for_instance("mh") == "mh" -class TestParseGroupCodes: +class TestParseProviderIdOverrides: @pytest.mark.unit @pytest.mark.parametrize("raw", [None, "", " "]) - def test_unset_means_no_group_is_onboarded(self, raw): - assert parse_group_codes(raw, "providers.json") == frozenset() + def test_unset_means_no_overrides(self, raw): + assert parse_provider_id_overrides(raw, "providers.json") == {} @pytest.mark.unit - def test_codes_are_case_insensitive_and_trimmed(self): - assert parse_group_codes('[" MH ", "ka"]', "providers.json") == {"mh", "ka"} + def test_codes_and_kinds_are_case_insensitive_and_ids_trimmed(self): + parsed = parse_provider_id_overrides( + '{" KA ": {"Advisory": " advisory.ka.com ", "scheme": "scheme.ka.com"}, "tn": {"scheme": "s.tn"}}', + "providers.json", + ) + + assert parsed == { + "ka": {"advisory": "advisory.ka.com", "scheme": "scheme.ka.com"}, + "tn": {"scheme": "s.tn"}, + } @pytest.mark.unit @pytest.mark.parametrize( "raw", [ "not json", - '{"mh": "mh.provider.oan.dev"}', - '"mh"', - "[42]", - '[""]', - '["MH", "mh"]', + '["ka"]', + '{"ka": "advisory.ka.com"}', + '{"ka": {}}', + '{"": {"advisory": "advisory.ka.com"}}', + '{"ka": {"video": "video.ka.com"}}', + '{"ka": {"advisory": ""}}', + '{"ka": {"advisory": 42}}', + '{"KA": {"advisory": "a"}, "ka": {"advisory": "b"}}', ], ) - def test_malformed_list_is_rejected(self, raw): + def test_malformed_overrides_are_rejected(self, raw): with pytest.raises(NetworkProviderConfigError, match="providers.json"): - parse_group_codes(raw, "providers.json") + parse_provider_id_overrides(raw, "providers.json") def _providers_file(monkeypatch, tmp_path, content): @@ -86,19 +97,19 @@ def _providers_file(monkeypatch, tmp_path, content): return path -class TestLoadGroupCodes: +class TestLoadProviderIdOverrides: @pytest.mark.unit def test_reads_the_configured_file(self, monkeypatch, tmp_path): - _providers_file(monkeypatch, tmp_path, '["mh"]') + _providers_file(monkeypatch, tmp_path, '{"ka": {"advisory": "advisory.ka.com"}}') - assert load_group_codes() == {"mh"} + assert load_provider_id_overrides() == {"ka": {"advisory": "advisory.ka.com"}} @pytest.mark.unit def test_unset_path_uses_the_bundled_empty_default(self, monkeypatch): monkeypatch.delenv(GROUP_PROVIDERS_CONFIG_PATH_ENV, raising=False) assert DEFAULT_GROUP_PROVIDERS_CONFIG_PATH.is_file() - assert load_group_codes() == frozenset() + assert load_provider_id_overrides() == {} @pytest.mark.unit def test_missing_file_raises(self, monkeypatch, tmp_path): @@ -106,20 +117,23 @@ def test_missing_file_raises(self, monkeypatch, tmp_path): monkeypatch.setenv(GROUP_PROVIDERS_CONFIG_PATH_ENV, str(missing)) with pytest.raises(NetworkProviderConfigError, match="absent.json"): - load_group_codes() + load_provider_id_overrides() @pytest.mark.unit def test_malformed_file_raises(self, monkeypatch, tmp_path): _providers_file(monkeypatch, tmp_path, "{broken") with pytest.raises(NetworkProviderConfigError, match="network_group_providers.json"): - load_group_codes() + load_provider_id_overrides() @pytest.mark.unit def test_example_file_is_valid(self): example = Path(__file__).parents[1] / "deploy" / "network_group_providers.example.json" - assert "mh" in parse_group_codes(example.read_text(), str(example)) + assert parse_provider_id_overrides(example.read_text(), str(example))["ka"] == { + "advisory": "advisory.ka.com", + "scheme": "scheme.ka.com", + } class TestLoadProviderNaming: @@ -134,7 +148,9 @@ def _clear_naming_env(self, monkeypatch): monkeypatch.delenv(name, raising=False) @pytest.mark.unit - def test_defaults(self): + def test_defaults(self, monkeypatch): + monkeypatch.delenv(GROUP_PROVIDERS_CONFIG_PATH_ENV, raising=False) + naming = load_provider_naming() assert naming.shared_instance == "bv" @@ -144,6 +160,13 @@ def test_defaults(self): "advisory": "Advisories from documents", "scheme": "Schemes from documents", } + assert naming.provider_id_overrides == {} + + @pytest.mark.unit + def test_carries_the_provider_id_overrides(self, monkeypatch, tmp_path): + _providers_file(monkeypatch, tmp_path, '{"ka": {"scheme": "scheme.ka.com"}}') + + assert load_provider_naming().provider_id_overrides == {"ka": {"scheme": "scheme.ka.com"}} @pytest.mark.unit def test_reads_overrides(self, monkeypatch): diff --git a/ui/src/lib/networkPublish.js b/ui/src/lib/networkPublish.js deleted file mode 100644 index 9c989e8..0000000 --- a/ui/src/lib/networkPublish.js +++ /dev/null @@ -1,20 +0,0 @@ -const GROUP_PROVIDER_NOT_CONFIGURED = 'group_provider_not_configured' - -const parseMetadata = artifact => { - try { - return JSON.parse(artifact.metadata_json || 'null') - } catch { - return null - } -} - -export const groupProviderSkipReason = artifacts => { - const latest = (artifacts || []) - .filter(artifact => artifact.stage === 'publishing_to_network') - .reduce((newest, artifact) => (!newest || artifact.id > newest.id ? artifact : newest), null) - - if (latest?.artifact_type !== 'network_publish_skipped') return null - const metadata = parseMetadata(latest) - if (metadata?.reason_code !== GROUP_PROVIDER_NOT_CONFIGURED) return null - return metadata.reason || 'Not published: no network provider is configured for this group' -} diff --git a/ui/src/lib/networkPublish.test.js b/ui/src/lib/networkPublish.test.js deleted file mode 100644 index 324d729..0000000 --- a/ui/src/lib/networkPublish.test.js +++ /dev/null @@ -1,46 +0,0 @@ -import { describe, expect, it } from 'vitest' -import { groupProviderSkipReason } from './networkPublish' - -const artifact = (id, artifactType, metadata) => ({ - id, - artifact_type: artifactType, - stage: 'publishing_to_network', - metadata_json: metadata ? JSON.stringify(metadata) : null, -}) - -const GROUP_SKIP = { - skipped: true, - reason_code: 'group_provider_not_configured', - group: 'mh', - reason: 'Not published: no network provider is configured for Maharashtra (mh)', -} - -describe('groupProviderSkipReason', () => { - it('returns the reason when the latest publish skipped for a missing Group Provider', () => { - expect(groupProviderSkipReason([artifact(3, 'network_publish_skipped', GROUP_SKIP)])).toBe(GROUP_SKIP.reason) - }) - - it('ignores a skip for an unmapped knowledge kind', () => { - const kindSkip = { skipped: true, reason_code: 'unmapped_kind', reason: 'no network catalog is mapped' } - expect(groupProviderSkipReason([artifact(3, 'network_publish_skipped', kindSkip)])).toBeNull() - }) - - it('clears once a later publish went through', () => { - const artifacts = [ - artifact(2, 'network_publish_skipped', GROUP_SKIP), - artifact(5, 'network_publish_payload', { result_status: 'ACCEPTED' }), - ] - expect(groupProviderSkipReason(artifacts)).toBeNull() - }) - - it('ignores artifacts from other stages', () => { - const other = { ...artifact(9, 'chunk_export', { ok: true }), stage: 'chunking' } - expect(groupProviderSkipReason([artifact(3, 'network_publish_skipped', GROUP_SKIP), other])).toBe(GROUP_SKIP.reason) - }) - - it('tolerates missing or unreadable metadata', () => { - expect(groupProviderSkipReason(undefined)).toBeNull() - expect(groupProviderSkipReason([])).toBeNull() - expect(groupProviderSkipReason([{ ...artifact(1, 'network_publish_skipped'), metadata_json: '{bad' }])).toBeNull() - }) -}) diff --git a/ui/src/views/DocumentOpsView.jsx b/ui/src/views/DocumentOpsView.jsx index 96462e2..892fabd 100644 --- a/ui/src/views/DocumentOpsView.jsx +++ b/ui/src/views/DocumentOpsView.jsx @@ -54,7 +54,6 @@ import { summarizeAuditAction, summarizeAvailableAction, } from '../lib/pipelineUi' -import { groupProviderSkipReason } from '../lib/networkPublish' function getChunkEmptyMessage(doc) { const stage = doc?.stage @@ -852,7 +851,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 networkPublishWarning = groupProviderSkipReason(doc?.artifacts) 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]) @@ -1002,10 +1000,6 @@ export default function DocumentOpsView() {
) : null} - {networkPublishWarning ? ( - - ) : null} -
Date: Thu, 15 Oct 2026 17:17:21 +0530 Subject: [PATCH 10/23] docs: every group has a provider [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- CONTEXT.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 058c542..4fd23e7 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -29,11 +29,11 @@ The state a document belongs to, named by its state code (`mh` for Maharashtra) _Avoid_: "tenant"; "state" or "instance" in new features. Never call Bharat Vistaar a Group. **Group Provider**: -The network provider a Group's documents are announced under in Publish to Network, with its own catalog per Knowledge Kind, set per deployment. A Group without one configured publishes nothing. +The network provider a Group's documents are announced under in Publish to Network, one per Knowledge Kind, each with its own catalog. Every Group has one; a deployment may give a Group its own provider id instead of the default. _Avoid_: "state provider". **Shared Provider**: -The single network provider used by documents with no Group (Bharat Vistaar, `default`), keeping the original catalog ids. +The network provider used by documents with no Group (Bharat Vistaar, `default`), one per Knowledge Kind, keeping the original catalog ids. _Avoid_: "national provider", "central provider". **Knowledge Kind**: From ba41ff4728dff64d0f5e6527d08228f25d443c49 Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Thu, 15 Oct 2026 17:23:22 +0530 Subject: [PATCH 11/23] refactor: move provider resolution out of the catalog builder [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- pipeline/catalog_builder.py | 45 +-------- pipeline/discovery_publish_service.py | 6 +- pipeline/network_group.py | 6 +- pipeline/network_provider.py | 41 ++++++++ tests/test_catalog_builder.py | 132 ++++++-------------------- tests/test_network_provider.py | 91 ++++++++++++++++++ 6 files changed, 175 insertions(+), 146 deletions(-) create mode 100644 pipeline/network_provider.py create mode 100644 tests/test_network_provider.py diff --git a/pipeline/catalog_builder.py b/pipeline/catalog_builder.py index 05c1025..eefaa20 100644 --- a/pipeline/catalog_builder.py +++ b/pipeline/catalog_builder.py @@ -11,31 +11,27 @@ """ import copy -from dataclasses import dataclass, field -from typing import Mapping, Optional +from typing import Optional from .network_constants import ( ADVISORY_CATALOG_ID, ADVISORY_CATALOG_NAME, - ADVISORY_PROVIDER_NAME, ADVISORY_RESOURCE_ID, ADVISORY_RESOURCE_NAME, ADVISORY_SUBJECT_CATEGORIES, ADVISORY_TOPICS, DEFAULT_DOCUMENT_KIND, - GROUP_PROVIDER_NAME_TEMPLATE, INFORMATION_MODE, - PROVIDER_ID_TEMPLATE, SCHEMA_CONTEXT_BASE, SCHEME_SUBJECT_CATEGORIES, SCHEME_TOPICS, SCHEMES_CATALOG_ID, SCHEMES_CATALOG_NAME, - SCHEMES_PROVIDER_NAME, SCHEMES_RESOURCE_ID, SCHEMES_RESOURCE_NAME, SERVED_LANGUAGES, ) +from .network_provider import Group _ADVISORY_RESOURCE_ATTRIBUTES = { "@context": f"{SCHEMA_CONTEXT_BASE}/KnowledgeAdvisory/v0.1/context.jsonld", @@ -79,36 +75,6 @@ PUBLISHABLE_DOCUMENT_KINDS = tuple(_KNOWLEDGE_KIND_CATALOGS) -@dataclass(frozen=True) -class Group: - code: str - name: str - - -@dataclass(frozen=True) -class ProviderNaming: - shared_instance: str = "bv" - id_template: str = PROVIDER_ID_TEMPLATE - group_name_template: str = GROUP_PROVIDER_NAME_TEMPLATE - provider_names: Mapping[str, str] = field( - default_factory=lambda: { - "advisory": ADVISORY_PROVIDER_NAME, - "scheme": SCHEMES_PROVIDER_NAME, - } - ) - provider_id_overrides: Mapping[str, Mapping[str, str]] = field(default_factory=dict) - - def provider(self, kind: str, group: Optional[Group]) -> dict: - instance = group.code if group is not None else self.shared_instance - provider_id = self.provider_id_overrides.get(instance, {}).get(kind) or self.id_template.format( - kind=kind, instance=instance - ) - name = self.provider_names[kind] - if group is not None: - name = self.group_name_template.format(group_name=group.name, provider_name=name) - return {"id": provider_id, "descriptor": {"code": provider_id, "name": name}} - - def normalize_document_kind(document_kind: Optional[str]) -> str: """Coerce a stored `documents.document_kind` to its lookup form.""" return (document_kind or DEFAULT_DOCUMENT_KIND).strip().lower() @@ -120,8 +86,8 @@ def is_publishable_kind(document_kind: Optional[str]) -> bool: def build_catalog( document_kind: Optional[str], + provider: dict, group: Optional[Group] = None, - naming: ProviderNaming = ProviderNaming(), ) -> Optional[dict]: """Return the catalog to announce for `document_kind`, or None. @@ -129,15 +95,14 @@ def build_catalog( operator-entered custom slug. Callers must skip the publish rather than send an empty `catalogs` array, which the spec rejects (`minItems: 1`). """ - kind = normalize_document_kind(document_kind) - spec = _KNOWLEDGE_KIND_CATALOGS.get(kind) + spec = _KNOWLEDGE_KIND_CATALOGS.get(normalize_document_kind(document_kind)) if spec is None: return None catalog_id = spec["catalog_id"] catalog_name = spec["catalog_name"] resource_id = spec["resource_id"] - provider = naming.provider(kind, group) + provider = copy.deepcopy(provider) if group is not None: prefix = f"{group.code}-" catalog_id = prefix + catalog_id diff --git a/pipeline/discovery_publish_service.py b/pipeline/discovery_publish_service.py index 338b9c5..554d9c6 100644 --- a/pipeline/discovery_publish_service.py +++ b/pipeline/discovery_publish_service.py @@ -16,10 +16,11 @@ import httpx -from .catalog_builder import Group, build_catalog, is_publishable_kind +from .catalog_builder import build_catalog, is_publishable_kind, normalize_document_kind from .instances import instance_display_name from .network_constants import BECKN_VERSION, RESULT_ACCEPTED from .network_group import load_provider_naming +from .network_provider import Group logger = logging.getLogger(__name__) @@ -75,7 +76,8 @@ def publish( if group is not None: catalog_group = Group(code=group, name=instance_display_name(group)) - catalog = build_catalog(document_kind, catalog_group, self.provider_naming) + provider = self.provider_naming.provider(normalize_document_kind(document_kind), catalog_group) + catalog = build_catalog(document_kind, provider, catalog_group) envelope = { "context": { diff --git a/pipeline/network_group.py b/pipeline/network_group.py index 82dcf3e..0d9af3f 100644 --- a/pipeline/network_group.py +++ b/pipeline/network_group.py @@ -5,7 +5,6 @@ from pathlib import Path from typing import Optional -from .catalog_builder import PUBLISHABLE_DOCUMENT_KINDS, ProviderNaming from .instances import PORTAL_INSTANCE from .network_constants import ( ADVISORY_PROVIDER_NAME, @@ -13,6 +12,7 @@ PROVIDER_ID_TEMPLATE, SCHEMES_PROVIDER_NAME, ) +from .network_provider import PROVIDER_KINDS, ProviderNaming GROUP_PROVIDERS_CONFIG_PATH_ENV = "NETWORK_GROUP_PROVIDERS_CONFIG_PATH" DEFAULT_GROUP_PROVIDERS_CONFIG_PATH = Path(__file__).with_name("network_group_providers.json") @@ -74,9 +74,9 @@ def parse_provider_id_overrides(raw: Optional[str], source: str) -> dict[str, di for kind, value in kinds.items(): normalized_kind = kind.strip().lower() if isinstance(kind, str) else "" provider_id = value.strip() if isinstance(value, str) else "" - if normalized_kind not in PUBLISHABLE_DOCUMENT_KINDS: + if normalized_kind not in PROVIDER_KINDS: raise NetworkProviderConfigError( - f"{source} entry {key!r} kind {kind!r} must be one of {list(PUBLISHABLE_DOCUMENT_KINDS)}" + f"{source} entry {key!r} kind {kind!r} must be one of {list(PROVIDER_KINDS)}" ) if not provider_id: raise NetworkProviderConfigError( diff --git a/pipeline/network_provider.py b/pipeline/network_provider.py new file mode 100644 index 0000000..4dac7c6 --- /dev/null +++ b/pipeline/network_provider.py @@ -0,0 +1,41 @@ +from dataclasses import dataclass, field +from typing import Mapping, Optional + +from .network_constants import ( + ADVISORY_PROVIDER_NAME, + GROUP_PROVIDER_NAME_TEMPLATE, + PROVIDER_ID_TEMPLATE, + SCHEMES_PROVIDER_NAME, +) + +PROVIDER_KINDS = ("advisory", "scheme") + + +@dataclass(frozen=True) +class Group: + code: str + name: str + + +@dataclass(frozen=True) +class ProviderNaming: + shared_instance: str = "bv" + id_template: str = PROVIDER_ID_TEMPLATE + group_name_template: str = GROUP_PROVIDER_NAME_TEMPLATE + provider_names: Mapping[str, str] = field( + default_factory=lambda: { + "advisory": ADVISORY_PROVIDER_NAME, + "scheme": SCHEMES_PROVIDER_NAME, + } + ) + provider_id_overrides: Mapping[str, Mapping[str, str]] = field(default_factory=dict) + + def provider(self, kind: str, group: Optional[Group]) -> dict: + instance = group.code if group is not None else self.shared_instance + provider_id = self.provider_id_overrides.get(instance, {}).get(kind) or self.id_template.format( + kind=kind, instance=instance + ) + name = self.provider_names[kind] + if group is not None: + name = self.group_name_template.format(group_name=group.name, provider_name=name) + return {"id": provider_id, "descriptor": {"code": provider_id, "name": name}} diff --git a/tests/test_catalog_builder.py b/tests/test_catalog_builder.py index e5e5adb..ee17169 100644 --- a/tests/test_catalog_builder.py +++ b/tests/test_catalog_builder.py @@ -3,8 +3,6 @@ import pytest from pipeline.catalog_builder import ( - Group, - ProviderNaming, build_catalog, is_publishable_kind, normalize_document_kind, @@ -15,14 +13,15 @@ SCHEMES_CATALOG_ID, SCHEMES_RESOURCE_ID, ) +from pipeline.network_provider import Group -BV_ADVISORY_PROVIDER = { +PROVIDER = { "id": "advisory.bv.provider.oan.dev", "descriptor": {"code": "advisory.bv.provider.oan.dev", "name": "Advisories from documents"}, } -BV_SCHEMES_PROVIDER = { - "id": "scheme.bv.provider.oan.dev", - "descriptor": {"code": "scheme.bv.provider.oan.dev", "name": "Schemes from documents"}, +MH_PROVIDER = { + "id": "advisory.mh.provider.oan.dev", + "descriptor": {"code": "advisory.mh.provider.oan.dev", "name": "Maharashtra Advisories from documents"}, } # Forbidden by the KnowledgeAdvisory schema under informationMode OnDemand. @@ -38,7 +37,7 @@ def _attributes(kind): - return build_catalog(kind)["resources"][0]["resourceAttributes"] + return build_catalog(kind, PROVIDER)["resources"][0]["resourceAttributes"] class TestNormalizeDocumentKind: @@ -60,18 +59,18 @@ def test_normalizes_to_lookup_form(self, raw, expected): class TestBuildAdvisoryCatalog: @pytest.mark.unit def test_catalog_envelope_fields(self): - catalog = build_catalog("advisory") + catalog = build_catalog("advisory", PROVIDER) assert catalog["id"] == ADVISORY_CATALOG_ID assert "bppId" not in catalog assert "bppUri" not in catalog assert catalog["descriptor"] == {"name": "Agricultural advisory from documents"} assert catalog["isActive"] is True - assert catalog["provider"] == BV_ADVISORY_PROVIDER + assert catalog["provider"] == PROVIDER @pytest.mark.unit def test_exactly_one_resource_with_a_stable_id(self): - catalog = build_catalog("advisory") + catalog = build_catalog("advisory", PROVIDER) assert len(catalog["resources"]) == 1 resource = catalog["resources"][0] @@ -98,18 +97,18 @@ def test_omits_fields_forbidden_under_on_demand(self, field): class TestBuildSchemeCatalog: @pytest.mark.unit def test_catalog_envelope_fields(self): - catalog = build_catalog("scheme") + catalog = build_catalog("scheme", PROVIDER) assert catalog["id"] == SCHEMES_CATALOG_ID assert "bppId" not in catalog assert "bppUri" not in catalog assert catalog["descriptor"] == {"name": "Schemes from documents"} assert catalog["isActive"] is True - assert catalog["provider"] == BV_SCHEMES_PROVIDER + assert catalog["provider"] == PROVIDER @pytest.mark.unit def test_exactly_one_resource_with_a_stable_id(self): - catalog = build_catalog("scheme") + catalog = build_catalog("scheme", PROVIDER) assert len(catalog["resources"]) == 1 resource = catalog["resources"][0] @@ -139,7 +138,7 @@ class TestUnmappedKinds: @pytest.mark.unit @pytest.mark.parametrize("kind", ["document", "video", "how_to_faq", "", None]) def test_returns_none_so_the_caller_skips_publishing(self, kind): - assert build_catalog(kind) is None + assert build_catalog(kind, PROVIDER) is None assert is_publishable_kind(kind) is False @pytest.mark.unit @@ -151,136 +150,67 @@ def test_knowledge_kinds_are_publishable(self, kind): class TestBuilderIsolation: @pytest.mark.unit def test_mutating_a_built_catalog_does_not_leak_into_the_next(self): - first = build_catalog("advisory") + first = build_catalog("advisory", PROVIDER) first["resources"][0]["resourceAttributes"]["topics"].append("tampered") - second = build_catalog("advisory") + second = build_catalog("advisory", PROVIDER) assert second["resources"][0]["resourceAttributes"]["topics"] == ["Crop production"] @pytest.mark.unit def test_mutating_a_built_catalogs_provider_does_not_leak_into_the_next(self): - first = build_catalog("advisory") + first = build_catalog("advisory", PROVIDER) first["provider"]["descriptor"]["name"] = "tampered" - second = build_catalog("advisory") + second = build_catalog("advisory", PROVIDER) - assert second["provider"] == BV_ADVISORY_PROVIDER + assert second["provider"] == PROVIDER MAHARASHTRA = Group(code="mh", name="Maharashtra") -class TestGroupProviderCatalog: +class TestGroupCatalog: @pytest.mark.unit @pytest.mark.parametrize( - "kind,catalog_id,resource_id,provider_id,name", + "kind,catalog_id,resource_id", [ ( "advisory", "mh-cat-oan-knowledge-provider-advisories", "mh-res-oan-knowledge-provider-advisories", - "advisory.mh.provider.oan.dev", - "Maharashtra Advisories from documents", ), ( "scheme", "mh-cat-oan-knowledge-provider-schemes", "mh-res-oan-knowledge-provider-schemes", - "scheme.mh.provider.oan.dev", - "Maharashtra Schemes from documents", ), ], ) - def test_ids_and_names_carry_the_group(self, kind, catalog_id, resource_id, provider_id, name): - catalog = build_catalog(kind, MAHARASHTRA) + def test_ids_carry_the_group_and_the_catalog_takes_the_providers_name(self, kind, catalog_id, resource_id): + catalog = build_catalog(kind, MH_PROVIDER, MAHARASHTRA) assert catalog["id"] == catalog_id - assert catalog["descriptor"] == {"name": name} + assert catalog["descriptor"] == {"name": "Maharashtra Advisories from documents"} assert catalog["resources"][0]["id"] == resource_id - assert catalog["provider"] == { - "id": provider_id, - "descriptor": {"code": provider_id, "name": name}, - } + assert catalog["provider"] == MH_PROVIDER @pytest.mark.unit @pytest.mark.parametrize("kind", ["advisory", "scheme"]) - def test_resource_attributes_match_the_shared_provider(self, kind): - grouped = build_catalog(kind, MAHARASHTRA)["resources"][0] - shared = build_catalog(kind)["resources"][0] + def test_resource_attributes_match_the_shared_catalog(self, kind): + grouped = build_catalog(kind, MH_PROVIDER, MAHARASHTRA)["resources"][0] + shared = build_catalog(kind, PROVIDER)["resources"][0] assert grouped["resourceAttributes"] == shared["resourceAttributes"] assert grouped["descriptor"] == shared["descriptor"] @pytest.mark.unit def test_unmapped_kind_still_publishes_nothing(self): - assert build_catalog("document", MAHARASHTRA) is None + assert build_catalog("document", MH_PROVIDER, MAHARASHTRA) is None @pytest.mark.unit - def test_no_group_uses_the_shared_instance(self): - catalog = build_catalog("advisory", None) + def test_no_group_keeps_the_unprefixed_ids(self): + catalog = build_catalog("advisory", PROVIDER, None) assert catalog["id"] == ADVISORY_CATALOG_ID - assert catalog["provider"] == BV_ADVISORY_PROVIDER - - -class TestProviderNaming: - NAMING = ProviderNaming( - shared_instance="portal", - id_template="{instance}-{kind}.example.net", - group_name_template="{provider_name} ({group_name})", - provider_names={"advisory": "Advice", "scheme": "Schemes"}, - ) - - @pytest.mark.unit - def test_shared_provider_follows_the_configured_naming(self): - catalog = build_catalog("scheme", None, self.NAMING) - - assert catalog["id"] == SCHEMES_CATALOG_ID - assert catalog["provider"] == { - "id": "portal-scheme.example.net", - "descriptor": {"code": "portal-scheme.example.net", "name": "Schemes"}, - } - - @pytest.mark.unit - def test_group_provider_follows_the_configured_naming(self): - catalog = build_catalog("advisory", MAHARASHTRA, self.NAMING) - - assert catalog["descriptor"] == {"name": "Advice (Maharashtra)"} - assert catalog["provider"] == { - "id": "mh-advisory.example.net", - "descriptor": {"code": "mh-advisory.example.net", "name": "Advice (Maharashtra)"}, - } - - -class TestProviderIdOverrides: - NAMING = ProviderNaming( - provider_id_overrides={ - "ka": {"advisory": "advisory.ka.com", "scheme": "scheme.ka.com"}, - "tn": {"scheme": "scheme.tn.com"}, - } - ) - - @pytest.mark.unit - @pytest.mark.parametrize("kind,provider_id", [("advisory", "advisory.ka.com"), ("scheme", "scheme.ka.com")]) - def test_overridden_group_uses_its_own_provider_id(self, kind, provider_id): - catalog = build_catalog(kind, Group(code="ka", name="Karnataka"), self.NAMING) - - assert catalog["provider"]["id"] == provider_id - assert catalog["provider"]["descriptor"]["code"] == provider_id - assert catalog["id"].startswith("ka-") - - @pytest.mark.unit - def test_a_kind_without_an_override_falls_back_to_the_template(self): - catalog = build_catalog("advisory", Group(code="tn", name="Tamil Nadu"), self.NAMING) - - assert catalog["provider"]["id"] == "advisory.tn.provider.oan.dev" - - @pytest.mark.unit - def test_group_without_overrides_uses_the_defaults(self): - catalog = build_catalog("scheme", MAHARASHTRA, self.NAMING) - - assert catalog["provider"] == { - "id": "scheme.mh.provider.oan.dev", - "descriptor": {"code": "scheme.mh.provider.oan.dev", "name": "Maharashtra Schemes from documents"}, - } + assert catalog["provider"] == PROVIDER diff --git a/tests/test_network_provider.py b/tests/test_network_provider.py new file mode 100644 index 0000000..32d4e0d --- /dev/null +++ b/tests/test_network_provider.py @@ -0,0 +1,91 @@ +import pytest + +from pipeline.network_provider import Group, ProviderNaming + +MAHARASHTRA = Group(code="mh", name="Maharashtra") + + +def _provider(provider_id, name): + return {"id": provider_id, "descriptor": {"code": provider_id, "name": name}} + + +class TestDefaultProviderNaming: + @pytest.mark.unit + @pytest.mark.parametrize( + "kind,group,expected", + [ + ("advisory", None, _provider("advisory.bv.provider.oan.dev", "Advisories from documents")), + ("scheme", None, _provider("scheme.bv.provider.oan.dev", "Schemes from documents")), + ( + "advisory", + MAHARASHTRA, + _provider("advisory.mh.provider.oan.dev", "Maharashtra Advisories from documents"), + ), + ( + "scheme", + MAHARASHTRA, + _provider("scheme.mh.provider.oan.dev", "Maharashtra Schemes from documents"), + ), + ], + ) + def test_provider_per_kind_and_instance(self, kind, group, expected): + assert ProviderNaming().provider(kind, group) == expected + + +class TestConfiguredProviderNaming: + NAMING = ProviderNaming( + shared_instance="portal", + id_template="{instance}-{kind}.example.net", + group_name_template="{provider_name} ({group_name})", + provider_names={"advisory": "Advice", "scheme": "Schemes"}, + ) + + @pytest.mark.unit + def test_shared_provider_follows_the_configured_naming(self): + assert self.NAMING.provider("scheme", None) == _provider("portal-scheme.example.net", "Schemes") + + @pytest.mark.unit + def test_group_provider_follows_the_configured_naming(self): + assert self.NAMING.provider("advisory", MAHARASHTRA) == _provider( + "mh-advisory.example.net", "Advice (Maharashtra)" + ) + + +class TestProviderIdOverrides: + NAMING = ProviderNaming( + provider_id_overrides={ + "ka": {"advisory": "advisory.ka.com", "scheme": "scheme.ka.com"}, + "tn": {"scheme": "scheme.tn.com"}, + "bv": {"advisory": "advisory.bharat.example"}, + } + ) + + @pytest.mark.unit + @pytest.mark.parametrize("kind,provider_id", [("advisory", "advisory.ka.com"), ("scheme", "scheme.ka.com")]) + def test_overridden_group_uses_its_own_provider_id(self, kind, provider_id): + provider = self.NAMING.provider(kind, Group(code="ka", name="Karnataka")) + + assert provider["id"] == provider_id + assert provider["descriptor"]["code"] == provider_id + + @pytest.mark.unit + def test_override_changes_only_the_id(self): + provider = self.NAMING.provider("scheme", Group(code="ka", name="Karnataka")) + + assert provider["descriptor"]["name"] == "Karnataka Schemes from documents" + + @pytest.mark.unit + def test_a_kind_without_an_override_falls_back_to_the_template(self): + provider = self.NAMING.provider("advisory", Group(code="tn", name="Tamil Nadu")) + + assert provider["id"] == "advisory.tn.provider.oan.dev" + + @pytest.mark.unit + def test_group_without_overrides_uses_the_defaults(self): + assert self.NAMING.provider("scheme", MAHARASHTRA) == _provider( + "scheme.mh.provider.oan.dev", "Maharashtra Schemes from documents" + ) + + @pytest.mark.unit + def test_shared_instance_can_be_overridden(self): + assert self.NAMING.provider("advisory", None)["id"] == "advisory.bharat.example" From 69b9fd88b0b48ae7f1ff7217de6c80810c61f719 Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Thu, 15 Oct 2026 17:24:09 +0530 Subject: [PATCH 12/23] docs: list network_provider module [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index ef17b18..d9706ed 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -61,7 +61,8 @@ Core modules (all under `pipeline/`): - `db.py` — SQLite access layer; SQLite is the **canonical** metadata/review-state store (documents, pages, chunks, jobs, artifacts, audit log), not just a cache - `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 +- `catalog_builder.py` — pure builder mapping a document's knowledge kind (`advisory`/`scheme`) to the single `OnDemand` Beckn catalog announced for that kind; takes the resolved provider and prefixes catalog/resource ids for a Group; returns `None` for any other kind. No env, no I/O +- `network_provider.py` — pure provider resolution for Publish to Network: the provider id (overrides, then `{kind}.{instance}` template) and name for a Knowledge Kind and Group. `network_group.py` builds its `ProviderNaming` from env and the overrides file; `catalog_builder.py` takes the resolved provider - `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) From 427afe5c2424a2fb2210a47e5e6675fe336b971f Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Thu, 15 Oct 2026 17:28:17 +0530 Subject: [PATCH 13/23] refactor: rename overrides path to PROVIDER_CONFIG_OVERRIDE_PATH [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- ENV.md | 6 +++--- ...-publish-to-network-under-per-group-provider.md | 2 +- pipeline/network_group.py | 12 ++++++------ tests/test_discovery_publish_service.py | 4 ++-- tests/test_network_group.py | 14 +++++++------- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/ENV.md b/ENV.md index c7726bb..284c3ad 100644 --- a/ENV.md +++ b/ENV.md @@ -249,13 +249,13 @@ Separate from the SQLite/Qdrant catalog above (which tracks PROD vector publicat First piece of the network-discovery push architecture: once a document's PROD promotion succeeds, the `publishing_to_network` pipeline stage POSTs a Beckn-shaped `publish` envelope to an external Discovery Service. It's a PROD-only stage (see `docs/ADR/0004-publishing-to-network-is-prod-only.md`) — skipped entirely when `DISABLE_PROD_SETTING=true`, since a document never reaches the network without first promoting to PROD. This is going to be the only way the experience layer discovers schemas going forward — the Master Scheme Catalog pull API above stays in place for backward compatibility. The catalog announces the *knowledge kind* the document was classified as — one catalog with exactly one `OnDemand` resource for `advisory`, another for `scheme`; any other `document_kind` publishes nothing. `DISCOVERY_SERVICE_ENDPOINT` and `NETWORK_SENDER_ID` are **required** whenever PROD promotion is enabled — there is no separate feature flag for this stage beyond `DISABLE_PROD_SETTING`. -Each catalog's provider is named per Knowledge Kind and instance: id and descriptor code `{kind}.{instance}.provider.oan.dev` (`advisory.bv.provider.oan.dev`, `scheme.mh.provider.oan.dev`), name `Advisories|Schemes from documents`. A document filed under a state (its **Group**, taken from the document's `instance`) uses the group code as the instance, prefixes catalog and resource ids with it (`mh-cat-oan-knowledge-provider-advisories`) and names catalog and provider ` ` (`Maharashtra Advisories from documents`). Documents under the portal instance (`PORTAL_INSTANCE`, `bv`) or the default instance (`DEFAULT_INSTANCE`) have no Group: they use the portal instance in the provider id and keep the unprefixed ids. Every Group publishes. An instance can replace the default provider id per Knowledge Kind in the provider id overrides file (`NETWORK_GROUP_PROVIDERS_CONFIG_PATH`); a kind it leaves out keeps the default. A missing or malformed file, or a template with an unknown placeholder, stops both the API and the worker from booting. The file is not mounted by `docker-compose.yml` — mount your own into the `api` and `worker` containers and point the variable at it; `deploy/network_group_providers.example.json` shows the shape. See `docs/ADR/0006-publish-to-network-under-per-group-provider.md`. +Each catalog's provider is named per Knowledge Kind and instance: id and descriptor code `{kind}.{instance}.provider.oan.dev` (`advisory.bv.provider.oan.dev`, `scheme.mh.provider.oan.dev`), name `Advisories|Schemes from documents`. A document filed under a state (its **Group**, taken from the document's `instance`) uses the group code as the instance, prefixes catalog and resource ids with it (`mh-cat-oan-knowledge-provider-advisories`) and names catalog and provider ` ` (`Maharashtra Advisories from documents`). Documents under the portal instance (`PORTAL_INSTANCE`, `bv`) or the default instance (`DEFAULT_INSTANCE`) have no Group: they use the portal instance in the provider id and keep the unprefixed ids. Every Group publishes. An instance can replace the default provider id per Knowledge Kind in the provider id overrides file (`PROVIDER_CONFIG_OVERRIDE_PATH`); a kind it leaves out keeps the default. A missing or malformed file, or a template with an unknown placeholder, stops both the API and the worker from booting. The file is not mounted by `docker-compose.yml` — mount your own into the `api` and `worker` containers and point the variable at it; `deploy/network_group_providers.example.json` shows the shape. See `docs/ADR/0006-publish-to-network-under-per-group-provider.md`. | Variable | Default | Purpose | |----------|---------|---------| | `DISCOVERY_SERVICE_ENDPOINT` | *(empty — required)* | Base URL of the external Discovery Service; the pipeline POSTs to `{DISCOVERY_SERVICE_ENDPOINT}/publish` | | `NETWORK_SENDER_ID` | *(empty — required)* | This instance's Beckn `senderId`, also sent as `networkId` | -| `NETWORK_GROUP_PROVIDERS_CONFIG_PATH` | unset (bundled `pipeline/network_group_providers.json`, `{}` — no overrides) | Path to a JSON object of instance code → `{kind: provider id}`, set per deployment, e.g. `{"ka": {"advisory": "advisory.ka.com", "scheme": "scheme.ka.com"}}`. Codes and kinds are case-insensitive; kinds are `advisory` or `scheme`. A missing file, invalid JSON, an unknown kind or an empty id stop the API and worker from starting | +| `PROVIDER_CONFIG_OVERRIDE_PATH` | unset (bundled `pipeline/network_group_providers.json`, `{}` — no overrides) | Path to a JSON object of instance code → `{kind: provider id}`, set per deployment, e.g. `{"ka": {"advisory": "advisory.ka.com", "scheme": "scheme.ka.com"}}`. Codes and kinds are case-insensitive; kinds are `advisory` or `scheme`. A missing file, invalid JSON, an unknown kind or an empty id stop the API and worker from starting | | `NETWORK_PROVIDER_ID_TEMPLATE` | `{kind}.{instance}.provider.oan.dev` | Default provider id and descriptor code, unless overridden in the file above. `{kind}` is `advisory` or `scheme`; `{instance}` is the group code, or `PORTAL_INSTANCE` for documents with no Group | | `NETWORK_ADVISORY_PROVIDER_NAME` | `Advisories from documents` | Provider name for advisory catalogs | | `NETWORK_SCHEMES_PROVIDER_NAME` | `Schemes from documents` | Provider name for scheme catalogs | @@ -277,7 +277,7 @@ Each catalog's provider is named per Knowledge Kind and instance: id and descrip | OCR / translation / chunking / domain tags | — | config / status | ✅ runs jobs | | Qdrant (`VECTOR_DB_*`) | — | ✅ | ✅ | | Publish to Network (`DISCOVERY_SERVICE_ENDPOINT`, `NETWORK_SENDER_ID`) | — | — | ✅ runs the publish activity | -| Network provider config (`NETWORK_GROUP_PROVIDERS_CONFIG_PATH`, `NETWORK_*_PROVIDER_*`) | — | validated at boot | ✅ validated at boot, used by the publish activity | +| Network provider config (`PROVIDER_CONFIG_OVERRIDE_PATH`, `NETWORK_*_PROVIDER_*`) | — | validated at boot | ✅ validated at boot, used by the publish activity | --- diff --git a/docs/ADR/0006-publish-to-network-under-per-group-provider.md b/docs/ADR/0006-publish-to-network-under-per-group-provider.md index a96ebda..4389b39 100644 --- a/docs/ADR/0006-publish-to-network-under-per-group-provider.md +++ b/docs/ADR/0006-publish-to-network-under-per-group-provider.md @@ -13,7 +13,7 @@ Decision: - Documents with no Group (Bharat Vistaar `bv`, `default`) use the portal instance `bv` in the provider id and keep ADR 0003's unprefixed catalog and resource ids. - Every Group publishes; only an unmapped Knowledge Kind skips. A real publish failure stays fatal, per ADR 0004. - The provider id template, the per-kind provider names and the Group name template are environment settings, so a deployment can change them without a code change. A template with an unknown placeholder stops the API and the worker from booting. -- An instance that wants its own provider id overrides it per Knowledge Kind in a JSON file whose path is `NETWORK_GROUP_PROVIDERS_CONFIG_PATH`, e.g. `{"ka": {"advisory": "advisory.ka.com", "scheme": "scheme.ka.com"}}`. Instances and kinds it leaves out keep the template id. It defaults to a bundled empty `pipeline/network_group_providers.json`, so onboarding a Group needs no config. A missing or malformed file stops both the API and the worker from booting, so a bad deploy can't quietly publish under the wrong id. +- An instance that wants its own provider id overrides it per Knowledge Kind in a JSON file whose path is `PROVIDER_CONFIG_OVERRIDE_PATH`, e.g. `{"ka": {"advisory": "advisory.ka.com", "scheme": "scheme.ka.com"}}`. Instances and kinds it leaves out keep the template id. It defaults to a bundled empty `pipeline/network_group_providers.json`, so onboarding a Group needs no config. A missing or malformed file stops both the API and the worker from booting, so a bad deploy can't quietly publish under the wrong id. - "No Group" follows the configured `PORTAL_INSTANCE` and `DEFAULT_INSTANCE` values rather than hardcoding `bv` and `default`. - The publish endpoint stays the same for every Group. diff --git a/pipeline/network_group.py b/pipeline/network_group.py index 0d9af3f..dd70992 100644 --- a/pipeline/network_group.py +++ b/pipeline/network_group.py @@ -14,8 +14,8 @@ ) from .network_provider import PROVIDER_KINDS, ProviderNaming -GROUP_PROVIDERS_CONFIG_PATH_ENV = "NETWORK_GROUP_PROVIDERS_CONFIG_PATH" -DEFAULT_GROUP_PROVIDERS_CONFIG_PATH = Path(__file__).with_name("network_group_providers.json") +PROVIDER_CONFIG_OVERRIDE_PATH_ENV = "PROVIDER_CONFIG_OVERRIDE_PATH" +DEFAULT_PROVIDER_CONFIG_OVERRIDE_PATH = Path(__file__).with_name("network_group_providers.json") PROVIDER_ID_TEMPLATE_ENV = "NETWORK_PROVIDER_ID_TEMPLATE" GROUP_PROVIDER_NAME_TEMPLATE_ENV = "NETWORK_GROUP_PROVIDER_NAME_TEMPLATE" ADVISORY_PROVIDER_NAME_ENV = "NETWORK_ADVISORY_PROVIDER_NAME" @@ -87,13 +87,13 @@ def parse_provider_id_overrides(raw: Optional[str], source: str) -> dict[str, di return overrides -def group_providers_config_path() -> Path: - configured = os.environ.get(GROUP_PROVIDERS_CONFIG_PATH_ENV, "").strip() - return Path(configured) if configured else DEFAULT_GROUP_PROVIDERS_CONFIG_PATH +def provider_config_override_path() -> Path: + configured = os.environ.get(PROVIDER_CONFIG_OVERRIDE_PATH_ENV, "").strip() + return Path(configured) if configured else DEFAULT_PROVIDER_CONFIG_OVERRIDE_PATH def load_provider_id_overrides() -> dict[str, dict[str, str]]: - path = group_providers_config_path() + path = provider_config_override_path() try: raw = path.read_text(encoding="utf-8") except OSError as exc: diff --git a/tests/test_discovery_publish_service.py b/tests/test_discovery_publish_service.py index e3ac25c..0318df7 100644 --- a/tests/test_discovery_publish_service.py +++ b/tests/test_discovery_publish_service.py @@ -444,7 +444,7 @@ def test_skipped_publish_logs_no_request(self, monkeypatch, caplog): def _use_group_providers(monkeypatch, tmp_path, content): path = tmp_path / "network_group_providers.json" path.write_text(content) - monkeypatch.setenv("NETWORK_GROUP_PROVIDERS_CONFIG_PATH", str(path)) + monkeypatch.setenv("PROVIDER_CONFIG_OVERRIDE_PATH", str(path)) @pytest.fixture(autouse=True) @@ -538,7 +538,7 @@ def test_unmapped_kind_is_skipped_for_a_group(self, monkeypatch): monkeypatch.setenv("DISCOVERY_SERVICE_ENDPOINT", "https://discovery.example.com") monkeypatch.setenv("NETWORK_SENDER_ID", "docs-pipeline-bv") - monkeypatch.delenv("NETWORK_GROUP_PROVIDERS_CONFIG_PATH", raising=False) + monkeypatch.delenv("PROVIDER_CONFIG_OVERRIDE_PATH", raising=False) service = DiscoveryPublishService() result = service.publish(transaction_id="txn-doc", document_kind="document", group="ka") diff --git a/tests/test_network_group.py b/tests/test_network_group.py index 6a3b4ba..4f3a6c6 100644 --- a/tests/test_network_group.py +++ b/tests/test_network_group.py @@ -3,8 +3,8 @@ import pytest from pipeline.network_group import ( - DEFAULT_GROUP_PROVIDERS_CONFIG_PATH, - GROUP_PROVIDERS_CONFIG_PATH_ENV, + DEFAULT_PROVIDER_CONFIG_OVERRIDE_PATH, + PROVIDER_CONFIG_OVERRIDE_PATH_ENV, NetworkProviderConfigError, group_for_instance, load_provider_id_overrides, @@ -93,7 +93,7 @@ def test_malformed_overrides_are_rejected(self, raw): def _providers_file(monkeypatch, tmp_path, content): path = tmp_path / "network_group_providers.json" path.write_text(content) - monkeypatch.setenv(GROUP_PROVIDERS_CONFIG_PATH_ENV, str(path)) + monkeypatch.setenv(PROVIDER_CONFIG_OVERRIDE_PATH_ENV, str(path)) return path @@ -106,15 +106,15 @@ def test_reads_the_configured_file(self, monkeypatch, tmp_path): @pytest.mark.unit def test_unset_path_uses_the_bundled_empty_default(self, monkeypatch): - monkeypatch.delenv(GROUP_PROVIDERS_CONFIG_PATH_ENV, raising=False) + monkeypatch.delenv(PROVIDER_CONFIG_OVERRIDE_PATH_ENV, raising=False) - assert DEFAULT_GROUP_PROVIDERS_CONFIG_PATH.is_file() + assert DEFAULT_PROVIDER_CONFIG_OVERRIDE_PATH.is_file() assert load_provider_id_overrides() == {} @pytest.mark.unit def test_missing_file_raises(self, monkeypatch, tmp_path): missing = tmp_path / "absent.json" - monkeypatch.setenv(GROUP_PROVIDERS_CONFIG_PATH_ENV, str(missing)) + monkeypatch.setenv(PROVIDER_CONFIG_OVERRIDE_PATH_ENV, str(missing)) with pytest.raises(NetworkProviderConfigError, match="absent.json"): load_provider_id_overrides() @@ -149,7 +149,7 @@ def _clear_naming_env(self, monkeypatch): @pytest.mark.unit def test_defaults(self, monkeypatch): - monkeypatch.delenv(GROUP_PROVIDERS_CONFIG_PATH_ENV, raising=False) + monkeypatch.delenv(PROVIDER_CONFIG_OVERRIDE_PATH_ENV, raising=False) naming = load_provider_naming() From 72f354ff49b2acb998f9e90bdee211369ea351f7 Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Thu, 15 Oct 2026 17:32:57 +0530 Subject: [PATCH 14/23] docs: correct provider move claims and catalog builder docstring [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- ...006-publish-to-network-under-per-group-provider.md | 7 ++++--- pipeline/catalog_builder.py | 11 ++++++----- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/docs/ADR/0006-publish-to-network-under-per-group-provider.md b/docs/ADR/0006-publish-to-network-under-per-group-provider.md index 4389b39..021096e 100644 --- a/docs/ADR/0006-publish-to-network-under-per-group-provider.md +++ b/docs/ADR/0006-publish-to-network-under-per-group-provider.md @@ -7,7 +7,7 @@ status: accepted Context: ADR 0003 announced every document under one provider, with one catalog per Knowledge Kind. A state's experience (for example Maharashtra's, through DSS) needs to consume only its own state's knowledge from the network, and one shared provider gives it no way to do that (engineering-tracker#190). Decision: -- Every catalog has a provider per Knowledge Kind and instance. Its id and descriptor code are `{kind}.{instance}.provider.oan.dev` (`advisory.mh.provider.oan.dev`, `scheme.bv.provider.oan.dev`). It is named `Advisories|Schemes from documents`, prefixed with the Group name for a Group (`Maharashtra Advisories from documents`). +- Every catalog has a provider per Knowledge Kind and instance. By default its id and descriptor code are `{kind}.{instance}.provider.oan.dev` (`advisory.mh.provider.oan.dev`, `scheme.bv.provider.oan.dev`). It is named `Advisories|Schemes from documents`, prefixed with the Group name for a Group (`Maharashtra Advisories from documents`). - Documents with a Group use the group code as the instance. Catalog and resource ids are today's ids prefixed with the group code (`mh-cat-oan-knowledge-provider-advisories`, `mh-res-oan-knowledge-provider-advisories`, and the same for schemes), and the catalog takes the provider's name. - The group code is derived from the document's `instance`, not from the uploader's group, so a Super Admin uploading for a state publishes under that state. - Documents with no Group (Bharat Vistaar `bv`, `default`) use the portal instance `bv` in the provider id and keep ADR 0003's unprefixed catalog and resource ids. @@ -18,6 +18,7 @@ Decision: - The publish endpoint stays the same for every Group. Consequences: -- Ids are permanent under `MERGE` (ADR 0003). Documents already published under the old shared provider `knowledge-provider` are not moved, so state knowledge published before this change stays there; the next `bv` publish re-announces the unprefixed catalogs under `advisory.bv`/`scheme.bv`. Changing the id template or an override later strands entries the same way. -- Depends on the network adapter layer accepting the per-Group provider, catalog and resource ids. +- Catalog and resource ids are permanent under `MERGE` (ADR 0003). State knowledge published before this change stays in the unprefixed catalogs; nothing moves it to the Group's `mh-` catalogs. +- The next `bv` publish re-announces the unprefixed catalogs under `advisory.bv`/`scheme.bv` instead of `knowledge-provider`, keeping their catalog and resource ids. Changing the id template or an override later does the same. Whether the network moves such a catalog to the new provider or keeps the old entry depends on the adapter, and is untested. +- Depends on the network adapter layer accepting the new provider ids, for `bv` as well as Groups, and the Groups' prefixed catalog and resource ids. - Only the Publish to Network payload changes. Chunks and the search indexes gain no `group` field; they keep `instance` and `instance_name`, where Bharat Vistaar and `default` documents carry `bv`/`default` and state documents carry the state code, so #202 can filter on `instance`. diff --git a/pipeline/catalog_builder.py b/pipeline/catalog_builder.py index eefaa20..f1ac72d 100644 --- a/pipeline/catalog_builder.py +++ b/pipeline/catalog_builder.py @@ -2,12 +2,13 @@ Builds the Network Catalog Envelope's catalog for a document's knowledge kind. One catalog holding exactly one OnDemand resource per kind, identical for every -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. +document of that kind and Group - 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 and the transport 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 provider in +network_provider.py and the transport in discovery_publish_service.py. See +docs/ADR/0003-static-ondemand-network-catalog-per-knowledge-kind.md and +docs/ADR/0006-publish-to-network-under-per-group-provider.md. """ import copy From 63be4ad92c5de9bd3485cc8d7ec09fb01c94baa2 Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Thu, 15 Oct 2026 17:37:19 +0530 Subject: [PATCH 15/23] docs: document provider override and naming settings in .env.example [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- .env.example | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/.env.example b/.env.example index e5e0dd3..6c538be 100644 --- a/.env.example +++ b/.env.example @@ -223,6 +223,16 @@ DISCOVERY_SERVICE_ENDPOINT= NETWORK_SENDER_ID= # Publicly reachable base URL of this provider, sent as each catalog's bppUri. NETWORK_SENDER_URI= +# Provider per knowledge kind and instance (ADR 0006). All optional; defaults shown. +# NETWORK_PROVIDER_ID_TEMPLATE={kind}.{instance}.provider.oan.dev +# NETWORK_ADVISORY_PROVIDER_NAME=Advisories from documents +# NETWORK_SCHEMES_PROVIDER_NAME=Schemes from documents +# NETWORK_GROUP_PROVIDER_NAME_TEMPLATE={group_name} {provider_name} +# Per-instance provider id overrides: JSON of instance -> {kind: provider id}, +# e.g. {"ka": {"advisory": "advisory.ka.com", "scheme": "scheme.ka.com"}}. +# A path inside the api and worker containers; mount the file there yourself. +# Unset uses the bundled empty file; a missing or malformed file stops the API and worker from booting. +# PROVIDER_CONFIG_OVERRIDE_PATH=/app/deploy/network_group_providers.example.json # --- Keycloak container (only when running Keycloak inside docker-compose) --- @@ -259,7 +269,3 @@ KC_SMTP_STARTTLS=true KC_SMTP_SSL=false KC_SMTP_USERNAME= KC_SMTP_PASSWORD= - -# Group Provider config file (ADR 0006): JSON of group code -> provider id, set per deployment. -# Unset uses the bundled empty map; a missing or malformed file stops the API and worker from booting. -NETWORK_GROUP_PROVIDERS_CONFIG_PATH=./deploy/network_group_providers.example.json From f67505e0428800653cc7bd531290fa9dc9587509 Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Thu, 15 Oct 2026 17:39:20 +0530 Subject: [PATCH 16/23] docs: list provider settings without comments in .env.example [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- .env.example | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/.env.example b/.env.example index 6c538be..6578363 100644 --- a/.env.example +++ b/.env.example @@ -223,16 +223,11 @@ DISCOVERY_SERVICE_ENDPOINT= NETWORK_SENDER_ID= # Publicly reachable base URL of this provider, sent as each catalog's bppUri. NETWORK_SENDER_URI= -# Provider per knowledge kind and instance (ADR 0006). All optional; defaults shown. -# NETWORK_PROVIDER_ID_TEMPLATE={kind}.{instance}.provider.oan.dev -# NETWORK_ADVISORY_PROVIDER_NAME=Advisories from documents -# NETWORK_SCHEMES_PROVIDER_NAME=Schemes from documents -# NETWORK_GROUP_PROVIDER_NAME_TEMPLATE={group_name} {provider_name} -# Per-instance provider id overrides: JSON of instance -> {kind: provider id}, -# e.g. {"ka": {"advisory": "advisory.ka.com", "scheme": "scheme.ka.com"}}. -# A path inside the api and worker containers; mount the file there yourself. -# Unset uses the bundled empty file; a missing or malformed file stops the API and worker from booting. -# PROVIDER_CONFIG_OVERRIDE_PATH=/app/deploy/network_group_providers.example.json +NETWORK_PROVIDER_ID_TEMPLATE={kind}.{instance}.provider.oan.dev +NETWORK_ADVISORY_PROVIDER_NAME=Advisories from documents +NETWORK_SCHEMES_PROVIDER_NAME=Schemes from documents +NETWORK_GROUP_PROVIDER_NAME_TEMPLATE={group_name} {provider_name} +PROVIDER_CONFIG_OVERRIDE_PATH= # --- Keycloak container (only when running Keycloak inside docker-compose) --- From 6a51589c30de7e3f54520f64381004ca5d3ced0f Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Thu, 15 Oct 2026 17:50:03 +0530 Subject: [PATCH 17/23] fix: require kind and instance in the provider id template [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- ENV.md | 2 +- pipeline/network_group.py | 17 ++++++++++++++--- tests/test_network_group.py | 12 ++++++++++++ 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/ENV.md b/ENV.md index 284c3ad..acdf57d 100644 --- a/ENV.md +++ b/ENV.md @@ -256,7 +256,7 @@ Each catalog's provider is named per Knowledge Kind and instance: id and descrip | `DISCOVERY_SERVICE_ENDPOINT` | *(empty — required)* | Base URL of the external Discovery Service; the pipeline POSTs to `{DISCOVERY_SERVICE_ENDPOINT}/publish` | | `NETWORK_SENDER_ID` | *(empty — required)* | This instance's Beckn `senderId`, also sent as `networkId` | | `PROVIDER_CONFIG_OVERRIDE_PATH` | unset (bundled `pipeline/network_group_providers.json`, `{}` — no overrides) | Path to a JSON object of instance code → `{kind: provider id}`, set per deployment, e.g. `{"ka": {"advisory": "advisory.ka.com", "scheme": "scheme.ka.com"}}`. Codes and kinds are case-insensitive; kinds are `advisory` or `scheme`. A missing file, invalid JSON, an unknown kind or an empty id stop the API and worker from starting | -| `NETWORK_PROVIDER_ID_TEMPLATE` | `{kind}.{instance}.provider.oan.dev` | Default provider id and descriptor code, unless overridden in the file above. `{kind}` is `advisory` or `scheme`; `{instance}` is the group code, or `PORTAL_INSTANCE` for documents with no Group | +| `NETWORK_PROVIDER_ID_TEMPLATE` | `{kind}.{instance}.provider.oan.dev` | Default provider id and descriptor code, unless overridden in the file above. `{kind}` is `advisory` or `scheme`; `{instance}` is the group code, or `PORTAL_INSTANCE` for documents with no Group. Must use both, or the API and worker refuse to start | | `NETWORK_ADVISORY_PROVIDER_NAME` | `Advisories from documents` | Provider name for advisory catalogs | | `NETWORK_SCHEMES_PROVIDER_NAME` | `Schemes from documents` | Provider name for scheme catalogs | | `NETWORK_GROUP_PROVIDER_NAME_TEMPLATE` | `{group_name} {provider_name}` | Provider and catalog name for a Group, e.g. `Maharashtra Advisories from documents` | diff --git a/pipeline/network_group.py b/pipeline/network_group.py index dd70992..c87031f 100644 --- a/pipeline/network_group.py +++ b/pipeline/network_group.py @@ -2,6 +2,7 @@ import json import os +import string from pathlib import Path from typing import Optional @@ -101,16 +102,22 @@ def load_provider_id_overrides() -> dict[str, dict[str, str]]: return parse_provider_id_overrides(raw, str(path)) -def _checked_template(env_name: str, default: str, **placeholders: str) -> str: +def _checked_template( + env_name: str, default: str, required: frozenset[str] = frozenset(), **placeholders: str +) -> str: template = _env(env_name, default) try: rendered = template.format(**placeholders) - except (KeyError, IndexError, ValueError) as exc: + used = {field for _, field, _, _ in string.Formatter().parse(template) if field} + except (KeyError, IndexError, ValueError, AttributeError) as exc: raise NetworkProviderConfigError( f"{env_name}={template!r} may only use {sorted(placeholders)}: {exc}" ) from exc if not rendered.strip(): raise NetworkProviderConfigError(f"{env_name} renders an empty value") + missing = required - used + if missing: + raise NetworkProviderConfigError(f"{env_name}={template!r} must use {sorted(missing)}") return template @@ -118,7 +125,11 @@ def load_provider_naming() -> ProviderNaming: return ProviderNaming( shared_instance=portal_instance(), id_template=_checked_template( - PROVIDER_ID_TEMPLATE_ENV, PROVIDER_ID_TEMPLATE, kind="advisory", instance="bv" + PROVIDER_ID_TEMPLATE_ENV, + PROVIDER_ID_TEMPLATE, + frozenset({"kind", "instance"}), + kind="advisory", + instance="bv", ), group_name_template=_checked_template( GROUP_PROVIDER_NAME_TEMPLATE_ENV, diff --git a/tests/test_network_group.py b/tests/test_network_group.py index 4f3a6c6..34515c0 100644 --- a/tests/test_network_group.py +++ b/tests/test_network_group.py @@ -190,6 +190,7 @@ def test_reads_overrides(self, monkeypatch): ("NETWORK_PROVIDER_ID_TEMPLATE", "{state}.provider.oan.dev"), ("NETWORK_PROVIDER_ID_TEMPLATE", "{0}.provider.oan.dev"), ("NETWORK_PROVIDER_ID_TEMPLATE", "{kind.provider.oan.dev"), + ("NETWORK_PROVIDER_ID_TEMPLATE", "{kind.x}.{instance}.provider.oan.dev"), ("NETWORK_GROUP_PROVIDER_NAME_TEMPLATE", "{group} {provider_name}"), ], ) @@ -199,6 +200,17 @@ def test_unknown_placeholders_are_rejected(self, monkeypatch, env_name, template with pytest.raises(NetworkProviderConfigError, match=env_name): load_provider_naming() + @pytest.mark.unit + @pytest.mark.parametrize( + "template", + ["{kind}.oan.dev", "{instance}.provider.oan.dev", "provider.oan.dev"], + ) + def test_id_template_must_name_both_kind_and_instance(self, monkeypatch, template): + monkeypatch.setenv("NETWORK_PROVIDER_ID_TEMPLATE", template) + + with pytest.raises(NetworkProviderConfigError, match="must use"): + load_provider_naming() + class TestBootRefusesMalformedProviderMap: @pytest.fixture From 720dcb8c308b11b0d7fab7b7fb07a989e7ca1bdb Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Tue, 20 Oct 2026 14:02:41 +0530 Subject: [PATCH 18/23] refactor: reuse tenancy's portal and default instance [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- pipeline/auth/tenancy.py | 7 +++++-- pipeline/network_group.py | 8 ++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/pipeline/auth/tenancy.py b/pipeline/auth/tenancy.py index 7b1d23a..91a0f43 100644 --- a/pipeline/auth/tenancy.py +++ b/pipeline/auth/tenancy.py @@ -26,6 +26,10 @@ def default_instance() -> str: return (os.environ.get("DEFAULT_INSTANCE") or "default").strip().lower() or "default" +def portal_instance() -> str: + return (os.environ.get("PORTAL_INSTANCE") or PORTAL_INSTANCE).strip().lower() or PORTAL_INSTANCE + + def normalize_instance(value: str | None) -> str: text = (value or "").strip().lower() return text or default_instance() @@ -76,8 +80,7 @@ def resolve_create_instance(user: AuthUser, requested: str | None = None) -> str if requested_norm: return normalize_instance(requested_norm) # Prefer portal tag for platform operators when nothing chosen. - portal = (os.environ.get("PORTAL_INSTANCE") or PORTAL_INSTANCE).strip().lower() - return normalize_instance(portal or default_instance()) + return normalize_instance(portal_instance()) allowed = allowed_instances(user) or set() if not allowed: diff --git a/pipeline/network_group.py b/pipeline/network_group.py index c87031f..ef3007b 100644 --- a/pipeline/network_group.py +++ b/pipeline/network_group.py @@ -6,7 +6,7 @@ from pathlib import Path from typing import Optional -from .instances import PORTAL_INSTANCE +from .auth.tenancy import default_instance, portal_instance from .network_constants import ( ADVISORY_PROVIDER_NAME, GROUP_PROVIDER_NAME_TEMPLATE, @@ -31,12 +31,8 @@ def _env(name: str, default: str) -> str: return os.environ.get(name, "").strip() or default -def portal_instance() -> str: - return _env("PORTAL_INSTANCE", PORTAL_INSTANCE).lower() - - def _no_group_instances() -> set[str]: - return {portal_instance(), _env("DEFAULT_INSTANCE", "default").lower()} + return {portal_instance(), default_instance()} def group_for_instance(instance: Optional[str]) -> Optional[str]: From 4e8ac710398de55cbe4d08dc63b0bdb7a51d733d Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Tue, 20 Oct 2026 14:04:55 +0530 Subject: [PATCH 19/23] refactor: take the shared instance from PORTAL_INSTANCE only [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- pipeline/network_provider.py | 2 +- tests/test_network_provider.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pipeline/network_provider.py b/pipeline/network_provider.py index 4dac7c6..d1d1b84 100644 --- a/pipeline/network_provider.py +++ b/pipeline/network_provider.py @@ -19,7 +19,7 @@ class Group: @dataclass(frozen=True) class ProviderNaming: - shared_instance: str = "bv" + shared_instance: str id_template: str = PROVIDER_ID_TEMPLATE group_name_template: str = GROUP_PROVIDER_NAME_TEMPLATE provider_names: Mapping[str, str] = field( diff --git a/tests/test_network_provider.py b/tests/test_network_provider.py index 32d4e0d..7da182c 100644 --- a/tests/test_network_provider.py +++ b/tests/test_network_provider.py @@ -29,7 +29,7 @@ class TestDefaultProviderNaming: ], ) def test_provider_per_kind_and_instance(self, kind, group, expected): - assert ProviderNaming().provider(kind, group) == expected + assert ProviderNaming(shared_instance="bv").provider(kind, group) == expected class TestConfiguredProviderNaming: @@ -53,6 +53,7 @@ def test_group_provider_follows_the_configured_naming(self): class TestProviderIdOverrides: NAMING = ProviderNaming( + shared_instance="bv", provider_id_overrides={ "ka": {"advisory": "advisory.ka.com", "scheme": "scheme.ka.com"}, "tn": {"scheme": "scheme.tn.com"}, From 9348e25d0a9f0bcf97f4409eb4b8fb844c8873aa Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Tue, 20 Oct 2026 14:08:30 +0530 Subject: [PATCH 20/23] refactor: move group_for_instance into tenancy [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- pipeline/auth/tenancy.py | 7 ++++++ pipeline/document_repository.py | 2 +- pipeline/network_group.py | 13 +---------- tests/test_network_group.py | 36 +----------------------------- tests/test_tenancy.py | 39 +++++++++++++++++++++++++++++++++ 5 files changed, 49 insertions(+), 48 deletions(-) diff --git a/pipeline/auth/tenancy.py b/pipeline/auth/tenancy.py index 91a0f43..e7813f6 100644 --- a/pipeline/auth/tenancy.py +++ b/pipeline/auth/tenancy.py @@ -30,6 +30,13 @@ def portal_instance() -> str: return (os.environ.get("PORTAL_INSTANCE") or PORTAL_INSTANCE).strip().lower() or PORTAL_INSTANCE +def group_for_instance(instance: str | None) -> str | None: + code = (instance or "").strip().lower() + if not code or code in {portal_instance(), default_instance()}: + return None + return code + + def normalize_instance(value: str | None) -> str: text = (value or "").strip().lower() return text or default_instance() diff --git a/pipeline/document_repository.py b/pipeline/document_repository.py index cf83033..83ff1ce 100644 --- a/pipeline/document_repository.py +++ b/pipeline/document_repository.py @@ -9,7 +9,7 @@ from typing import Optional from . import db -from .network_group import group_for_instance +from .auth.tenancy import group_for_instance class DocumentRepository: diff --git a/pipeline/network_group.py b/pipeline/network_group.py index ef3007b..f6c70fa 100644 --- a/pipeline/network_group.py +++ b/pipeline/network_group.py @@ -6,7 +6,7 @@ from pathlib import Path from typing import Optional -from .auth.tenancy import default_instance, portal_instance +from .auth.tenancy import portal_instance from .network_constants import ( ADVISORY_PROVIDER_NAME, GROUP_PROVIDER_NAME_TEMPLATE, @@ -31,17 +31,6 @@ def _env(name: str, default: str) -> str: return os.environ.get(name, "").strip() or default -def _no_group_instances() -> set[str]: - return {portal_instance(), default_instance()} - - -def group_for_instance(instance: Optional[str]) -> Optional[str]: - code = (instance or "").strip().lower() - if not code or code in _no_group_instances(): - return None - return code - - def parse_provider_id_overrides(raw: Optional[str], source: str) -> dict[str, dict[str, str]]: if raw is None or not raw.strip(): return {} diff --git a/tests/test_network_group.py b/tests/test_network_group.py index 34515c0..3511a1d 100644 --- a/tests/test_network_group.py +++ b/tests/test_network_group.py @@ -6,7 +6,6 @@ DEFAULT_PROVIDER_CONFIG_OVERRIDE_PATH, PROVIDER_CONFIG_OVERRIDE_PATH_ENV, NetworkProviderConfigError, - group_for_instance, load_provider_id_overrides, load_provider_naming, parse_provider_id_overrides, @@ -14,44 +13,11 @@ @pytest.fixture(autouse=True) -def _default_no_group_instances(monkeypatch): +def _default_instances(monkeypatch): monkeypatch.delenv("PORTAL_INSTANCE", raising=False) monkeypatch.delenv("DEFAULT_INSTANCE", raising=False) -class TestGroupForInstance: - @pytest.mark.unit - @pytest.mark.parametrize( - "instance,expected", - [ - ("mh", "mh"), - (" MH ", "mh"), - ("ka", "ka"), - ("bv", None), - ("BV", None), - ("default", None), - ("", None), - (None, None), - ], - ) - def test_states_are_groups_and_platform_instances_are_not(self, instance, expected): - assert group_for_instance(instance) == expected - - @pytest.mark.unit - def test_follows_a_configured_portal_instance(self, monkeypatch): - monkeypatch.setenv("PORTAL_INSTANCE", "portal") - - assert group_for_instance("portal") is None - assert group_for_instance("mh") == "mh" - - @pytest.mark.unit - def test_follows_a_configured_default_instance(self, monkeypatch): - monkeypatch.setenv("DEFAULT_INSTANCE", "local") - - assert group_for_instance("local") is None - assert group_for_instance("mh") == "mh" - - class TestParseProviderIdOverrides: @pytest.mark.unit @pytest.mark.parametrize("raw", [None, "", " "]) diff --git a/tests/test_tenancy.py b/tests/test_tenancy.py index 60148c2..a2149cb 100644 --- a/tests/test_tenancy.py +++ b/tests/test_tenancy.py @@ -12,6 +12,7 @@ allowed_instances, assert_document_instance_access, assert_instance_access, + group_for_instance, resolve_create_instance, user_can_access_instance, ) @@ -361,3 +362,41 @@ def test_api_helpers_hide_cross_tenant_mutations(db_connection, monkeypatch): api._require_document_for_user("wf-tenant-b-doc", user) assert exc.value.status_code == 404 assert api._document_for_user_or_none("wf-tenant-b-doc", user) is None + + +class TestGroupForInstance: + @pytest.fixture(autouse=True) + def _default_instances(self, monkeypatch): + monkeypatch.delenv("PORTAL_INSTANCE", raising=False) + monkeypatch.delenv("DEFAULT_INSTANCE", raising=False) + + @pytest.mark.unit + @pytest.mark.parametrize( + "instance,expected", + [ + ("mh", "mh"), + (" MH ", "mh"), + ("ka", "ka"), + ("bv", None), + ("BV", None), + ("default", None), + ("", None), + (None, None), + ], + ) + def test_states_are_groups_and_platform_instances_are_not(self, instance, expected): + assert group_for_instance(instance) == expected + + @pytest.mark.unit + def test_follows_a_configured_portal_instance(self, monkeypatch): + monkeypatch.setenv("PORTAL_INSTANCE", "portal") + + assert group_for_instance("portal") is None + assert group_for_instance("mh") == "mh" + + @pytest.mark.unit + def test_follows_a_configured_default_instance(self, monkeypatch): + monkeypatch.setenv("DEFAULT_INSTANCE", "local") + + assert group_for_instance("local") is None + assert group_for_instance("mh") == "mh" From e11ba5d3a8e73ef1a4c4db22e1f480d68aca394d Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Tue, 20 Oct 2026 14:13:31 +0530 Subject: [PATCH 21/23] fix: reject malformed provider ids at boot [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- ENV.md | 4 +- ...ish-to-network-under-per-group-provider.md | 2 +- pipeline/network_group.py | 38 +++++++++++++++---- tests/test_network_group.py | 30 +++++++++++++++ 4 files changed, 63 insertions(+), 11 deletions(-) diff --git a/ENV.md b/ENV.md index acdf57d..2b1aca4 100644 --- a/ENV.md +++ b/ENV.md @@ -255,8 +255,8 @@ Each catalog's provider is named per Knowledge Kind and instance: id and descrip |----------|---------|---------| | `DISCOVERY_SERVICE_ENDPOINT` | *(empty — required)* | Base URL of the external Discovery Service; the pipeline POSTs to `{DISCOVERY_SERVICE_ENDPOINT}/publish` | | `NETWORK_SENDER_ID` | *(empty — required)* | This instance's Beckn `senderId`, also sent as `networkId` | -| `PROVIDER_CONFIG_OVERRIDE_PATH` | unset (bundled `pipeline/network_group_providers.json`, `{}` — no overrides) | Path to a JSON object of instance code → `{kind: provider id}`, set per deployment, e.g. `{"ka": {"advisory": "advisory.ka.com", "scheme": "scheme.ka.com"}}`. Codes and kinds are case-insensitive; kinds are `advisory` or `scheme`. A missing file, invalid JSON, an unknown kind or an empty id stop the API and worker from starting | -| `NETWORK_PROVIDER_ID_TEMPLATE` | `{kind}.{instance}.provider.oan.dev` | Default provider id and descriptor code, unless overridden in the file above. `{kind}` is `advisory` or `scheme`; `{instance}` is the group code, or `PORTAL_INSTANCE` for documents with no Group. Must use both, or the API and worker refuse to start | +| `PROVIDER_CONFIG_OVERRIDE_PATH` | unset (bundled `pipeline/network_group_providers.json`, `{}` — no overrides) | Path to a JSON object of instance code → `{kind: provider id}`, set per deployment, e.g. `{"ka": {"advisory": "advisory.ka.com", "scheme": "scheme.ka.com"}}`. Codes and kinds are case-insensitive; kinds are `advisory` or `scheme`. A missing file, invalid JSON, an unknown kind, or an empty or invalid id (same rule as the template) stop the API and worker from starting | +| `NETWORK_PROVIDER_ID_TEMPLATE` | `{kind}.{instance}.provider.oan.dev` | Default provider id and descriptor code, unless overridden in the file above. `{kind}` is `advisory` or `scheme`; `{instance}` is the group code, or `PORTAL_INSTANCE` for documents with no Group. Must use both and render a valid provider id (lowercase letters, digits, `-` and `.`, no leading, trailing or repeated `.`), or the API and worker refuse to start | | `NETWORK_ADVISORY_PROVIDER_NAME` | `Advisories from documents` | Provider name for advisory catalogs | | `NETWORK_SCHEMES_PROVIDER_NAME` | `Schemes from documents` | Provider name for scheme catalogs | | `NETWORK_GROUP_PROVIDER_NAME_TEMPLATE` | `{group_name} {provider_name}` | Provider and catalog name for a Group, e.g. `Maharashtra Advisories from documents` | diff --git a/docs/ADR/0006-publish-to-network-under-per-group-provider.md b/docs/ADR/0006-publish-to-network-under-per-group-provider.md index 021096e..3c87282 100644 --- a/docs/ADR/0006-publish-to-network-under-per-group-provider.md +++ b/docs/ADR/0006-publish-to-network-under-per-group-provider.md @@ -12,7 +12,7 @@ Decision: - The group code is derived from the document's `instance`, not from the uploader's group, so a Super Admin uploading for a state publishes under that state. - Documents with no Group (Bharat Vistaar `bv`, `default`) use the portal instance `bv` in the provider id and keep ADR 0003's unprefixed catalog and resource ids. - Every Group publishes; only an unmapped Knowledge Kind skips. A real publish failure stays fatal, per ADR 0004. -- The provider id template, the per-kind provider names and the Group name template are environment settings, so a deployment can change them without a code change. A template with an unknown placeholder stops the API and the worker from booting. +- The provider id template, the per-kind provider names and the Group name template are environment settings, so a deployment can change them without a code change. A template with an unknown placeholder, an id template missing `{kind}` or `{instance}`, or a provider id (rendered or overridden) outside lowercase letters, digits, `-` and dot-separated parts stops the API and the worker from booting, since a published id can't be removed later. - An instance that wants its own provider id overrides it per Knowledge Kind in a JSON file whose path is `PROVIDER_CONFIG_OVERRIDE_PATH`, e.g. `{"ka": {"advisory": "advisory.ka.com", "scheme": "scheme.ka.com"}}`. Instances and kinds it leaves out keep the template id. It defaults to a bundled empty `pipeline/network_group_providers.json`, so onboarding a Group needs no config. A missing or malformed file stops both the API and the worker from booting, so a bad deploy can't quietly publish under the wrong id. - "No Group" follows the configured `PORTAL_INSTANCE` and `DEFAULT_INSTANCE` values rather than hardcoding `bv` and `default`. - The publish endpoint stays the same for every Group. diff --git a/pipeline/network_group.py b/pipeline/network_group.py index f6c70fa..17d2c3b 100644 --- a/pipeline/network_group.py +++ b/pipeline/network_group.py @@ -2,6 +2,7 @@ import json import os +import re import string from pathlib import Path from typing import Optional @@ -21,6 +22,7 @@ GROUP_PROVIDER_NAME_TEMPLATE_ENV = "NETWORK_GROUP_PROVIDER_NAME_TEMPLATE" ADVISORY_PROVIDER_NAME_ENV = "NETWORK_ADVISORY_PROVIDER_NAME" SCHEMES_PROVIDER_NAME_ENV = "NETWORK_SCHEMES_PROVIDER_NAME" +PROVIDER_ID_PATTERN = re.compile(r"[a-z0-9-]+(?:\.[a-z0-9-]+)*") class NetworkProviderConfigError(ValueError): @@ -31,6 +33,14 @@ def _env(name: str, default: str) -> str: return os.environ.get(name, "").strip() or default +def _check_provider_id(provider_id: str, source: str) -> None: + if not PROVIDER_ID_PATTERN.fullmatch(provider_id): + raise NetworkProviderConfigError( + f"{source} gives provider id {provider_id!r}; it must be lowercase letters, digits, " + "'-' and '.', with no leading, trailing or repeated '.'" + ) + + def parse_provider_id_overrides(raw: Optional[str], source: str) -> dict[str, dict[str, str]]: if raw is None or not raw.strip(): return {} @@ -68,6 +78,7 @@ def parse_provider_id_overrides(raw: Optional[str], source: str) -> dict[str, di raise NetworkProviderConfigError( f"{source} entry {key!r} kind {kind!r} must have a non-empty provider id" ) + _check_provider_id(provider_id, f"{source} entry {key!r} kind {kind!r}") provider_ids[normalized_kind] = provider_id overrides[code] = provider_ids return overrides @@ -106,16 +117,27 @@ def _checked_template( return template +def _checked_id_template(shared_instance: str) -> str: + template = _checked_template( + PROVIDER_ID_TEMPLATE_ENV, + PROVIDER_ID_TEMPLATE, + frozenset({"kind", "instance"}), + kind="advisory", + instance=shared_instance, + ) + for kind in PROVIDER_KINDS: + for instance in (shared_instance, "mh"): + _check_provider_id( + template.format(kind=kind, instance=instance), f"{PROVIDER_ID_TEMPLATE_ENV}={template!r}" + ) + return template + + def load_provider_naming() -> ProviderNaming: + shared_instance = portal_instance() return ProviderNaming( - shared_instance=portal_instance(), - id_template=_checked_template( - PROVIDER_ID_TEMPLATE_ENV, - PROVIDER_ID_TEMPLATE, - frozenset({"kind", "instance"}), - kind="advisory", - instance="bv", - ), + shared_instance=shared_instance, + id_template=_checked_id_template(shared_instance), group_name_template=_checked_template( GROUP_PROVIDER_NAME_TEMPLATE_ENV, GROUP_PROVIDER_NAME_TEMPLATE, diff --git a/tests/test_network_group.py b/tests/test_network_group.py index 3511a1d..bb31eef 100644 --- a/tests/test_network_group.py +++ b/tests/test_network_group.py @@ -49,6 +49,11 @@ def test_codes_and_kinds_are_case_insensitive_and_ids_trimmed(self): '{"ka": {"advisory": ""}}', '{"ka": {"advisory": 42}}', '{"KA": {"advisory": "a"}, "ka": {"advisory": "b"}}', + '{"ka": {"advisory": "Advisory.KA.com"}}', + '{"ka": {"advisory": "advisory ka.com"}}', + '{"ka": {"advisory": ".advisory.ka.com"}}', + '{"ka": {"advisory": "advisory.ka.com."}}', + '{"ka": {"advisory": "advisory..ka.com"}}', ], ) def test_malformed_overrides_are_rejected(self, raw): @@ -177,6 +182,31 @@ def test_id_template_must_name_both_kind_and_instance(self, monkeypatch, templat with pytest.raises(NetworkProviderConfigError, match="must use"): load_provider_naming() + @pytest.mark.unit + @pytest.mark.parametrize( + "template", + [ + "{kind} {instance}.provider", + "{kind}.{instance}.", + ".{kind}.{instance}", + "{kind}..{instance}", + "{kind}.{instance}.Provider.oan.dev", + "{kind}/{instance}", + ], + ) + def test_id_template_must_render_a_valid_id(self, monkeypatch, template): + monkeypatch.setenv("NETWORK_PROVIDER_ID_TEMPLATE", template) + + with pytest.raises(NetworkProviderConfigError, match="provider id"): + load_provider_naming() + + @pytest.mark.unit + @pytest.mark.parametrize("template", ["{kind}-{instance}.provider.oan.dev", "{instance}.{kind}.example-1.net"]) + def test_id_template_with_a_valid_shape_is_accepted(self, monkeypatch, template): + monkeypatch.setenv("NETWORK_PROVIDER_ID_TEMPLATE", template) + + assert load_provider_naming().id_template == template + class TestBootRefusesMalformedProviderMap: @pytest.fixture From fb5d8bc72363063896d26e9d1962dfbfe75e85e6 Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Tue, 20 Oct 2026 14:16:18 +0530 Subject: [PATCH 22/23] fix: refuse an invalid provider id at publish time [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- ENV.md | 2 +- ...ish-to-network-under-per-group-provider.md | 2 +- pipeline/activities.py | 10 +++++--- pipeline/network_group.py | 11 +++------ pipeline/network_provider.py | 15 ++++++++++++ tests/test_activities.py | 23 +++++++++++++++++++ tests/test_network_provider.py | 17 +++++++++++++- 7 files changed, 66 insertions(+), 14 deletions(-) diff --git a/ENV.md b/ENV.md index 2b1aca4..5c261de 100644 --- a/ENV.md +++ b/ENV.md @@ -256,7 +256,7 @@ Each catalog's provider is named per Knowledge Kind and instance: id and descrip | `DISCOVERY_SERVICE_ENDPOINT` | *(empty — required)* | Base URL of the external Discovery Service; the pipeline POSTs to `{DISCOVERY_SERVICE_ENDPOINT}/publish` | | `NETWORK_SENDER_ID` | *(empty — required)* | This instance's Beckn `senderId`, also sent as `networkId` | | `PROVIDER_CONFIG_OVERRIDE_PATH` | unset (bundled `pipeline/network_group_providers.json`, `{}` — no overrides) | Path to a JSON object of instance code → `{kind: provider id}`, set per deployment, e.g. `{"ka": {"advisory": "advisory.ka.com", "scheme": "scheme.ka.com"}}`. Codes and kinds are case-insensitive; kinds are `advisory` or `scheme`. A missing file, invalid JSON, an unknown kind, or an empty or invalid id (same rule as the template) stop the API and worker from starting | -| `NETWORK_PROVIDER_ID_TEMPLATE` | `{kind}.{instance}.provider.oan.dev` | Default provider id and descriptor code, unless overridden in the file above. `{kind}` is `advisory` or `scheme`; `{instance}` is the group code, or `PORTAL_INSTANCE` for documents with no Group. Must use both and render a valid provider id (lowercase letters, digits, `-` and `.`, no leading, trailing or repeated `.`), or the API and worker refuse to start | +| `NETWORK_PROVIDER_ID_TEMPLATE` | `{kind}.{instance}.provider.oan.dev` | Default provider id and descriptor code, unless overridden in the file above. `{kind}` is `advisory` or `scheme`; `{instance}` is the group code, or `PORTAL_INSTANCE` for documents with no Group. Must use both and render a valid provider id (lowercase letters, digits, `-` and `.`, no leading, trailing or repeated `.`), or the API and worker refuse to start. A document whose instance code renders an invalid id fails its publish without retry | | `NETWORK_ADVISORY_PROVIDER_NAME` | `Advisories from documents` | Provider name for advisory catalogs | | `NETWORK_SCHEMES_PROVIDER_NAME` | `Schemes from documents` | Provider name for scheme catalogs | | `NETWORK_GROUP_PROVIDER_NAME_TEMPLATE` | `{group_name} {provider_name}` | Provider and catalog name for a Group, e.g. `Maharashtra Advisories from documents` | diff --git a/docs/ADR/0006-publish-to-network-under-per-group-provider.md b/docs/ADR/0006-publish-to-network-under-per-group-provider.md index 3c87282..c743c44 100644 --- a/docs/ADR/0006-publish-to-network-under-per-group-provider.md +++ b/docs/ADR/0006-publish-to-network-under-per-group-provider.md @@ -12,7 +12,7 @@ Decision: - The group code is derived from the document's `instance`, not from the uploader's group, so a Super Admin uploading for a state publishes under that state. - Documents with no Group (Bharat Vistaar `bv`, `default`) use the portal instance `bv` in the provider id and keep ADR 0003's unprefixed catalog and resource ids. - Every Group publishes; only an unmapped Knowledge Kind skips. A real publish failure stays fatal, per ADR 0004. -- The provider id template, the per-kind provider names and the Group name template are environment settings, so a deployment can change them without a code change. A template with an unknown placeholder, an id template missing `{kind}` or `{instance}`, or a provider id (rendered or overridden) outside lowercase letters, digits, `-` and dot-separated parts stops the API and the worker from booting, since a published id can't be removed later. +- The provider id template, the per-kind provider names and the Group name template are environment settings, so a deployment can change them without a code change. A template with an unknown placeholder, an id template missing `{kind}` or `{instance}`, or a provider id (rendered or overridden) outside lowercase letters, digits, `-` and dot-separated parts stops the API and the worker from booting, since a published id can't be removed later. A document whose instance code still renders an invalid id fails its publish, without retry, before anything is sent. - An instance that wants its own provider id overrides it per Knowledge Kind in a JSON file whose path is `PROVIDER_CONFIG_OVERRIDE_PATH`, e.g. `{"ka": {"advisory": "advisory.ka.com", "scheme": "scheme.ka.com"}}`. Instances and kinds it leaves out keep the template id. It defaults to a bundled empty `pipeline/network_group_providers.json`, so onboarding a Group needs no config. A missing or malformed file stops both the API and the worker from booting, so a bad deploy can't quietly publish under the wrong id. - "No Group" follows the configured `PORTAL_INSTANCE` and `DEFAULT_INSTANCE` values rather than hardcoding `bv` and `default`. - The publish endpoint stays the same for every Group. diff --git a/pipeline/activities.py b/pipeline/activities.py index 1ef09c1..2d30234 100644 --- a/pipeline/activities.py +++ b/pipeline/activities.py @@ -30,6 +30,7 @@ from .document_repository import DocumentRepository from .instances import instance_display_name from .network_constants import RESULT_REJECTED +from .network_provider import InvalidProviderIdError from .ocr import ocr_pdf as run_ocr_pdf from .ocr import ocr_pdf_in_segments as run_ocr_pdf_in_segments from .translation import load_translation_config, translate_pages @@ -1467,9 +1468,12 @@ async def publish_catalog_to_network(workflow_id: str, transaction_id: str) -> d group = repository.get_document_group(workflow_id) service = DiscoveryPublishService() - result = await asyncio.to_thread( - service.publish, transaction_id, document_kind, workflow_id, group - ) + try: + result = await asyncio.to_thread( + service.publish, transaction_id, document_kind, workflow_id, group + ) + except InvalidProviderIdError as exc: + raise ApplicationError(str(exc), non_retryable=True) from exc if result["skipped"]: artifact_type = "network_publish_skipped" diff --git a/pipeline/network_group.py b/pipeline/network_group.py index 17d2c3b..d37a5cd 100644 --- a/pipeline/network_group.py +++ b/pipeline/network_group.py @@ -2,7 +2,6 @@ import json import os -import re import string from pathlib import Path from typing import Optional @@ -14,7 +13,7 @@ PROVIDER_ID_TEMPLATE, SCHEMES_PROVIDER_NAME, ) -from .network_provider import PROVIDER_KINDS, ProviderNaming +from .network_provider import PROVIDER_ID_RULE, PROVIDER_KINDS, ProviderNaming, is_valid_provider_id PROVIDER_CONFIG_OVERRIDE_PATH_ENV = "PROVIDER_CONFIG_OVERRIDE_PATH" DEFAULT_PROVIDER_CONFIG_OVERRIDE_PATH = Path(__file__).with_name("network_group_providers.json") @@ -22,7 +21,6 @@ GROUP_PROVIDER_NAME_TEMPLATE_ENV = "NETWORK_GROUP_PROVIDER_NAME_TEMPLATE" ADVISORY_PROVIDER_NAME_ENV = "NETWORK_ADVISORY_PROVIDER_NAME" SCHEMES_PROVIDER_NAME_ENV = "NETWORK_SCHEMES_PROVIDER_NAME" -PROVIDER_ID_PATTERN = re.compile(r"[a-z0-9-]+(?:\.[a-z0-9-]+)*") class NetworkProviderConfigError(ValueError): @@ -34,11 +32,8 @@ def _env(name: str, default: str) -> str: def _check_provider_id(provider_id: str, source: str) -> None: - if not PROVIDER_ID_PATTERN.fullmatch(provider_id): - raise NetworkProviderConfigError( - f"{source} gives provider id {provider_id!r}; it must be lowercase letters, digits, " - "'-' and '.', with no leading, trailing or repeated '.'" - ) + if not is_valid_provider_id(provider_id): + raise NetworkProviderConfigError(f"{source} gives provider id {provider_id!r}; it must be {PROVIDER_ID_RULE}") def parse_provider_id_overrides(raw: Optional[str], source: str) -> dict[str, dict[str, str]]: diff --git a/pipeline/network_provider.py b/pipeline/network_provider.py index d1d1b84..491aa78 100644 --- a/pipeline/network_provider.py +++ b/pipeline/network_provider.py @@ -1,3 +1,4 @@ +import re from dataclasses import dataclass, field from typing import Mapping, Optional @@ -9,6 +10,16 @@ ) PROVIDER_KINDS = ("advisory", "scheme") +PROVIDER_ID_PATTERN = re.compile(r"[a-z0-9-]+(?:\.[a-z0-9-]+)*") +PROVIDER_ID_RULE = "lowercase letters, digits, '-' and '.', with no leading, trailing or repeated '.'" + + +class InvalidProviderIdError(ValueError): + pass + + +def is_valid_provider_id(provider_id: str) -> bool: + return PROVIDER_ID_PATTERN.fullmatch(provider_id) is not None @dataclass(frozen=True) @@ -35,6 +46,10 @@ def provider(self, kind: str, group: Optional[Group]) -> dict: provider_id = self.provider_id_overrides.get(instance, {}).get(kind) or self.id_template.format( kind=kind, instance=instance ) + if not is_valid_provider_id(provider_id): + raise InvalidProviderIdError( + f"Provider id {provider_id!r} for {kind} on instance {instance!r} must be {PROVIDER_ID_RULE}" + ) name = self.provider_names[kind] if group is not None: name = self.group_name_template.format(group_name=group.name, provider_name=name) diff --git a/tests/test_activities.py b/tests/test_activities.py index 37de1c6..eb06734 100644 --- a/tests/test_activities.py +++ b/tests/test_activities.py @@ -785,6 +785,29 @@ async def test_unmapped_kind_records_a_skip(self, monkeypatch): assert recorded["metadata"]["reason"] == "no network catalog is mapped to this document kind" + @pytest.mark.unit + @pytest.mark.asyncio + async def test_an_invalid_provider_id_fails_without_retry_or_artifact(self, monkeypatch): + import pipeline.activities as activities + import pipeline.db as db + from pipeline.network_provider import InvalidProviderIdError + + class FakeService: + def publish(self, transaction_id, document_kind, workflow_id=None, group=None): + raise InvalidProviderIdError("Provider id 'advisory.ladakh_ut.provider.oan.dev' is invalid") + + monkeypatch.setattr(activities, "DiscoveryPublishService", FakeService) + monkeypatch.setattr(db, "get_document", lambda workflow_id: {"document_kind": "advisory", "instance": "ladakh_ut"}) + recorded = {} + monkeypatch.setattr(db, "add_document_artifact", lambda **kwargs: recorded.update(kwargs)) + + with pytest.raises(ApplicationError, match="ladakh_ut") as excinfo: + await activities.publish_catalog_to_network("wf-x", "txn-x") + + assert excinfo.value.non_retryable is True + assert recorded == {} + + class TestWorkerActivityRegistration: """ Every @activity.defn in activities.py must be registered on the worker diff --git a/tests/test_network_provider.py b/tests/test_network_provider.py index 7da182c..fb66840 100644 --- a/tests/test_network_provider.py +++ b/tests/test_network_provider.py @@ -1,6 +1,6 @@ import pytest -from pipeline.network_provider import Group, ProviderNaming +from pipeline.network_provider import Group, InvalidProviderIdError, ProviderNaming MAHARASHTRA = Group(code="mh", name="Maharashtra") @@ -90,3 +90,18 @@ def test_group_without_overrides_uses_the_defaults(self): @pytest.mark.unit def test_shared_instance_can_be_overridden(self): assert self.NAMING.provider("advisory", None)["id"] == "advisory.bharat.example" + + +class TestInvalidProviderIdAtPublish: + @pytest.mark.unit + @pytest.mark.parametrize("code", ["ladakh_ut", "u p", "MH."]) + def test_an_instance_that_renders_an_invalid_id_is_refused(self, code): + with pytest.raises(InvalidProviderIdError, match="must be lowercase"): + ProviderNaming(shared_instance="bv").provider("advisory", Group(code=code, name="Somewhere")) + + @pytest.mark.unit + def test_an_invalid_override_is_refused(self): + naming = ProviderNaming(shared_instance="bv", provider_id_overrides={"ka": {"scheme": "Scheme.KA"}}) + + with pytest.raises(InvalidProviderIdError): + naming.provider("scheme", Group(code="ka", name="Karnataka")) From c0c96367a40226188c70f719d64c8aeecf9cf49f Mon Sep 17 00:00:00 2001 From: Hariharan Thiagarajan Date: Tue, 20 Oct 2026 14:54:18 +0530 Subject: [PATCH 23/23] fix: require group_name in the group provider name template [#190] Co-Authored-By: Claude Opus 5.5 (1M context) --- ENV.md | 2 +- ...-publish-to-network-under-per-group-provider.md | 2 +- pipeline/network_group.py | 1 + tests/test_network_group.py | 14 ++++++++++++++ 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/ENV.md b/ENV.md index 5c261de..0ec38ff 100644 --- a/ENV.md +++ b/ENV.md @@ -259,7 +259,7 @@ Each catalog's provider is named per Knowledge Kind and instance: id and descrip | `NETWORK_PROVIDER_ID_TEMPLATE` | `{kind}.{instance}.provider.oan.dev` | Default provider id and descriptor code, unless overridden in the file above. `{kind}` is `advisory` or `scheme`; `{instance}` is the group code, or `PORTAL_INSTANCE` for documents with no Group. Must use both and render a valid provider id (lowercase letters, digits, `-` and `.`, no leading, trailing or repeated `.`), or the API and worker refuse to start. A document whose instance code renders an invalid id fails its publish without retry | | `NETWORK_ADVISORY_PROVIDER_NAME` | `Advisories from documents` | Provider name for advisory catalogs | | `NETWORK_SCHEMES_PROVIDER_NAME` | `Schemes from documents` | Provider name for scheme catalogs | -| `NETWORK_GROUP_PROVIDER_NAME_TEMPLATE` | `{group_name} {provider_name}` | Provider and catalog name for a Group, e.g. `Maharashtra Advisories from documents` | +| `NETWORK_GROUP_PROVIDER_NAME_TEMPLATE` | `{group_name} {provider_name}` | Provider and catalog name for a Group, e.g. `Maharashtra Advisories from documents`. Must use `{group_name}`, or the API and worker refuse to start | | `DISCOVERY_SERVICE_PUBLISH_INITIAL_INTERVAL_SECONDS` | `30` | Wait before the first retry of a failed publish | | `DISCOVERY_SERVICE_PUBLISH_BACKOFF_COEFFICIENT` | `2.0` | Multiplier applied to the wait after each retry | | `DISCOVERY_SERVICE_PUBLISH_MAX_INTERVAL_SECONDS` | `300` | Cap on the wait between retries | diff --git a/docs/ADR/0006-publish-to-network-under-per-group-provider.md b/docs/ADR/0006-publish-to-network-under-per-group-provider.md index c743c44..5d9b0aa 100644 --- a/docs/ADR/0006-publish-to-network-under-per-group-provider.md +++ b/docs/ADR/0006-publish-to-network-under-per-group-provider.md @@ -12,7 +12,7 @@ Decision: - The group code is derived from the document's `instance`, not from the uploader's group, so a Super Admin uploading for a state publishes under that state. - Documents with no Group (Bharat Vistaar `bv`, `default`) use the portal instance `bv` in the provider id and keep ADR 0003's unprefixed catalog and resource ids. - Every Group publishes; only an unmapped Knowledge Kind skips. A real publish failure stays fatal, per ADR 0004. -- The provider id template, the per-kind provider names and the Group name template are environment settings, so a deployment can change them without a code change. A template with an unknown placeholder, an id template missing `{kind}` or `{instance}`, or a provider id (rendered or overridden) outside lowercase letters, digits, `-` and dot-separated parts stops the API and the worker from booting, since a published id can't be removed later. A document whose instance code still renders an invalid id fails its publish, without retry, before anything is sent. +- The provider id template, the per-kind provider names and the Group name template are environment settings, so a deployment can change them without a code change. A template with an unknown placeholder, an id template missing `{kind}` or `{instance}`, a Group name template missing `{group_name}`, or a provider id (rendered or overridden) outside lowercase letters, digits, `-` and dot-separated parts stops the API and the worker from booting, since a published id can't be removed later. A document whose instance code still renders an invalid id fails its publish, without retry, before anything is sent. - An instance that wants its own provider id overrides it per Knowledge Kind in a JSON file whose path is `PROVIDER_CONFIG_OVERRIDE_PATH`, e.g. `{"ka": {"advisory": "advisory.ka.com", "scheme": "scheme.ka.com"}}`. Instances and kinds it leaves out keep the template id. It defaults to a bundled empty `pipeline/network_group_providers.json`, so onboarding a Group needs no config. A missing or malformed file stops both the API and the worker from booting, so a bad deploy can't quietly publish under the wrong id. - "No Group" follows the configured `PORTAL_INSTANCE` and `DEFAULT_INSTANCE` values rather than hardcoding `bv` and `default`. - The publish endpoint stays the same for every Group. diff --git a/pipeline/network_group.py b/pipeline/network_group.py index d37a5cd..f3464f5 100644 --- a/pipeline/network_group.py +++ b/pipeline/network_group.py @@ -136,6 +136,7 @@ def load_provider_naming() -> ProviderNaming: group_name_template=_checked_template( GROUP_PROVIDER_NAME_TEMPLATE_ENV, GROUP_PROVIDER_NAME_TEMPLATE, + frozenset({"group_name"}), group_name="Maharashtra", provider_name="Advisories from documents", ), diff --git a/tests/test_network_group.py b/tests/test_network_group.py index bb31eef..77f6da8 100644 --- a/tests/test_network_group.py +++ b/tests/test_network_group.py @@ -182,6 +182,20 @@ def test_id_template_must_name_both_kind_and_instance(self, monkeypatch, templat with pytest.raises(NetworkProviderConfigError, match="must use"): load_provider_naming() + @pytest.mark.unit + @pytest.mark.parametrize("template", ["{provider_name}", "Knowledge from documents"]) + def test_group_name_template_must_name_the_group(self, monkeypatch, template): + monkeypatch.setenv("NETWORK_GROUP_PROVIDER_NAME_TEMPLATE", template) + + with pytest.raises(NetworkProviderConfigError, match="must use"): + load_provider_naming() + + @pytest.mark.unit + def test_group_name_template_without_the_provider_name_is_accepted(self, monkeypatch): + monkeypatch.setenv("NETWORK_GROUP_PROVIDER_NAME_TEMPLATE", "{group_name} knowledge") + + assert load_provider_naming().group_name_template == "{group_name} knowledge" + @pytest.mark.unit @pytest.mark.parametrize( "template",