Skip to content

Commit db7dabc

Browse files
feat(extensions): add OperationValidator.precheck pre-body-parse hook (#1548)
Add an optional ``precheck`` method to ``OperationValidatorExtension`` that extensions can override to gate a request *before* its body is read off the wire. Wire it as a FastAPI ``Depends`` ahead of the body parameter on the billable POST routes (retain, recall, reflect, file retain, mental-model create, mental-model refresh) so a rejecting precheck short-circuits the request without ever materialising the JSON payload in memory. The post-body-parse ``validate_retain`` / ``validate_recall`` / ``validate_reflect`` hooks are unchanged and remain the source of truth for precise per-call cost and quota arithmetic. ``precheck`` is intentionally a cheap, side-effect-free check — its sole purpose is to let an extension short-circuit work that would otherwise allocate the request body unnecessarily (e.g. a quota-exhausted caller submitting many large bodies). Why before body parse: FastAPI resolves dependencies before deserialising the route's body parameter. A validator that runs only after parse — i.e. inside the route handler's body — sees the already-materialised request, which is the wrong layer for "this caller should not be allowed to spend resources on this request at all" decisions. Wiring as ``Depends`` puts the gate at the right layer with a one-line change per route. Verified: - FastAPI 0.125.0 resolves ``Depends`` raising ``HTTPException`` before Pydantic deserialises the body, regardless of declaration order. A reproducer using a ``model_validator(mode='before')`` recorder confirms zero body-parse calls on the rejection path. - The new ``PrecheckContext`` carries only operation name + bank_id + request_context (already-resolved tenant). No body access — by design. - Default ``precheck`` returns ``ValidationResult.accept()``; existing validators are unaffected. Tests: +7 unit tests covering the default no-op, the FastAPI Depends wiring, accept/reject paths, status-code/reason propagation, and explicit "body never parsed on rejection" assertions for retain / recall / reflect plus a "GET routes are unaffected" guard. All passing.
1 parent b83bb87 commit db7dabc

4 files changed

Lines changed: 393 additions & 3 deletions

File tree

hindsight-api-slim/hindsight_api/api/http.py

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2892,6 +2892,57 @@ def get_request_context(authorization: str | None = Header(default=None)) -> Req
28922892
api_key = authorization.strip()
28932893
return RequestContext(api_key=api_key)
28942894

2895+
def precheck_for(operation: str):
2896+
"""
2897+
Build a FastAPI dependency that runs ``OperationValidator.precheck``.
2898+
2899+
FastAPI resolves dependencies before deserialising the route's body
2900+
parameter. Wiring this dependency on the billable POST routes lets
2901+
an extension reject a request — e.g. with HTTP 402 when a tenant's
2902+
balance is exhausted — without the request body ever being read or
2903+
materialised in memory.
2904+
2905+
The dependency intentionally:
2906+
- authenticates the tenant (so ``request_context.tenant_id`` is
2907+
resolved before the precheck runs);
2908+
- falls through silently when no validator is configured or the
2909+
validator's default no-op precheck is in effect;
2910+
- converts a rejection ``ValidationResult`` into the corresponding
2911+
``HTTPException`` directly (the per-route ``OperationValidationError``
2912+
catch blocks don't see exceptions raised in dependencies, so we
2913+
translate here instead of relying on each handler's try/except).
2914+
2915+
Args:
2916+
operation: Short identifier for the route, e.g. ``"retain"``.
2917+
2918+
Returns:
2919+
A FastAPI dependency callable suitable for ``Depends(...)``.
2920+
"""
2921+
2922+
async def _precheck_dep(
2923+
bank_id: str,
2924+
request_context: RequestContext = Depends(get_request_context),
2925+
) -> None:
2926+
validator = getattr(app.state.memory, "_operation_validator", None)
2927+
if validator is None:
2928+
return
2929+
from hindsight_api.extensions import PrecheckContext
2930+
2931+
await app.state.memory._authenticate_tenant(request_context)
2932+
ctx = PrecheckContext(
2933+
operation=operation,
2934+
bank_id=bank_id,
2935+
request_context=request_context,
2936+
)
2937+
result = await validator.precheck(ctx)
2938+
if not result.allowed:
2939+
raise HTTPException(
2940+
status_code=result.status_code,
2941+
detail=result.reason or "Operation not allowed",
2942+
)
2943+
2944+
return _precheck_dep
2945+
28952946
# Global exception handler for authentication errors
28962947
@app.exception_handler(AuthenticationError)
28972948
async def authentication_error_handler(request, exc: AuthenticationError):
@@ -3149,7 +3200,10 @@ async def api_get_observation_history(
31493200
)
31503201
@audited("recall")
31513202
async def api_recall(
3152-
bank_id: str, request: RecallRequest, request_context: RequestContext = Depends(get_request_context)
3203+
bank_id: str,
3204+
request: RecallRequest,
3205+
request_context: RequestContext = Depends(get_request_context),
3206+
_precheck: None = Depends(precheck_for("recall")),
31533207
):
31543208
"""Run a recall and return results with trace."""
31553209
import time
@@ -3337,7 +3391,10 @@ def _fact_to_result(fact: "MemoryFact") -> RecallResult:
33373391
)
33383392
@audited("reflect")
33393393
async def api_reflect(
3340-
bank_id: str, request: ReflectRequest, request_context: RequestContext = Depends(get_request_context)
3394+
bank_id: str,
3395+
request: ReflectRequest,
3396+
request_context: RequestContext = Depends(get_request_context),
3397+
_precheck: None = Depends(precheck_for("reflect")),
33413398
):
33423399
metrics = get_metrics_collector()
33433400

