Rifqi Hafizuddin Claude Opus 5 commited on
Commit
67af818
Β·
1 Parent(s): 48e558b

[NOTICKET] feat(knowledge): seam version gate (N6b) + ingest auth posture (N11)

Browse files

N6b - SEAM VERSION GATE. `parsed_doc_from_artifact` read the artifact envelope
and ignored `schema_version` entirely, so a shape this build cannot read would
be parsed anyway on a best-effort basis. That is the one failure worth being
loud about: a shape we do not understand is not a smaller version of the right
input, it is a different input, and reading it produces entries that look fine
and are wrong.

Rules: major mismatch rejects; a NEWER minor rejects (structure this code has
never seen); an older minor or any patch is accepted, because minor bumps have
been additive by convention (0.3.0 -> 0.3.3 added source_title,
normaliser_version, page_no) and the committed eval fixture is deliberately an
older minor than the current parser - rejecting it would break the baseline
measurement for no safety gain. An absent version warns and proceeds, so
pre-envelope fixtures and bare chunk lists stay loadable.

SUPPORTED_SCHEMA is a literal, not an import from knowledge_parsing:
extraction must not depend on the parsing package at runtime - that
independence is the seam's whole point. Drift is caught instead by a local
seam test that imports knowledge_parsing.contracts directly and asserts the
two agree, so a bump on either side fails in tests rather than at read time.
Verified all three committed fixtures still load (0.3.0 and 0.3.3).

N11 - INGEST AUTH POSTURE, decided rather than inherited. The service-secret
gate cannot be armed here for the same reason it was unwired in #37: the browser
SPA is the sole caller, we do not own that repo, and a secret shipped to a
browser is not a secret - arming it would 401 the feature. But this is a WRITE
that spends LLM budget, a different risk class from the read surface, so the
posture is bounded by four controls rather than inherited silently:

1. rate limit, 5/minute - shipped here, verified firing (5x202 then 2x429).
Deliberately far below chat's 30/minute: a legitimate caller clicks this a
few times a year, not a few times a minute.
2. single-flight per scope - already shipped.
3. pre-flight spend ceiling ) both land with the pipeline (N10c)
4. content-hash idempotency )

What they do NOT do is identify the caller: user_id is still whatever the
request says, so one user can spend budget under another's id. That closes
only with a forwarded identity (#43). Stated in the router docstring so the
next reader knows it is an accepted posture, not an oversight.

Verification: ruff clean on touched paths; `import main` OK; suite 570 passed /
7 skipped (555 + 15 new local seam-gate tests; tests/ stays gitignored).

N7b (torch CPU pin) is NOT in this commit - it is blocked, see the report.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

src/api/v1/knowledge.py CHANGED
@@ -18,13 +18,27 @@ difference is real: charts is called unconditionally on every turn where absence
18
  is normal, whereas a polled `job_id` was handed to the caller, so its absence is
19
  genuinely exceptional.
20
 
21
- ⚠️ **No auth.** `user_id` is caller-supplied, exactly like every other live
22
- endpoint (DEV_PLAN Β§0.7 #37 β€” the browser SPA is the sole caller and cannot send
23
- a service secret). This route is a *write* that spends LLM budget, which is a
24
- different risk class from the read surface, and the posture is an open decision
25
- (KNOWLEDGE_SURFACE_PLAN.md Β§6). The bounded controls β€” single-flight, the
26
- pre-flight spend ceiling and content-addressed idempotency β€” limit the damage
27
- but do not identify the caller.
 
 
 
 
 
 
 
 
 
 
 
 
 
 
28
 
29
  `scope_id` is deliberately NOT accepted from the caller: it is derived
30
  server-side from `users.company`. Accepting a tenant key on an unauthenticated
@@ -33,12 +47,13 @@ route is a cross-tenant write with extra steps.
33
 
34
  import uuid
35
 
36
- from fastapi import APIRouter, HTTPException, Query, status
37
  from pydantic import BaseModel, Field, field_validator
38
 
39
  from src.knowledge_ingest.models import IngestJob
40
  from src.knowledge_ingest.service import IngestService
41
  from src.middlewares.logging import get_logger, log_execution
 
42
 
43
  # NOTE: this module must NOT use `from __future__ import annotations`.
44
  # With it, FastAPI cannot resolve a Pydantic body model's annotation through
@@ -111,8 +126,15 @@ class IngestAccepted(BaseModel):
111
  status_code=status.HTTP_202_ACCEPTED,
112
  summary="Extract knowledge from a user's documents (async batch)",
113
  )
 
 
 
 
 
 
 
114
  @log_execution(logger)
115
- async def ingest(payload: IngestRequest) -> IngestAccepted:
116
  """Admit a batch and return immediately. Poll `GET /jobs/{job_id}`."""
