From 3d5bc38401a05446ddd6b8b4636249378dc07b44 Mon Sep 17 00:00:00 2001 From: Chaim Date: Tue, 21 Apr 2026 16:12:32 +0000 Subject: [PATCH] feat(kb): per-document metadata via sidecar JSON Each file in the inbox can now be accompanied by a `.meta.json` sidecar that supplies identifier, published_at, effective_at, and source_url. The scanner applies filename-derived defaults first and lets the sidecar override individual fields. - list_inbox / list_failed skip `.meta.json` so sidecars aren't ingested as standalone documents. - mark_processed, mark_failed, and requeue-failed move the sidecar alongside its parent (best-effort). - mark_failed now sanitizes the error string to ASCII before writing it to S3 object metadata (HTTP header encoding). - search SQL selects published_at/effective_at; the formatter shows the full heading_path plus publication date so citations are unambiguous. - scripts/kb_sidecar_template.json documents the expected shape. Refs Task Master #2 Co-Authored-By: Claude Opus 4.7 (1M context) --- api/routes/admin_kb.py | 50 +++++++++++++++++++++++++++--- api/services/kb/s3.py | 32 ++++++++++++++++--- api/services/kb/search.py | 1 + mcp_server/tools/legal_kb_tools.py | 14 +++++++-- scripts/kb_sidecar_template.json | 7 +++++ 5 files changed, 93 insertions(+), 11 deletions(-) create mode 100644 scripts/kb_sidecar_template.json diff --git a/api/routes/admin_kb.py b/api/routes/admin_kb.py index 0063668..c484d64 100644 --- a/api/routes/admin_kb.py +++ b/api/routes/admin_kb.py @@ -80,13 +80,45 @@ def _derive_metadata(filename: str, kind: str) -> dict: if stem.lower().endswith(ext): stem = stem[: -len(ext)] break - # Title is just the stem; identifier left empty unless obvious. - return {"title": stem, "identifier": None} + return { + "title": stem, + "identifier": None, + "published_at": None, + "effective_at": None, + "source_url": None, + } + + +def _sidecar_metadata(item_key: str) -> dict: + """Fetch the optional `.meta.json` sidecar and return parsed fields. + + The sidecar is expected to be a JSON object with any of: + {title, identifier, published_at, effective_at, source_url} + + Missing or unreadable sidecars return {}. + """ + import json + sidecar_key = item_key + ".meta.json" + try: + data = kb_s3.fetch(sidecar_key) + except Exception: + return {} + try: + parsed = json.loads(data.decode("utf-8")) + except Exception as e: + logger.warning("[admin.kb] bad sidecar %s: %s", sidecar_key, e) + return {} + # Accept only whitelisted keys — ignore typos / extra fields. + allowed = {"title", "identifier", "published_at", "effective_at", "source_url"} + return {k: v for k, v in parsed.items() if k in allowed and v} @router.post("/requeue-failed") async def requeue_failed(request: Request): - """Move everything under failed// back to inbox// for retry.""" + """Move everything under failed// back to inbox// for retry. + + The sidecar `.meta.json` (if present) is moved alongside. + """ _verify_admin(request) items = kb_s3.list_failed() moved = [] @@ -95,6 +127,11 @@ async def requeue_failed(request: Request): dst = f"inbox/{item['kind']}/{item['filename']}" try: kb_s3.move(src, dst) + # Best-effort sidecar move. + try: + kb_s3.move(src + ".meta.json", dst + ".meta.json") + except Exception: + pass moved.append({"from": src, "to": dst}) except Exception as e: moved.append({"from": src, "error": str(e)}) @@ -110,15 +147,20 @@ async def scan_inbox(request: Request): src_key = item["key"] kind = item["kind"] filename = item["filename"] + # Merge: filename-derived defaults < sidecar overrides. meta = _derive_metadata(filename, kind) + meta.update(_sidecar_metadata(src_key)) try: data = kb_s3.fetch(src_key) result = await kb_ingest.ingest_source( kind=kind, title=meta["title"], - identifier=meta["identifier"], + identifier=meta.get("identifier"), content=data, filename=filename, + published_at=meta.get("published_at"), + effective_at=meta.get("effective_at"), + source_url=meta.get("source_url"), original_path=f"s3://{kb_s3._bucket()}/processed/{kind}/{filename}", ) dst = kb_s3.mark_processed(src_key, filename, kind) diff --git a/api/services/kb/s3.py b/api/services/kb/s3.py index c1f54e2..1906ab7 100644 --- a/api/services/kb/s3.py +++ b/api/services/kb/s3.py @@ -45,7 +45,12 @@ def _bucket() -> str: def list_inbox() -> list[dict]: - """Return inbox objects across all kinds, excluding folder markers.""" + """Return inbox objects across all kinds, excluding folder markers and sidecars. + + Sidecar files (.meta.json) carry per-document metadata and are + not standalone documents, so they are filtered out here — the scanner + fetches them separately when processing their parent. + """ s3 = _client() bucket = _bucket() items: list[dict] = [] @@ -55,9 +60,10 @@ def list_inbox() -> list[dict]: for page in paginator.paginate(Bucket=bucket, Prefix=prefix): for obj in page.get("Contents", []) or []: key = obj["Key"] - # Skip folder markers (MinIO creates zero-byte objects with trailing /) if key.endswith("/"): continue + if key.endswith(".meta.json"): + continue filename = key[len(prefix):] if not filename: continue @@ -92,6 +98,13 @@ def move(src_key: str, dst_key: str) -> None: def mark_processed(src_key: str, filename: str, kind: str) -> str: dst = f"processed/{kind}/{filename}" move(src_key, dst) + # Best-effort: move the sidecar too if it exists. + sidecar_src = src_key + ".meta.json" + sidecar_dst = dst + ".meta.json" + try: + move(sidecar_src, sidecar_dst) + except Exception: + pass return dst @@ -107,6 +120,8 @@ def list_failed() -> list[dict]: key = obj["Key"] if key.endswith("/"): continue + if key.endswith(".meta.json"): + continue filename = key[len(prefix):] if not filename: continue @@ -118,15 +133,24 @@ def mark_failed(src_key: str, filename: str, kind: str, reason: str) -> str: s3 = _client() bucket = _bucket() dst = f"failed/{kind}/{filename}" + # S3 metadata is encoded as HTTP headers, so only ASCII is safe. + safe_reason = reason.encode("ascii", "ignore").decode("ascii")[:800] s3.copy_object( Bucket=bucket, Key=dst, CopySource={"Bucket": bucket, "Key": src_key}, - Metadata={"ingest_error": reason[:800]}, + Metadata={"ingest_error": safe_reason}, MetadataDirective="REPLACE", ) s3.delete_object(Bucket=bucket, Key=src_key) - logger.info("[kb.s3] failed %s -> %s (%s)", src_key, dst, reason[:80]) + # Best-effort: move the sidecar too. + sidecar_src = src_key + ".meta.json" + sidecar_dst = dst + ".meta.json" + try: + move(sidecar_src, sidecar_dst) + except Exception: + pass + logger.info("[kb.s3] failed %s -> %s (%s)", src_key, dst, safe_reason[:80]) return dst diff --git a/api/services/kb/search.py b/api/services/kb/search.py index 53fc2ef..c456ae1 100644 --- a/api/services/kb/search.py +++ b/api/services/kb/search.py @@ -63,6 +63,7 @@ async def search( ) SELECT s.kind, s.title, s.identifier, s.source_url, + s.published_at, s.effective_at, c.heading_path, c.section_ref, c.content, f.score FROM fused f diff --git a/mcp_server/tools/legal_kb_tools.py b/mcp_server/tools/legal_kb_tools.py index 519af55..2da7dad 100644 --- a/mcp_server/tools/legal_kb_tools.py +++ b/mcp_server/tools/legal_kb_tools.py @@ -24,10 +24,18 @@ def _format_hits(hits: list[dict]) -> str: for i, h in enumerate(hits, 1): kind_he = _KIND_HEBREW.get(h["kind"], h["kind"]) ident = f" {h['identifier']}" if h.get("identifier") else "" - ref = h.get("section_ref") or h.get("heading_path") or "" header = f"{i}. [{kind_he}{ident}] {h['title']}" - if ref: - header += f" — {ref}" + + # Prefer the full hierarchical path when present; fall back to section_ref. + path = h.get("heading_path") or h.get("section_ref") or "" + if path: + header += f" — {path}" + + # Date annotation for time-sensitive citations. + pub = h.get("published_at") + if pub: + header += f" (פורסם {pub})" + lines.append(header) content = (h.get("content") or "").strip() if len(content) > 900: diff --git a/scripts/kb_sidecar_template.json b/scripts/kb_sidecar_template.json new file mode 100644 index 0000000..58d8ac5 --- /dev/null +++ b/scripts/kb_sidecar_template.json @@ -0,0 +1,7 @@ +{ + "title": "תקנה 37 — בדיקה מחדש", + "identifier": "127/2022", + "published_at": "2022-04-12", + "effective_at": "2022-04-12", + "source_url": "https://www.btl.gov.il/Laws1/00_0_127-2022.pdf" +}