@@ -3835,6 +3892,7 @@ async def api_create_mental_model(
38353892
bank_id: str,
38363893
body: CreateMentalModelRequest,
38373894
request_context: RequestContext = Depends(get_request_context),
3895+
_precheck: None = Depends(precheck_for("mental_model_create")),
38383896
):
38393897
"""Create a mental model (async - returns operation_id)."""
38403898
try:
@@ -3883,6 +3941,7 @@ async def api_refresh_mental_model(
38833941
bank_id: str,
38843942
mental_model_id: str,
38853943
request_context: RequestContext = Depends(get_request_context),
3944+
_precheck: None = Depends(precheck_for("mental_model_refresh")),
38863945
):
38873946
"""Refresh a mental model by re-running its source query (async)."""
38883947
try:
@@ -5729,7 +5788,10 @@ async def api_list_webhook_deliveries(
57295788
)
57305789
@audited("retain")
57315790
async def api_retain(
5732-
bank_id: str, request: RetainRequest, request_context: RequestContext = Depends(get_request_context)
5791+
bank_id: str,
5792+
request: RetainRequest,
5793+
request_context: RequestContext = Depends(get_request_context),
5794+
_precheck: None = Depends(precheck_for("retain")),
57335795
):
57345796
"""Retain memories with optional async processing."""
57355797
metrics = get_metrics_collector()
@@ -5899,6 +5961,7 @@ async def api_file_retain(
58995961
files: list[UploadFile] = File(..., description="Files to upload and convert"),
59005962
request: str = Form(..., description="JSON string with FileRetainRequest model"),
59015963
request_context: RequestContext = Depends(get_request_context),
5964+
_precheck: None = Depends(precheck_for("files_retain")),
59025965
):
59035966
"""Upload and convert files to memories."""
59045967
from hindsight_api.config import get_config

hindsight-api-slim/hindsight_api/extensions/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
# Core operations
4141
OperationValidationError,
4242
OperationValidatorExtension,
43+
PrecheckContext,
4344
RecallContext,
4445
RecallResult,
4546
ReflectContext,
@@ -72,6 +73,7 @@
7273
"DeferOperation",
7374
"OperationValidationError",
7475
"OperationValidatorExtension",
76+
"PrecheckContext",
7577
"RecallContext",
7678
"RecallResult",
7779
"ReflectContext",

hindsight-api-slim/hindsight_api/extensions/operation_validator.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,33 @@ def reject(cls, reason: str, status_code: int = 403) -> "ValidationResult":
8282
# =============================================================================
8383

8484

85+
@dataclass
86+
class PrecheckContext:
87+
"""Context for a pre-body-parse precheck on an operation.
88+
89+
Unlike :class:`RetainContext` / :class:`RecallContext` / etc., this
90+
context is constructed *before* the request body is deserialised. It
91+
therefore intentionally carries only the cheap, already-resolved
92+
pieces of request state:
93+
94+
- ``operation``: a short string identifying the route, e.g. ``"retain"``,
95+
``"recall"``, ``"reflect"``, ``"files_retain"``, ``"mental_model_create"``,
96+
``"mental_model_refresh"``.
97+
- ``bank_id``: parsed from the URL path.
98+
- ``request_context``: the authenticated :class:`RequestContext` (tenant
99+
already resolved by the tenant extension).
100+
101+
Implementations should keep precheck cheap and side-effect-free. The
102+
full per-request validators (``validate_retain`` / ``validate_recall``
103+
/ ``validate_reflect``) still run after the body is parsed and remain
104+
the source of truth for the precise per-call cost / quota arithmetic.
105+
"""
106+
107+
operation: str
108+
bank_id: str
109+
request_context: "RequestContext"
110+
111+
85112
@dataclass
86113
class RetainContext:
87114
"""Context for a retain operation validation (pre-operation).
@@ -407,6 +434,42 @@ class OperationValidatorExtension(Extension, ABC):
407434
- consolidate (mental models consolidation)
408435
"""
409436

437+
# =========================================================================
438+
# Pre-body-parse hook (optional - default no-op)
439+
# =========================================================================
440+
441+
async def precheck(self, ctx: PrecheckContext) -> ValidationResult:
442+
"""
443+
Cheap pre-body-parse check, called before the request body is read.
444+
445+
FastAPI resolves ``Depends`` callables before deserialising the route
446+
body; routes that wire ``precheck`` as a dependency therefore short
447+
-circuit here without ever materialising the JSON payload in memory.
448+
That makes this the right hook for "should this caller be allowed to
449+
spend resources on this request at all" decisions — e.g. a balance
450+
is exhausted, a key is revoked, or a tenant is rate-limited.
451+
452+
Implementations should:
453+
- Be cheap: prefer cached lookups, avoid heavy DB queries.
454+
- Use only data on ``ctx`` (operation name + bank_id + request_context);
455+
the body is not yet available.
456+
- Be conservative on errors: prefer ``ValidationResult.accept()`` so
457+
a transient lookup failure doesn't turn into a request rejection.
458+
The post-body ``validate_*`` hooks still run and remain the source
459+
of truth for the precise per-call cost check.
460+
461+
Default implementation accepts everything. Override to opt in.
462+
463+
Args:
464+
ctx: Pre-body context with operation name, bank_id, and
465+
request_context (tenant already resolved).
466+
467+
Returns:
468+
ValidationResult indicating whether the request may proceed to
469+
body parsing and the post-parse validators.
470+
"""
471+
return ValidationResult.accept()
472+
410473
# =========================================================================
411474
# Pre-operation validation hooks (abstract - must be implemented)
412475
# =========================================================================

0 commit comments

Comments
 (0)