117
  job, reused = await _service.start(
118
  user_id=payload.user_id, document_ids=payload.document_ids
 
18
  is normal, whereas a polled `job_id` was handed to the caller, so its absence is
19
  genuinely exceptional.
20
 
21
+ ⚠️ **No caller authentication β€” decided 2026-09-07, not inherited.** `user_id` is
22
+ caller-supplied, exactly like every other live endpoint. The service-secret gate
23
+ (DEV_PLAN Β§0.7 #37) cannot be armed here for the same reason it was unwired
24
+ there: the browser SPA is the sole caller, we do not own that repo, and a secret
25
+ shipped to a browser is not a secret. Arming it would 401 the feature.
26
+
27
+ But this is a **write that spends LLM budget**, which is a different risk class
28
+ from the read surface, so the posture is not inherited silently β€” it is bounded
29
+ by four controls, and the gap they leave is stated rather than papered over:
30
+
31
+ 1. **Rate limit** (below) β€” caps calls per client per window.
32
+ 2. **Single-flight per scope** β€” a second click returns the running job.
33
+ 3. **Pre-flight spend ceiling** β€” the free stages produce an exact estimate and
34
+ the job stops rather than spending past it (N10c, lands with the pipeline).
35
+ 4. **Content-addressed idempotency** β€” re-ingesting an unchanged document
36
+ re-spends nothing (N10c).
37
+
38
+ **What they do not do is identify the caller.** `user_id` is still whatever the
39
+ request says, so one user can spend budget under another's id. That closes only
40
+ with a real forwarded identity (DEV_PLAN #43). Recorded here so the next reader
41
+ knows this is a bounded, accepted posture and not an oversight.
42
 
43
  `scope_id` is deliberately NOT accepted from the caller: it is derived
44
  server-side from `users.company`. Accepting a tenant key on an unauthenticated
 
47
 
48
  import uuid
49
 
50
+ from fastapi import APIRouter, HTTPException, Query, Request, status
51
  from pydantic import BaseModel, Field, field_validator
52
 
53
  from src.knowledge_ingest.models import IngestJob
54
  from src.knowledge_ingest.service import IngestService
55
  from src.middlewares.logging import get_logger, log_execution
56
+ from src.middlewares.rate_limit import limiter
57
 
58
  # NOTE: this module must NOT use `from __future__ import annotations`.
59
  # With it, FastAPI cannot resolve a Pydantic body model's annotation through
 
126
  status_code=status.HTTP_202_ACCEPTED,
127
  summary="Extract knowledge from a user's documents (async batch)",
128
  )
129
+ # N11, control 1 of 4. Deliberately far below the chat route's 30/minute: this
130
+ # starts GPU-less but LLM-spending batch work, and a legitimate caller clicks it
131
+ # a few times a year, not a few times a minute. `slowapi` needs a Starlette
132
+ # `Request` param named `request`, which is why the body is `payload`.
133
+ # NOTE: `get_remote_address` buckets by client IP β€” if calls ever arrive through
134
+ # a proxy, every caller shares one bucket. Revisit when identity is forwarded.
135
+ @limiter.limit("5/minute")
136
  @log_execution(logger)
137
+ async def ingest(request: Request, payload: IngestRequest) -> IngestAccepted:
138
  """Admit a batch and return immediately. Poll `GET /jobs/{job_id}`."""
139
  job, reused = await _service.start(
140
  user_id=payload.user_id, document_ids=payload.document_ids
src/knowledge_extraction/adapter.py CHANGED
@@ -24,8 +24,12 @@ import hashlib
24
  import json
25
  from typing import Any
26
 
 
 
27
  from .models import Chunk, ParsedDoc
28
 
 
 
29
  # Field names accepted for the same concept. `page_idx`/`page_idxs` are the
30
  # parsing half's contract; the rest are earlier drafts and the prototype's shape,
31
  # kept so old fixtures still load.
@@ -39,6 +43,84 @@ _PAGES_KEYS = ("page_idxs", "pages", "page_list")
39
  _PAGE_END_KEYS = ("page_idx_end", "page_end")
40
 
41
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
42
  def chunk_from_dict(raw: dict[str, Any], doc_id: str, ordinal: int = 0) -> Chunk:
43
  """Map one artifact item onto the internal chunk.
44
 
@@ -106,6 +188,8 @@ def parsed_doc_from_artifact(
106
  backend = artifact.get("parser_backend") or ""
107
  if backend:
108
  parser_version = f"{parser_version}/{backend}" if parser_version else backend
 
 
109
  declared_hash = artifact.get("content_hash")
110
  declared_pages = artifact.get("n_pages")
111
  else:
 
24
  import json
25
  from typing import Any
26
 
27
+ from src.middlewares.logging import get_logger
28
+
29
  from .models import Chunk, ParsedDoc
30
 
31
+ logger = get_logger("knowledge_adapter")
32
+
33
  # Field names accepted for the same concept. `page_idx`/`page_idxs` are the
34
  # parsing half's contract; the rest are earlier drafts and the prototype's shape,
35
  # kept so old fixtures still load.
 
43
  _PAGE_END_KEYS = ("page_idx_end", "page_end")
44
 
45
 
46
+ # ── Seam version gate (N6b) ──────────────────────────────────────────────
47
+ #
48
+ # The artifact's SHAPE version, as `major.minor`. Parsing owns the contract and
49
+ # bumps `SCHEMA_VERSION` in `knowledge_parsing/contracts.py`; this is the range
50
+ # extraction knows how to read.
51
+ #
52
+ # Deliberately a literal rather than an import: extraction must never depend on
53
+ # the parsing package at runtime β€” that independence is the whole point of the
54
+ # seam. Drift is caught instead by a local seam test that imports
55
+ # `knowledge_parsing.contracts` directly and asserts the two agree, so a bump on
56
+ # either side fails loudly in tests rather than silently at read time.
57
+ SUPPORTED_SCHEMA = (0, 3)
58
+
59
+
60
+ class IncompatibleArtifactError(ValueError):
61
+ """The artifact declares a shape this code cannot read.
62
+
63
+ Raised rather than degraded on purpose. Every other seam in this repo
64
+ tolerates missing structure, because a chunk without a heading is still
65
+ usable β€” but a shape we do not understand is not a smaller version of the
66
+ right input, it is a different input. Reading it anyway produces entries
67
+ that look fine and are wrong, which is the single most expensive failure
68
+ this pipeline can have.
69
+ """
70
+
71
+
72
+ def _parse_version(declared: str) -> tuple[int, int] | None:
73
+ parts = str(declared).split(".")
74
+ try:
75
+ return int(parts[0]), int(parts[1])
76
+ except (IndexError, ValueError):
77
+ return None
78
+
79
+
80
+ def check_schema_version(declared: Any) -> None:
81
+ """Gate one artifact's declared `schema_version`. Raises or returns None.
82
+
83
+ - **Major mismatch β†’ reject.** A major bump is by definition breaking.
84
+ - **Newer minor β†’ reject.** The artifact carries structure this code has
85
+ never seen; guessing at it is how a silent misread starts.
86
+ - **Older minor, or any patch β†’ accept.** Minor bumps have been additive by
87
+ convention (0.3.0 β†’ 0.3.3 added `source_title`, `normaliser_version` and
88
+ `page_no`), and the committed eval fixture is deliberately an older minor
89
+ than the current parser, so rejecting those would break the baseline
90
+ measurement for no safety gain.
91
+ - **Absent β†’ accept with a warning.** Bare chunk lists and pre-envelope
92
+ fixtures carry no version; the CLI already refuses unlabelled artifacts
93
+ where labelling matters.
94
+ """
95
+ if declared in (None, ""):
96
+ logger.warning(
97
+ "artifact_schema_version_missing",
98
+ extra={"supported": ".".join(map(str, SUPPORTED_SCHEMA))},
99
+ )
100
+ return
101
+
102
+ parsed = _parse_version(declared)
103
+ if parsed is None:
104
+ raise IncompatibleArtifactError(
105
+ f"artifact schema_version {declared!r} is not a major.minor version"
106
+ )
107
+
108
+ major, minor = parsed
109
+ supported_major, supported_minor = SUPPORTED_SCHEMA
110
+ if major != supported_major or minor > supported_minor:
111
+ raise IncompatibleArtifactError(
112
+ f"artifact schema_version {declared} is not readable by this "
113
+ f"extraction build (supports {supported_major}.{supported_minor}.x). "
114
+ "Re-parse the document, or update the adapter to the new shape."
115
+ )
116
+
117
+ if minor < supported_minor:
118
+ logger.info(
119
+ "artifact_schema_version_older",
120
+ extra={"declared": str(declared), "supported": f"{supported_major}.{supported_minor}"},
121
+ )
122
+
123
+
124
  def chunk_from_dict(raw: dict[str, Any], doc_id: str, ordinal: int = 0) -> Chunk:
125
  """Map one artifact item onto the internal chunk.
126
 
 
188
  backend = artifact.get("parser_backend") or ""
189
  if backend:
190
  parser_version = f"{parser_version}/{backend}" if parser_version else backend
191
+ # Gate the shape BEFORE reading any of it (N6b).
192
+ check_schema_version(artifact.get("schema_version"))
193
  declared_hash = artifact.get("content_hash")
194
  declared_pages = artifact.get("n_pages")
195
  else: