505 Commits

Author	SHA1	Message	Date
Nicolò Boschi	830f92118e	fix(docs): link the models page to configuration relatively, not via /docs/ (#4361) ... The multi-LLM chain link on the models page pointed at /docs/developer/configuration#..., but the docs are served from the site root, so that path does not exist. It went unnoticed since #3983 because the current docs are not part of a normal docs build — only the released 0.9 snapshot is — and surfaced when cutting 0.10.0: release.sh snapshots the current docs as the new default version, and the docs build it runs failed on the broken link, aborting the release before anything was tagged or pushed. Every other link on the page uses the relative ./configuration form, which Docusaurus resolves within each version, so this one now does too.	2026-09-14 12:41:48 +02:00
Chris Bartholomew	54fc68e705	feat(llm): per-member timeout and retry budget in a multi-LLM chain (#4336) ... * feat(llm): per-member timeout and retry budget in a multi-LLM chain A failover chain is itself a retry: when a member fails, the next one is tried. Retrying a non-terminal member first only delays that handoff, and when the member is failing *because* it is saturated -- rejecting with 503, or timing out under its own queue -- the immediate retry is near-certain to fail the same way. Every one of those attempts holds the caller's slot. The terminal member is the opposite case. It has nowhere to fail over to, so its retry budget is the only thing standing between a transient error and a failed request, and it is the member where honouring a Retry-After actually pays. A single operation-wide MAX_RETRIES cannot express both, and until now that was the only knob: _member_to_llm applied the operation's resolved request defaults to every member. Lowering it to fail over promptly also stripped the last member's ability to ride out a rate limit. Adds HINDSIGHT_API_<OP>LLM_<n>_TIMEOUT and _MAX_RETRIES, so a chain can be configured to fail fast on the way down and retry only at the bottom: HINDSIGHT_API_LLM_MAX_RETRIES=0 # primary: hand off immediately HINDSIGHT_API_LLM_1_MAX_RETRIES=2 # last member: absorb transients Unset means inherit, so an existing chain is unchanged -- _member_call_defaults returns the operation's kwargs untouched when a member overrides neither field. 0 is deliberately distinct from unset: it is a meaningful setting (fail over immediately) and is parsed as 0 rather than collapsing back to the default. Per-member timeout matters for the same reason and is arguably the sharper tool: a stalled member holds a slot for the full operation timeout before the chain can move on, but lowering that timeout globally also cuts off members whose responses are legitimately slow. Tests cover inherit-when-unset (asserting the kwargs are identical to the operation's, so the untouched path cannot drift), override, the two fields being independent, explicit 0 surviving, per-op prefixes, invalid input, and the end-to-end shape this exists for. Each override test was confirmed to fail with the override logic removed. * fix(retain): let each chain member own its transport retry budget Fact extraction forwarded its operation retry budget on every call, and the chain hands per-call kwargs to every member unchanged, so a per-call value beat each member's HINDSIGHT_API_LLM_<n>_MAX_RETRIES. With the primary at 0 and the terminal member at 2, retain gave the terminal member 0 retries. The retain LLM is already built with that budget as its default, so dropping the per-call value keeps single-LLM behaviour and lets member overrides apply. The malformed-JSON re-prompt loop still counts from the operation budget. --------- Co-authored-by: Nicolò Boschi <boschi1997@gmail.com>	2026-09-14 10:38:18 +02:00
Nicolò Boschi	6ac46e2307	feat(api,clients): admission control with a bounded wait, and client retry (#4253) ... * feat(api,clients): admission control with a bounded wait, and client retry The engine already caps concurrent work (`recall_max_concurrent` and friends), but `async with semaphore` is backpressure, not admission control: it bounds how much runs and lets an unbounded queue form behind it. Measured on a 2-vCPU container, 1024 concurrent recalls against a 32-permit semaphore produced a 12.8s p50 -- the latency did not go away, it moved into the semaphore queue, and the server spent CPU on responses whose callers had long since gone. Server: per-operation lanes with a deadline ------------------------------------------- `api/admission.py` adds lanes with two numbers: how much runs concurrently, and how long a request may queue before it is refused with 503 + `Retry-After`. Enforced by an `admit_for` dependency on the same routes that carry `precheck_for`, so a refusal happens before the body is deserialised -- the cheapest point to say no, and where an extension already rejects on quota. Lanes are per operation because per-request cost spans three orders of magnitude (measured: ~0.1ms /health/live, ~0.5ms bank stats, ~23ms recall); one global cap calibrated for recall would throttle health checks 100x too hard. Only recall, reflect and retain get lanes -- the other PrecheckOperations are low-volume administrative routes, and gating them would add knobs nobody tunes. Limits are PER WORKER and derived from the CPU budget this process actually has, via the existing cgroup-aware detector (`os.cpu_count()` reports the host's cores under `--cpus`, which would size limits for a machine the process cannot use). `in_flight` is a latency target, not a capacity limit: a c=1024 sweep measured throughput flat at 40-45 rps whether the limit was 8, 16 or 24 per worker, while p50 moved 1.4s -> 2.3s. It is bounded on both sides -- too low throttles I/O-bound work (8 permits against a 500ms provider caps a worker at 16 rps), too high rebuilds the queue this exists to prevent. A queued request whose client disconnects releases its place immediately, using the token `ClientDisconnectCancellationMiddleware` already puts on the scope. That is what makes a patient 30s deadline affordable: the queue self-cleans, so waiting costs nothing when nobody is listening. Verified end to end -- a client that gave up at 1s freed its slot at 1.003s, not at the deadline. Clients: retry the idempotent calls ----------------------------------- Both maintained wrappers retry recall and reflect on 429/503. Writes are not retried: the Python wrapper documents that `operation_id` is ignored for synchronous retain, so a retry there could duplicate. Two properties matter more than the retry. `Retry-After` is honoured, because the server sends it knowing its own queue depth. And the wait is jittered -- a burst that all receive `Retry-After: 1` and obey it exactly returns in lockstep and rebuilds the spike. The generated Python client ships `ExponentialRetry`, which has neither, and was off by default; this is why it stays off. * fix(admission): decrement queued once on abandon, drop no-op lanes, regen docs skill - An abandoned waiter decremented stats.queued in its except branch and again in finally, driving the gauge negative. - admit_for on dry-run/mental-model/files routes was a no-op (no lane exists). - Stale config comments on the kill switch and reflect sizing. - HTTP-level test for 503 + Retry-After; regenerated docs skill. * chore(embed): re-sync bundled env.example with repo root	2026-09-14 10:28:18 +02:00
Nicolò Boschi	2cd0561f14	feat(metrics): /metrics covers every worker (labelled api_worker), and event-loop lag as a histogram (#4319) ... * feat(metrics): a metrics port per worker, and event-loop lag as a histogram With --workers N every worker is its own process with its own metrics, but they share one port, so a scrape of /metrics reaches one worker at random. Counters jump between processes from one scrape to the next (a rate over them reads each switch as a reset), and process_cpu_seconds_total describes a random worker, so a worker whose event loop is saturated is invisible while the pod total still looks like headroom. - HINDSIGHT_API_METRICS_WORKER_BASE_PORT (default 0, off): each worker also serves its own registry on BASE + slot. The slot (0..N-1) is claimed with an exclusive flock on a per-slot lock file; the kernel drops it when the process exits, so a respawned worker takes over its predecessor's port. /metrics on the API port is unchanged. - HINDSIGHT_API_LOOP_LAG_METRIC (default false): the existing loop-lag probe records every sample in a hindsight.event_loop.lag histogram (seconds, with sub-second buckets), independent of its log reports. * feat(metrics): one /metrics covering every worker, labelled api_worker=<slot> Replaces the per-worker ports from the previous commit. Labelling alone would not fix the scrape: each scrape still reaches one worker, the others' series go missing from about half the scrapes, and Prometheus marks them stale. So with HINDSIGHT_API_METRICS_WORKER_LABEL on, each worker claims a slot (flock on a per-slot lock file; the kernel frees it when the process exits) and publishes a snapshot of its registry every 5 s to a directory the server's workers share. /metrics, whichever worker answers, returns every live worker's series with api_worker="<slot>": its own read live, the others from their latest snapshot, never summed. A snapshot older than 15 s is a gone worker and is skipped; a corrupt one is skipped without breaking the scrape. One port and one scrape target, so no change to charts or scrape configs. The event-loop lag histogram from the previous commit is unchanged.	2026-09-14 09:52:57 +02:00
Nicolò Boschi	d9df6d2a4d	perf(recall): three CPU cuts on the recall path (audit serialization, phase sampling, on-loop query embedding) (#4314) ... * perf(api): serialize a recall's audit row once, and decode embedding batches with orjson Two costs on every recall that a CPU profile of a recall-heavy API put at ~5% of its busy CPU (450 recalls/s, 2 vCPU): - The HTTP audit wrapper built a Python dict of the whole response with model_dump(mode="json") on the request path, and the writer then re-encoded that dict with json.dumps (3.5% alone). A pydantic response now goes straight to JSON with model_dump_json(), one pass in Rust, carried on AuditEntry.response_json. Anything else still goes through _safe_json, which uses orjson when it is installed. The stored document is the same; the column is JSON, so key order and escaping are not observable. - A TEI embedding batch is a large JSON array of floats, parsed by the stdlib decoder behind response.json() (1.4%). orjson.loads(response.content) when available. orjson is optional in both places: without it the previous code path runs unchanged. * perf(metrics): opt-in sampling for recall-phase observations Every recall records ~10 phase histograms, and OTel's aggregation behind them was ~4.6% of a recall-heavy API's busy CPU. HINDSIGHT_API_RECALL_PHASE_SAMPLE_EVERY=N records 1 in N calls, sampled independently per call, so each phase's distribution -- and its percentiles -- stay unbiased; only absolute counts scale by 1/N. The default of 1 records everything, as before. * perf(embeddings): embed a recall query on the event loop instead of a worker thread A recall embeds one short string. On the thread path that costs an executor hop plus httpx's pure-Python sync stack: 1.02 ms of CPU per query, against 0.39 ms for an aiohttp request made on the loop (measured in-process against the same TEI server; identical vectors). RemoteTEIEmbeddings.aencode_query makes one attempt and returns None on any failure, so the existing thread path and its retry policy still handle every error. It also declines when uninitialized or when a test injected a client. generate_embeddings_batch sends both paths through the same alignment and vector validation. * fix(recall-perf): declare orjson, move phase sampling into config orjson was imported behind an ImportError guard but never declared, so it was not installed and both speedups (audit rows, TEI decode) were dead code; ty also failed on the unresolved import. Declare it and drop the fallbacks. HINDSIGHT_API_RECALL_PHASE_SAMPLE_EVERY was read straight from the environment in metrics.py; it is now a HindsightConfig field with docs and env-template entries, plus a sampling unit test. The TEI embedder's aiohttp session attributes are initialised in __init__ instead of via getattr. * docs: regenerate docs skill for the recall-phase sampling flag * test(tei): hold per-thread clients so a freed client's reused id can't read as shared	2026-09-11 11:06:50 +02:00
Nicolò Boschi	5be9ad9156	perf(recall): account for a recall's time per phase, and fix the three things that showed up (#4299) ... * diag: measure event-loop lag, to tell a slow await from a busy loop Every per-phase timer in recall measures its own await, so a request that is runnable but not running is invisible to all of them: the phases stay fast while the total inflates. Measured on this API under load, the instrumented phases covered 10% of a recall's wall time (store hop 32ms, embedding 5ms of a 356ms mean) and no candidate I/O accounted for the other 90% — CPU, Postgres, the embedding service, the store, auth and the recall semaphore were each measured and excluded. Those two explanations need different fixes and look identical from the phase timings, so this measures the difference directly. The probe sleeps a known interval and reports the overshoot, which is time the loop spent elsewhere while the probe was ready. Lag near zero means a real await is missing a timer; lag tracking request latency means the loop is oversubscribed and tuning I/O will not help. Off unless HINDSIGHT_API_LOOP_LAG is set, and started from the lifespan hook so it runs on the loop that serves requests. * diag(recall): time the tenant auth await, and stop post_recall being zero by construction Two blind spots on the recall path, found while chasing 90% of a recall's wall time that no phase timer accounted for (store hop 32ms, embedding 5ms of a 356ms mean, event-loop lag 0.2ms — so a real await, not a busy loop). `recall_duration` was measured at the END of the handler, so it already contained response building, and `post_recall = handler_duration - pre_recall - recall_duration` was ~0 by construction. The line reported pre=0 post=0 for every request and charged everything to `recall=`, whichever layer actually spent it. It now ends where the engine call returns. `recall_async` awaits `_authenticate_tenant` before any phase timer starts, so that call was invisible to both the phase metrics and the handler's split. It is timed and logged when it exceeds 25ms. Diagnostics only; no behaviour change. * diag(recall): time the four untimed awaits in recall_async `recall_async` awaits four things that no phase timer covers: the operation-validator pre-hook (`validate_recall`), fuzzy tag-group resolution, the bank-config read, and the validator post-hook (`on_recall_complete`). Together with the already-timed embedding and store hop they are the whole request, so with them dark the phase metrics accounted for only ~10% of a recall's wall time and the remaining 90% looked like it belonged to some I/O nobody had instrumented. Each is timed and logged over 25ms. Diagnostics only; no behaviour change. Note for whoever reads the numbers: measured through a port-forward these are dominated by round trip latency, so the useful output is WHICH await dominates in-cluster, not the millisecond values from a laptop. * diag(recall): trace the whole request, not the parts that already had timers The recall phase metrics accounted for ~10% of a request's wall time on a loaded fleet — store hop 32ms and embedding 5ms of a 356ms mean — and the missing 90% survived every candidate: CPU at 4%, both databases idle with sub-millisecond queries, TEI at 9%, the store at 0.6%, the recall semaphore at ~0, connection pinning ruled out by a keep-alive A/B, and event-loop lag flat at 0.2ms (so a real await, not a busy loop). Each candidate was excluded one build at a time, which is slow and only ever rules things out. This times the whole path instead: * `http_to_handler` — ASGI entry to the endpoint body. `handler_start` is set INSIDE the endpoint, so routing, body parsing and dependency resolution (auth among them) sat outside every existing timer and read as unattributed time. * `engine_call` — the whole engine call, marked diagnostic because it contains the store and embedding phases and would otherwise double-count. * `post_engine` — from the engine returning to the response being built. * `validate_pre`, `bank_config`, `fuzzy_tags`, `validate_post` — the four awaits in `recall_async` that no phase covered. They are metrics, not logs over a threshold: a `>25ms` line shows only the tail and cannot distinguish "small and constant" from "small and rare", which is how a 30ms phase on 2% of requests briefly looked like an explanation for 319ms on all of them. `recall_duration` also now ends where the engine call returns rather than at the end of the handler, so `post_recall` — computed as the remainder — stops being ~0 by construction. Diagnostics only; no behaviour change. * diag(recall): time the two awaits in the precheck dependency `http_to_handler` — ASGI entry to the endpoint body — turned out to be a third of a recall, and nothing inside it was timed. It contains the `precheck_for` dependency, which the docstring says "authenticates the tenant" before the body is read, so the FIRST and uncached `_authenticate_tenant` call happens here. The one inside `recall_async` is a cached re-resolution, which is why auth kept measuring cheap and kept being cleared as a suspect. Adds `dep_auth` and `dep_precheck`. Measured locally through a port-forward, `dep_auth` is 76% of the pre-handler time and the largest single phase of the request — larger than the whole engine call. Round-trip latency inflates the absolute value; the ranking is the finding. Diagnostics only; no behaviour change. * diag(recall): close the accounting — the missing time is the metering post-hook The recall waterfall had ~950ms (45% of the engine call) that no phase covered, with the API at 1% of one core and every await, async-with and async-for in `recall_async` already timed. The cause was an instrumentation bug of my own: `on_recall_complete` has THREE call sites and only the first was wrapped, so `validate_post` recorded 0.0ms while the path actually taken went through one of the other two. With all three timed the residual is 1ms, and the breakdown is: validate_post 968 ms 45% metering post-hook search_with_retries 529 ms 24% the store + embedding validate_pre 404 ms 19% metering pre-hook bank_config 265 ms 12% engine_auth, fuzzy_tags, semaphore_acquire, validate_post ~ 0 Measured through a port-forward, so the absolute numbers are round-trip dominated; the shares are the finding. Metering is 63% of the engine call and is two synchronous control-plane hops per recall — a credits read before the work and a credits UPDATE plus `usage_records` and `crm_milestones` inserts after it. Also splits `http_to_handler` into `mw_and_routing` / `deps_total` / `body_parse`, which closes that region to a 0ms residual, and adds `recall_async_body` to separate "inside the coroutine" from "between the handler's timer and the body running" (the latter is 0). Diagnostics only; no behaviour change. * diag(recall): time the hop minus the store's own reported stages A store-answered recall reports the store's per-stage timings and `full_recall`, the whole hop. The gap between them is ours -- the client, the serialization either side, and any time the request sat in the channel -- and there was no way to read it. It cannot be derived after the fact: percentiles of separate phases are not additive, so subtracting one phase's p99 from another's says nothing. Recorded per request instead. On a measured window it is 4.8 ms mean and 10 ms at p99, which is what rules the client out as the source of a 1.9 s request tail. * perf(metrics): record a phase once, not into a histogram and a counter `record_recall_phase` and `record_validator_phase` each wrote the same measurement twice: into a duration histogram, and into a parallel counter of invocations. The histogram already carries `_count` for the identical attribute set, so the counter was a second copy of a number that was never missing. It is not free. A py-spy profile of the API under recall load put OpenTelemetry's `consume_measurement` at 8.9% of the process's busy CPU, and `record_recall_phase` alone at 5.35% -- the aggregation path, not the record call, is the cost, and it ran twice. `hindsight.recall.phase.calls` and `hindsight.validator.phase.calls` are removed rather than left emitting: a metric that exists and is never written is worse than one that is gone. Use `hindsight_recall_phase_duration_seconds_count`, which has the same value. Also adds `HINDSIGHT_API_RECALL_DIAGNOSTIC_PHASES=false` to drop the subset phases, which are the bulk of the instruments on a busy path and are only wanted while diagnosing. test_profiling sliced the histogram's argument block to the counter that follows it; the delimiter moves to the next instrument. * perf(api): a tunable gzip floor, and no TLS setup for a plaintext embedder Two things a CPU profile of a recall-heavy API found, neither of which buys anything on a deployment that is CPU-bound rather than bandwidth-bound: GZipMiddleware compressed every response over 1 KB, which a recall always is. That was ~5% of the request's CPU. `HINDSIGHT_API_GZIP_MIN_SIZE` raises the floor, and a negative value drops the middleware. Default is unchanged at 1024. `httpx.Client()` builds an SSLContext and loads the system CA bundle whatever the scheme, and an in-cluster TEI is plain http://. `ssl.load_default_certs` showed up in the profile for exactly that, paid again for each thread the pool retires and recreates. Skipped when the base URL is http://; an https:// TEI verifies exactly as before. * style: ruff format the recall phase timers verify-generated-files runs the lint hook and then fails on any resulting diff; these four files were committed unformatted. Formatting only, no behaviour change. * review(recall-perf): route the new knobs through config, tidy the timers, add tests - HINDSIGHT_API_GZIP_MIN_SIZE / RECALL_DIAGNOSTIC_PHASES / LOOP_LAG_REPORT_SECONDS (renamed from LOOP_LAG) are HindsightConfig fields now, documented and in .env.example, instead of ad-hoc os.environ reads. - get_request_context: the timestamp line sat above the docstring, which demoted it to a no-op string. - _bind_bank_id decides once per function whether to time the recall body, and no longer swallows exceptions from the metrics call. - Reuse semaphore_wait_start / backend_acquire_start instead of parallel timers. - loop_lag: keep a strong reference to the probe task, drop the noqa lambda. - Tests: diagnostic-phase flag, TEI verify for http vs https, loop-lag probe. - ruff format (the verify-generated-files failure).	2026-09-10 17:48:15 +02:00
Nicolò Boschi	d11371c2ba	perf(db): skip asyncpg's release-time reset (#4259) ... The pool releases a connection without asyncpg's reset query. That query runs on every release and cost ~1.9% of a recall-heavy API's busy CPU in a py-spy profile (the reset and release frames). Measured on a CPU-bound 2 vCPU API pod: 9.8 ms of CPU per request instead of 10.5, and a ceiling of 182 rps instead of 175. Safety depends on what the reset was cleaning up. Its first statement is `SELECT pg_advisory_unlock_all()`, so a session-scoped advisory lock taken on a pooled connection would now outlive its holder. Core has one session-scoped lock: the migration runner's `pg_try_advisory_lock`. It runs on SQLAlchemy's own connection, not this pool, and releases explicitly. Extensions that take session-scoped locks on the pool must release them explicitly or use transaction-scoped locks. `db_session_setup_on_acquire` stays on by default, because a transaction-mode pooler still needs session settings re-applied on every acquire.	2026-09-10 17:36:29 +02:00
Nicolò Boschi	11e624325b	refactor(config): make HindsightConfig the only parser of HINDSIGHT_API_* env vars (#4260) ... * refactor(config): make HindsightConfig the only parser of HINDSIGHT_API_* env vars Thirty-odd call sites across the engine read HINDSIGHT_API_* out of os.environ themselves rather than off the resolved config. Two parsers for one variable is how the engine and the config drift apart: LLMProvider.from_env() had grown its own copies of the provider defaulting, the Gemini tier gating and the cache-affinity default, each carrying a comment asking the next reader not to let them disagree. Those comments are now unnecessary. Every fixed, server-level HINDSIGHT_API_* value is parsed in config.py and read as a field. Seventeen variables that worked but had no field got one, including the seven xai-oauth knobs; the five that carry secrets are registered in _CREDENTIAL_FIELDS so they stay off the API surface. Three fields become `str | None` — host, otel_service_name, xai_oauth_base_url. Each had a caller that needed to tell "the operator set this" from "this is the default" and was reading the environment a second time to find out. The default is now applied at the single point of use. requires_api_key moves to a new leaf module, engine/provider_auth.py. config.py needs it while building HindsightConfig and llm_wrapper needs the built config at import time to size its semaphores; that cycle is the reason the LLM factory had its own env parser to begin with. Both existing import paths still work. Two consequences worth knowing: * LLMProvider.from_env() now builds the full config, so an unrelated invalid setting surfaces there instead of being bypassed. The test that asserted the opposite asserts the new contract instead. * resolve_daemon_host_port() takes configured_host from its caller rather than reading HINDSIGHT_API_HOST itself. Value vocabularies are preserved exactly where they differed from _parse_boolean_env — ACCESS_LOG still accepts yes/on, XAI_OAUTH_DEBUG_HEADERS still never raises — so no working deployment turns into a start-up error. A new test walks the package AST and fails on any HINDSIGHT_API_* read outside config.py, with a short exemption list (standalone Alembic, pre-config bootstraps, the open-ended per-extension config namespaces) and a second test that fails when an exemption goes stale. tests/conftest.py resets the config cache per test: now that values are read off a cached config, a test's monkeypatch.setenv would otherwise land against whichever config the first test in that xdist worker happened to build. * fix(config): restore the DEFAULT_HOST import and keep .env authoritative Two defects from the previous commit, both caught by CI's server start rather than the suite. DEFAULT_HOST was dropped from main.py's imports during a rebase while `config.host or DEFAULT_HOST` stayed, so every entry point died with a NameError. No test caught it: each one hands _parse_cli_args a config whose host is already a string, so the fallback branch never evaluated. The new TestParseCliArgsHostDefault covers the unset-host path, and --help no longer advertises "default: None". The second is worse. HindsightConfig is cached process-wide on first build, and an entry point imports its whole module graph before main() reaches load_dotenv_for_entrypoint(). Modules reading the config at import scope (llm_wrapper sizes its semaphores there) therefore froze a config built before the .env was applied, and it stayed frozen — a discovered .env silently ignored, surfacing as "LLM API key is required" on a server that had always started. load_dotenv_for_entrypoint() now clears the cache after loading, and daemon.py's log path and poller.py's backpressure value resolve per call instead of at import.	2026-09-09 18:40:13 +02:00
Nicolò Boschi	565303d913	docs(documents): say the tags PATCH replaces the array, and test clearing it (#4272) ... * test(documents): cover clearing a document's tags with an empty array The tags PATCH replaces the array rather than merging it, so `tags: []` is how a caller drops every tag. Every guard on that path is written `is not None` rather than a truthiness check so the empty list survives it, but nothing exercised it: a regression to `if tags:` would have turned a clear into a silent no-op and a 200. Adds an engine test (clears the document's tags and its units', runs the same observation-invalidation cascade, and is a no-op when repeated) and an HTTP test (PATCH `{"tags": []}` is 200, an omitted `tags` is still 422). * docs(documents): say that the tags PATCH replaces the array The endpoint description and the docs page both said only that tags are "propagated to all associated memory units", which leaves the question a caller actually has — does sending a tag ADD it, and how do I drop one — unanswered. The replace semantics were documented in exactly one place: two comments in the CLI tab of the docs page, which an API or SDK user never reads. States it where they will see it: the array replaces rather than merges, an omitted tag is dropped, `[]` clears them all, and only an omitted FIELD is the 422. Regenerates the spec, the clients and the docs skill.	2026-09-09 17:13:50 +02:00
Nicolò Boschi	f5b3f76a8d	fix(reflect): say when the structured-output extraction failed (#4230) (#4248) ... Reflect with a `response_schema` runs a second LLM call that reshapes the prose answer into the caller's schema. When that call errored or returned something unparseable, the bare `except` swallowed it and the caller got 200 with `structured_output: null` — indistinguishable from an answer that genuinely held nothing matching the schema. The machine-readable half, which is the reason a caller supplied a schema at all, failed invisibly: no retry signal, no alert. Returning 200 with the text answer is still right; the missing piece was saying the structured half did not happen. `StructuredOutputResult` now carries an `error`, and reflect surfaces it as a nullable `structured_output_error` on the response. Present => the extraction broke (retryable); absent with a null `structured_output` => nothing to extract. The mental-model refresh path uses the same helper and now records the reason in its `structured_output_failed` failure detail instead of only "extraction failed".	2026-09-09 12:59:00 +02:00
Nicolò Boschi	6f441b0aeb	revert: drop free-threaded CPython 3.14 support (#4037, #4067) (#4234) ... Load testing did not justify the maintenance cost of the -py3.14t target, so this removes it and the multi-loop server built on top of it. Removed outright: - `hindsight_api/_free_threading.py` and `HINDSIGHT_API_FREE_THREADING` — the guard that turned CPython's GIL-re-enable RuntimeWarning into an error. - `docker/standalone/Dockerfile.freethreaded`, `docker/freethreaded-smoke.sh`, the `test-api (free-threaded 3.14)` CI job, and the `-py3.14t` release image (including its `latest=false` carve-out in the image metadata step). - `multi_loop.py`, `HINDSIGHT_API_EVENT_LOOPS` / `--event-loops`, and `_serve_multi_loop`. Several event loops in one process is only a throughput win without the GIL; on a stock build the loops take turns, which `main.py` already warned about. Multi-loop hooks reverted with it: `run_background_tasks` on MemoryEngine and both `create_app`s, `LLMTraceRecorder.bind_loop` and its per-loop filter, and — from the #4123 follow-up — `ExtensionContext.is_primary` plus the thread-local context in `Extension`. With one loop per process the flag is permanently True and only one context is ever set, so both were dead weight on a public extension interface. `HINDSIGHT_API_MIGRATION_ISOLATION` loses its `auto` mode and now defaults to `false`. `auto` isolated only on a free-threaded interpreter, so this changes nothing for any existing deployment; `true`/`false` still force it either way. Kept, because they are real races that threads hit under the GIL too and only their rationale was free-threading-specific: the dateparser lock and the `regex>=2026.9.3` floor, one TEI HTTP client per thread, the shared bounded embeddings request pool, `bank_stats_cache`'s per-loop coalescing, and `_cross_loop.py` (still used by llm_wrapper, cross_encoder and llamacpp_llm). Their comments now stand on plain thread/loop-safety grounds. Ordinary Python 3.14 is untouched: the `build-api-python-versions` matrix still covers 3.11-3.14 and the litellm >=1.93.0 cp314 floor stays. Verified: lint.sh, ty, and the deterministic suite (8645 passed). OpenAPI and the generated clients show no drift, and the two .env.example copies stay byte-identical.	2026-09-09 10:30:09 +02:00
Rafael Kallis	4bf49c5ba0	feat(extensions): add StaticKeysTenantExtension — env-configured per-user API keys with per-schema isolation (#3675) ... * feat(extensions): add StaticKeysTenantExtension to the extensions registry Env-configured static API keys with per-user schema isolation, shipped as a standalone extension package (hindsight_ext_static_keys_tenant) following the supabase-tenant pattern: pyproject for tests, Dockerfile for image packaging, registry README entry, and developer docs pointer. Carries over the reviewed implementation: no third-party deps beyond the server, constant-time byte key comparison, fail-fast init validation (schema collisions, >63-char schema names, duplicate keys), and lowercase user-id normalization matching Postgres identifier folding. * fix(extensions): never echo API keys in config errors; derive per-key metering ids Review round 2 (nicoloboschi), must-fix #2 + inline comments: - The ValueError messages for a malformed HINDSIGHT_API_TENANT_USERS entry quoted the raw entry (user_id:api_key pair), so a misconfiguration like 'rafael:' would print the key of a nearby entry into startup logs — one paste into an issue and the key is disclosed. Errors now report the entry's index (and the user id once validated), never the key. - The duplicate-key error named the key itself; it now names the two conflicting user ids and the key's sha256-derived key_id. - _KeyEntry gains a stable, non-secret key_id (sha256 truncated to 16 hex chars), and RequestContext.api_key_id now carries it instead of a duplicate of tenant_id — metering can finally tell which of a user's keys authenticated, and errors can name a key without disclosing it. - Reworded the constant-time comment: the loop stops at the first match, so comparisons still depend on the matching key's position; harmless (invalid keys traverse the whole list) but the old text overpromised. * fix(extensions): refuse HINDSIGHT_API_TENANT_MCP_AUTH_DISABLED at startup Review round 2 (nicoloboschi), should-fix #4. On ApiKeyTenantExtension the flag downgrades one shared key to none; here it would hand unauthenticated MCP clients the base schema in a deployment built for per-user isolation. The extension now raises ValueError at init when the variable is set, and authenticate_mcp always delegates to authenticate() (no bypass). Documented in the package README's variable table. * docs(extensions): document key constraints and pre-provisioning Review round 2 (nicoloboschi), should-fix #3 + poller nit: - README states the two key-format constraints (ASCII, no comma) and why: the comma is the pair separator, and a non-ASCII key can never authenticate because header values arrive latin-1-decoded while env values are utf-8-decoded — the bytes never match, so the key would fail closed with a permanent silent 401. - Documents hindsight-admin run-db-migration as the way to pre-provision all configured tenant schemas, so the worker's idle-cycle fallback probes hit real schemas instead of raising swallowed EXISTS errors. * fix(extensions): serialize concurrent first-provision per schema Review round 2 (nicoloboschi), nit. Two concurrent first requests for the same user both saw the schema missing and both called run_migration (race inherited from supabase-tenant). A per-schema asyncio.Lock now serializes first initialization, with a re-check inside the lock so the loser of the race skips the redundant migration. Concurrent requests use distinct locks, so unrelated users never wait on each other. * ci(extensions): run static-keys-tenant tests and build its image Review round 2 (nicoloboschi), must-fix #1. The registry package had no CI coverage: its 40+ tests and its Dockerfile were never exercised on any change. Mirrors the supabase-tenant wiring exactly — a detect-changes filter and output mapping for hindsight-extensions/static-keys-tenant/**, a test-extension-static-keys-tenant job (uv sync, pytest, docker build on the latest-slim base), and the job in the report-pr-status gate. * fix(extensions): pre-encode configured keys once at init Follow-up to the constant-time comment (review round 2, inline nit): _KeyEntry now stores the compare_digest-ready bytes (utf-8/surrogateescape, the same codec bearer-token bytes are recovered with), so authenticate() encodes only the incoming key per request instead of re-encoding every configured key. Loop behavior is unchanged — bytes vs bytes, no fast path.	2026-09-09 10:19:47 +02:00
Chris Bartholomew	fb94ce0341	feat(mental-models): default list to metadata; MCP list returns metadata only (#4225) ... * feat(mental-models): default list to metadata; MCP list returns metadata only Listing mental models defaulted to returning every model's full synthesized content (and reflect_response). That bloats a caller's context and lets a single list call pull an entire bank's synthesized knowledge in bulk, when the intended way to read a model's content is the single-model read. - MCP list_mental_models tool: returns metadata only (id, name, tags, staleness); the `detail` parameter is removed. An agent discovers models here and reads a specific model's content with get_mental_model. - HTTP GET .../mental-models: `detail` now defaults to `metadata` instead of `full`. Content stays available opt-in via `detail=content`/`full`, and when requested it is delivered and metered the same as a single-model read. - Engine list_mental_models is unchanged and still honors `detail` for internal callers (bank-template export/import need full content). - Regenerated OpenAPI + clients (Python/TypeScript/Go). Tests: the MCP tool is metadata-only with no `detail` param; the HTTP list defaults to metadata and returns content only when detail=content is passed; is_stale is still reported per model on the list. * fix(mental-models): follow through on the list default flip in every caller Flipping the list endpoint's `detail` default from `full` to `metadata` left the callers that were relying on the old default reading nulls. - Control plane: `MentalModelsView` now asks for `detail=content` — it renders the content preview, source query and trigger chips, and seeds the update dialog from the listed row, so metadata alone crashed the search filter (`m.source_query.toLowerCase()` on null) and would have clobbered every trigger setting on save. The search filter is null-guarded too. - CLI: `hindsight mental-model list` asks for `content` (`--verbose` → `full`), restoring the per-row preview and keeping `--output json` useful to scripts. - Docs: the detail-levels table said `full (default)` for both endpoints and showed a `detail` argument on the `list_mental_models` MCP tool that no longer exists; the three SDK list examples printed `source_query` off a default list. Added an upgrade note. - Wrapper clients: the Python docstring still promised a server-side `full` default; the TS one said nothing. - Dropped the "metered the same as a single-model read" claim from the endpoint docstring — a `detail=content` list still validates as one `LIST_MENTAL_MODELS` bank read, not one read per model. * fix(hindsight-all): let the facade ask for mental-model content `mental_models.list()` in both facade paths (the client wrapper and the embedded namespaces) forwarded no `detail`, so after the list default flipped to metadata a hindsight-all caller got content-free rows with no way to ask for more — the one wrapper where the capability was not just defaulted away but unreachable. Forwards `detail` like the TypeScript and Python wrappers do. --------- Co-authored-by: Nicolò Boschi <boschi1997@gmail.com>	2026-09-08 18:47:54 +02:00
Nicolò Boschi	da0444a72a	feat(profiling): env-configured CPU profile, reported to the logs (#4215) ... * feat(profiling): env-configured CPU profile, reported to the logs Answers "what is burning the CPU?" for a process you cannot attach a debugger to. HINDSIGHT_API_PROFILE holds JSON -- the shape config.py already uses for structured env config -- and unset, nothing starts: HINDSIGHT_API_PROFILE='{"every": 60, "top": 20}' It reports to the log stream rather than a file or an endpoint, because the case it exists for is a process dying without explanation: a file inside the container dies with the container unless a volume was mounted in advance, and an endpoint needs a live process and a route to it. Container runtimes keep the previous container's stdout, so the last report before a crash is still readable afterwards. Each report is flushed as it is written, since a fatal signal takes buffered output with it. Three things the implementation had to work around, all verified on a running API: * The profiler is process-wide and single-instance. Since 3.12 it is a global monitoring tool, so enable() covers every thread whatever thread calls it, and a second concurrent profiler raises `tool 2 is already in use`. Per-thread profilers are not possible; this arms one for the process. * Snapshots must use getstats(), which reads the accumulated entries without stopping the profiler. Snapshot-and-clear through pstats disables the global tool, and every report after the first then silently contains nothing. Reports are deltas between snapshots, and a test asserts a second window still has data. * Sampling via sys._current_frames() is not an alternative on a free-threaded build: it stops the world, so it catches threads parked at safe points, which are I/O waits. It reported event-loop threads idle in selectors.select while /proc showed those same threads at 50-65% of a core. Every report therefore carries per-thread CPU from /proc, which profiler overhead cannot distort, as the arbiter. py-spy remains the better tool on a GIL build. It cannot read a Py_GIL_DISABLED process at all -- it locates threads through the GIL -- which is what left free-threaded deployments with nothing, and is why this exists. * fix(profiling): key window baselines by label, not id(code) CPython reuses object ids once an object is freed, and code objects are not all long-lived -- a process compiling code at runtime frees them constantly (the profiler's own output showed 8,684 compile() calls in one 30s window). Keyed by id(), a reused address would subtract another function's baseline and report a nonsense delta, silently, because the number still looks like a number. Found reviewing the diff, not by a failure. * chore(docs): regenerate the docs skill mirror skills/hindsight-docs/references/ is generated from hindsight-docs/, and verify-generated-files fails when the two diverge. Produced by ./scripts/generate-docs-skill.sh, not edited by hand. * fix(profiling): arm in create_app too, or --workers deployments profile the supervisor uvicorn with `--workers N` spawns worker processes that import the app and never run main(). Arming only in main() therefore profiles the supervisor -- which does nothing but waitpid() and ping its children -- while every request is served in a worker it cannot see. Found by running it against a real 2-worker deployment: 63 report lines, every one of them supervisor bookkeeping (waitpid, is_alive, multiprocess.ping, pickling), total tottime 0.005s, with the process serving 162 requests/s the whole time. create_app() is what a worker does import, so arming there covers them. install() is idempotent, so a single-process deployment that arms in both places gets one profiler. * fix(metrics): give the recall phase histogram millisecond-scale buckets Its unit is seconds and recall phases take milliseconds, but it was created without explicit boundaries, so the SDK default applied: 0, 5, 10, 25, ... In seconds, that makes the first bucket everything under five seconds, and every recall phase landed in it. The histogram could therefore report a mean but no usable percentile. Asked for per-phase p50/p90/p99 on a live pod it answered 2500/4500/4950 ms for all fifteen phases at once -- the interpolated midpoints of that first bucket, not measurements. A mean cannot explain a tail, and explaining the tail is what a phase breakdown is for: a phase averaging 49 ms is perfectly consistent with a 460 ms p99, and the histogram is what should tell them apart. Boundaries now span 1 ms to 10 s, which covers both a sub-millisecond fuse and a pathological store call.	2026-09-08 16:40:45 +02:00
Nicolò Boschi	9e6d9e76cc	feat(api,control-plane): a prompt tester for retain, and prompt preview for every operation (#4140) ... * feat(api,control-plane): a prompt tester for retain, and prompt preview for every operation Closes the "render prompts without calling an LLM" half of #3774. A bank's missions only mean something once you can see the prompt they land in, and today that means tracing Python constants and format calls. `POST /banks/{id}/prompts/preview` returns the messages retain, consolidation or reflect would send — in send order, no LLM call, no writes. The operation is the whole request: everything comes from the bank, and the runtime data an operation would be given is a fixed placeholder. A message arrives as `blocks`. The active ones concatenate back to the exact text sent — enforced by a test against what the extraction path itself builds. Each is identified by machine values only (`field`, a `section` slug, or the `heading` the prompt text carries); the response ships no display copy, so names and explanations live in the UI that localises them. An inactive block has no text and marks a setting switched off at the point it would land, so an unset mission is still visible where it would go. Both messages always come back: retain and consolidation keep their system prompt bank-agnostic so one provider-side cache serves every bank, and carry the mission in the user message instead. Only reflect puts its mission in the system prompt. **Two bugs found on the way, both pre-existing in dry-run extraction:** - Neither dry-run nor the preview applied retain strategies. Both resolved config directly instead of through `_resolve_retain_config`, so they ignored the bank's `retain_default_strategy` too — silently extracting and previewing under settings a real retain would never use. Both now resolve the way retain does and take an optional `strategy`. - The preview read the bank's config without running `validate_bank_read`, so a tenant extension denying `GET_BANK_CONFIG` was bypassed by an endpoint that renders that config as prompt text. Both paths now share `_authorize_bank_config_read`, which also carries the bank-existence check. **Control plane.** The dry-run dialog is gone; its work moved into the prompt tester on the bank Configuration tab, because changing a mission and seeing what it extracts is one loop that was split across two dialogs. Blocks re-render for free as settings change; a sample-text box and a Run button spend the LLM call on demand. A strategy picker renders any of the bank's named strategies. Editing a block saves that setting to the bank; `editable` comes from the config layer's own allowlist, so server-level fields say so rather than offering an edit that would collect a 400. `PromptBlockModel.kind` is required with no default: progenitor rejects a default on an inline enum with TypeError(InvalidValue), which breaks the Rust client. * chore(cli): skip preview_prompt in the OpenAPI coverage manifest	2026-09-07 17:27:51 +02:00
Sanderhoff-alt	a08ec8ddfe	fix(embeddings): send each Gemini input as its own Content (#4001) ... fix(embeddings): send each Gemini input as its own Content Passing a plain `list[str]` to the Google GenAI SDK's `models.embed_content()` reaches the API as several Parts of ONE Content, and the multimodal models (`gemini-embedding-2`+) fuse those into a single vector — the whole batch collapses to one embedding. Verified against the live Gemini API: three texts sent as a `list[str]` come back as 1 vector on `gemini-embedding-2` and `gemini-embedding-2-preview`, and as 3 when each text is its own `Content`. We worked around the fusion by forcing `batch_size = 1` for those models, which kept 1:1 alignment at the cost of one request per text. Since #4039 those requests fan out concurrently through `_encode_batched`, so this was never the ~30s serial stall the original report described — but it still burns one upstream request per fact, which is what runs into per-minute rate limits. Wrapping each text in its own `Content` fixes the cause instead of the symptom: every model returns one vector per input, so the whole batch travels in a single request and the `batch_size = 1` special case (and the model-name sniffing that drove it) goes away. Vectors are byte-identical on `gemini-embedding-001`, so existing embeddings do not need regenerating. - Wrap each input text in a distinct `Content` in `_embed_batch()`. - Drop the forced `batch_size = 1` and `_gemini_model_aggregates_inputs()`. - Add `HINDSIGHT_API_EMBEDDINGS_GEMINI_BATCH_SIZE` (default 100), matching the other providers' configurable batch size. The live `test_gemini_embedding_2_vertexai_one_vector_per_input` test now covers the batched case directly — all three texts go out in one request, which is exactly the shape that used to aggregate. A mocked client cannot prove this; the behaviour lives in the API.	2026-09-07 14:05:51 +02:00
Nicolò Boschi	66992496f5	fix(api): 404 bank-scoped reads for a bank that does not exist (#4175) (#4186) ... * fix(api): 404 bank-scoped reads for a bank that does not exist (#4175) GET /stats and GET /memories/list answered 200 with zeroed counters and an empty page for a bank nobody ever created — byte-identical to a healthy, empty bank. A monitor built on either kept passing after the bank it watched was renamed, deleted or recreated under another id, and a typo in bank_id was never surfaced. The same held for every other bank-scoped aggregate/list read: /graph, /stats/memories-timeseries, /entities, /entities/graph, /mental-models, /knowledge-base/{tree,export,search}, /directives, /documents, /tags, /operations, /observations/scopes, /config and /webhooks. (Sub-resource GETs already 404 on the missing child.) Each of those engine reads now calls _require_bank_exists after its own authentication and read authorization, so the check neither widens what a request may see nor creates the bank; the profile row is cached per process, so an existing bank costs no extra query. The 404 is declared in the OpenAPI spec on those operations, so generated clients have a documented missing-bank case. The Rust client's build script drops schema-less error responses first: progenitor models at most one error type per operation and the typed 422 is the one worth keeping. * fix(api): derive the 404 endpoint list from the routing table, not by hand Two follow-ups to the same fix. The regression test enumerated the 17 bank-scoped reads by hand, so a bank-scoped collection GET added later would be covered the day someone remembered to extend the file — exactly the sibling-parity trap the reviewed change is about. It now walks the app's own routes and asserts the contract over every one of them, with two commented exemptions (/document-transfer and /profile, both withdrawn endpoints that answer 410 Gone for every bank). The scan immediately found both, which the hand-written list had missed. The rebase onto main also landed the /profile removal underneath the earlier commit, leaving a declared 404 on an endpoint that can now only ever return 410. Dropped it, and regenerated the spec and clients. * fix(tests): create the bank in tests that read a bank they never created CI found three suites that reached an engine read with no bank row, which the new 404 turns from an empty result into an error. All three are test setup gaps, not behaviour the fix gets wrong — a real deployment always has the row, because every write path (retain included) creates it before anything else exists. - test_memories_extension: a store owns the facts, never the bank row itself, so the three seam reads now create the bank the way a retain would. - test_schema_isolation: the bank row goes into each tenant schema alongside the memory_units row it inserts directly — "created" is per schema, which is part of what the test is about. - test_knowledge_search_text_search_disabled: this engine is stubbed down to the one method under test and has no real pool, so the existence read is stubbed alongside _authenticate_tenant. Also carries the docs-skill copy of the OpenAPI spec, which verify-generated-files caught: it mirrors hindsight-docs/static/openapi.json and was left behind when the /profile 404 was dropped.	2026-09-07 13:32:10 +02:00
Nicolò Boschi	a3c5b350d3	fix(embeddings): bound embedding input by the model's context, prefix included (#4182) ... A knowledge page embeds `f"{name} {content}"`, and its content is generated against the page's own max_tokens budget — which the API allows up to 8192, the same number as the embedding model's input limit. The name and the joining space are added after that budget is spent, so a page sized to the cap arrived at the provider one token over it: a hard 400, refresh_mental_model failing for good after its 3 retries, and a mental model silently stuck on stale content (#4165). Nothing was bounding it. The only cap, `_truncate_inputs`, was opt-in behind HINDSIGHT_API_EMBEDDINGS_MAX_INPUT_TOKENS and defaulted to off, so by default oversized text went straight to the provider. Default it to 8192 — the input limit of essentially every remote embedding model (OpenAI text-embedding-3-*, Bedrock Titan V2, Cohere v3, a stock llama.cpp context), all of which reject an oversized input rather than truncating it the way SentenceTransformers does. 0 now means "no cap", since leaving the variable unset no longer does. The budget also has to cover the whole payload. `Embeddings._encode_prefixed` glues an asymmetric model's "passage: "/"query: " instruction on AFTER truncation ran, so a text cut to exactly the limit still went over by the prefix; charge it against the budget up front.	2026-09-07 13:27:53 +02:00
Nicolò Boschi	05c775c415	fix(consolidation): teach the prompt to name delete targets, and count discarded batch responses (#4151, #4152) (#4183) ... A consolidation response that fails `_ConsolidationBatchResponse` validation is classified FAIL_FAST, fails the batch, and gets bisected. When the halves then validate, every fact consolidates and nothing is left carrying `consolidation_failed_at` -- so `failed_consolidation` reads 0 and the run is indistinguishable from a clean one, even though everything those responses asked for was thrown away. #4152 is the concrete instance. `deletes[].observation_id` is required, but the prompt never showed the shape of a delete entry: both worked examples ended in `"deletes": []` and the `deletes` field rule said only *when* to delete. A model that answered with a reason-only delete had its whole response rejected -- the good creates and updates alongside it included -- and the supersession-cleanup path quietly did nothing for a whole backlog drain. Three changes: - Prompt: a worked example with a populated `deletes` array, and a field rule stating that every entry must carry the exact `observation_id`, that prose in `reason` is not enough, and that a bad entry costs the whole response. - `_DeleteAction` accepts `id` as an alias for `observation_id` -- the name a model copying the observation's own field emits, unambiguous because a delete entry has exactly one identifier. The field stays required (a delete naming no target has no defensible fallback), and the generated JSON schema still advertises `observation_id` alone, so grammar-constrained providers see no change. This only stops a near-miss from discarding the batch. - Visibility: a `hindsight.consolidation.batch_failures` counter labelled by failure class and exception type, an `llm_batch_failures` count in the consolidation job's stats, and a warning line in the run summary when it is non-zero. `failed_consolidation` is a gauge over stuck rows and structurally cannot report a failure that bisection recovered from. Recording happens on the exception path only, so a successful batch costs nothing. Tests: `test_consolidation_delete_schema.py` covers the alias, that a delete naming nothing still fails closed, that the JSON schema is unchanged, the prompt rules, and both end-to-end delete paths. `test_consolidation_batch_failure_visibility.py` covers the counter -- including that retried attempts each count, and that a clean run still reports zero. `test_consolidation_delete_prompt_llm.py` is the `hs_llm_core` prompt-following check against a real model.	2026-09-07 12:46:19 +02:00
Nicolò Boschi	864d926928	feat(llm): route retain items to a chain member by their metadata (#4188) ... Adds a `{"mode": "metadata", "routes": [...]}` multi-LLM strategy that picks the chain member from each retained item's own metadata, so one deployment can extract different documents with different models. Selection happens in extract_facts_from_text, which handles exactly one retain item, so the feature is stateless: nothing is persisted, no values are unioned across a batch, and a batch of differently-routed items sends each item to its own member. First matching route wins; an item matching no route keeps the primary. Retain-only by design — this chooses which model extracts a document, not where its data can end up. Recall, reflect, consolidation, mental models and dry-run extraction all keep using the primary, and the docs say so rather than implying a data boundary the design cannot enforce. Batch retain and an explicit metadata strategy on a non-retain operation are both rejected at startup instead of being silently ignored. The strategy JSON is parsed into a pydantic model at the boundary (strict + extra=forbid), so `{"member": true}` and a misspelled `"membr"` fail loudly.	2026-09-07 12:11:59 +02:00
Nicolò Boschi	b1de1b9418	fix(operations): allow cancelling in-flight operations (#4131) ... `DELETE /v1/default/banks/{bank}/operations/{id}` only accepted `pending` operations, so an operation stranded in `processing` — orphaned when a worker was killed before it could write a terminal status — could only be cleared by hand-editing `async_operations` and restarting the container. Cancel now accepts `processing` too. It stays cooperative and is never immediate: the row is flipped to `cancelled` and the worker running it stops at its next `_check_op_alive` checkpoint (between retain sub-batches/documents, between consolidation LLM batches). For the orphaned case nothing is running, so the flip is the whole fix. No heartbeat and no per-batch bookkeeping is added. Making the flip stick required guarding the worker writes that had none, and would otherwise overwrite it: - `_schedule_retry` — the one that actually resurrected cancelled work: a task failing after cancellation went back to `pending` and was re-claimed. - `_mark_failed` (poller and engine), `_defer_operation`. `_mark_completed` already guarded on `status='processing'`; tests now pin it. The sibling rollup counted only `completed`/`failed` as done, so a cancelled child stranded its `batch_retain` parent in `processing` forever — the same wedge one level up. Both rollup copies now treat `cancelled` as done and settle the parent on `cancelled` (a real failure still outranks it), cancel performs the rollup itself so cancelling the last outstanding child terminalizes the parent, and a cancelled parent is never flipped back by a child finishing later. Control plane: the Cancel button was gated on `pending`, hiding the fix from the UI an operator would reach for. It now shows for `processing` rows too.	2026-09-07 11:21:06 +02:00
Nicolò Boschi	d69abe7b95	fix(reranker): give every remote reranker bounded retry (#4139) ... Closes #4134. The reranker twin of #4103: of fourteen CrossEncoderModel implementations only RemoteTEICrossEncoder retried anything. This is worse than the embeddings case, because nothing absorbs it. create_cross_encoder_from_env returns the reranker bare when no HINDSIGHT_API_RERANKER_<n>_* members are configured — the default — and neither CrossEncoderReranker.rerank nor its caller in memory_engine catches. So one 429 from Cohere/Google/SiliconFlow/Alibaba/LiteLLM/ ZeroEntropy failed the whole recall, synchronously, with the caller waiting. Retry now lives on CrossEncoderModel.predict, which delegates to a _predict every backend implements. Putting it on the shared entry point rather than in each provider is the point: retry applies to the one method a backend has to write, so the next remote reranker inherits it instead of having to remember. _predict is deliberately NOT abstract — CrossEncoderModel is exported from hindsight_api, and a subclass that overrides predict directly must keep working; it simply opts out. test_every_backend_implements_predict_via_the_shared_entry_point enumerates the backends from the module and fails if one opts out by accident. The policy machinery moves out of embeddings.py into engine/remote_retry.py (RetryPolicy, RetryBudget, call_with_retry, acall_with_retry, is_transient_remote_error, status_code_of) so the rerankers share it rather than growing a fourth private copy. Embeddings behaviour is unchanged — same code, provider-agnostic names. Tuned by four new static knobs, defaulting to 3 retries within a 10s wall-clock budget. Tighter than the embedding budget: rerank runs after retrieval has already spent time on the same request, and its failure mode is a degraded ranking rather than no answer. Exempt, with the reason recorded next to each: local/flashrank/jina-mlx (in-process — a bad tensor is not fixed by trying again), rrf (passthrough), and tei (own retry loop, which a second layer would multiply). MultiCrossEncoder keeps no policy of its own because its members each hold one. That last part makes MultiCrossEncoder's docstring true for the first time. It has always claimed "each member keeps its own retry budget, so we only advance after a member has exhausted its retries" — true only of the TEI member, so every other chain advanced on the first blip and burned a fallback over something a one-second backoff absorbs.	2026-09-07 10:06:50 +02:00
Nicolò Boschi	163fbb0ede	feat(api)!: retire the bank profile and background endpoints (#4127) ... * feat(api)!: retire the bank profile and background endpoints GET/PUT /v1/default/banks/{bank_id}/profile and POST /v1/default/banks/{bank_id}/background have been deprecated for several releases. They now answer 410 Gone with the replacement call in the detail, joining the two endpoints (entity regenerate, synchronous document export) that already do. The routes stay in the OpenAPI spec with unchanged signatures, so no generated SDK method disappears from under a caller — only the behaviour changes. Disposition traits and the reflect mission are bank configuration, and already were: _get_bank_profile_authenticated overlaid config on top of the legacy DB columns. The `name` these endpoints also returned is a display-only label available on the bank list. To make the config API a complete replacement, GET .../config is no longer gated on HINDSIGHT_API_ENABLE_BANK_CONFIG_API — that flag now gates only the writes (PATCH/DELETE). A bank must always be able to read its own resolved settings. Clients migrated in the same change: - control plane: bank-profile-view and bank-config-view read disposition and mission from the config API, the display name comes from the filtered bank list, and the dead /api/profile proxy route is gone. - hindsight-cli: `bank disposition`, `bank set-disposition` and the hidden `bank background` move to the config API. `background` warns that it now replaces the mission rather than LLM-merging into it — nothing replaces that merge — and its `--no-update-disposition` flag is accepted but ignored, as the server stopped inferring disposition from the mission long ago. - TS wrapper: getBankProfile carries a @deprecated pointer. * test(control-plane): cover the composed bank profile, and drop its extra fetch bank-context only needs the display name, so it reads the id-filtered bank list directly instead of going through getBankProfile, which would also fetch the bank config it has no use for. * feat(cli)!: drop the deprecated `bank background` command The server endpoint is gone, and the LLM merge it performed has no replacement — `bank mission` sets the mission outright. Keeping the command as an alias would have silently turned a merge into an overwrite, so it is removed rather than repointed. * fix(ci): update the CLI coverage manifest, doc example and TS client test - .openapi-coverage.toml: the three retired operations move to [skip] alongside export_documents_sync_removed, and the stale add_bank_background / update_bank_disposition field sections are dropped. The CLI helper is renamed set_bank_disposition so it no longer satisfies the coverage grep by name while calling update_bank_config underneath. - cli-reference.sh: the two `bank background` snippets become one `bank mission`, the command that replaces them. - main_operations.test.ts: TestBankProfile asserts the 410 and reads the same data back from the bank config. try/catch rather than .rejects, since this file runs under both jest and Deno's @std/expect shim. * style: rustfmt the CLI edits, and say why the 410 handlers keep unused params	2026-09-04 18:09:05 +02:00
Nicolò Boschi	c1f70087f3	fix(embeddings): route Cohere and ZeroEntropy through the shared retry policy (#4129) ... The same gap #4103 reported for the native Gemini provider, in the two remaining remote embedding backends that had no retry at all: - CohereEmbeddings builds a plain cohere.Client, and the SDK's request-level max_retries defaults to 0 — nothing retries. - ZeroEntropyEmbeddings posts through a bare httpx.Client and turns any HTTPError straight into a RuntimeError. Either way a single 429 failed the whole retain or consolidation operation, leaving only the worker's coarse whole-task retry to absorb an ordinary quota window. Both now use EmbeddingRetryPolicy with _call_with_retry, as the LiteLLM backends have since #3090 and Gemini since #4124: bounded attempts, a wall-clock budget shared across the concurrent batches of one encode(), and the HINDSIGHT_API_EMBEDDINGS_* knobs. Cohere's startup dimension probe is retried too, via to_thread so a blocking backoff cannot stall the model loads running concurrently with initialize(). Order matters in the ZeroEntropy path: the retry wraps the post and raise_for_status, while the RuntimeError wrap stays outside it. That wrap erases the status code, and the status code is what _is_transient_embedding_error classifies on — retrying inside the wrap would retry 401s and give up on 429s. TEI and the openai family are deliberately untouched: both already retry, through _request_with_retry and the OpenAI SDK's own max_retries.	2026-09-04 17:42:37 +02:00
Nicolò Boschi	36f4a061f9	fix(embeddings): route Gemini through the shared embedding retry policy (#4124) ... A shared Gemini project hands out 429 RESOURCE_EXHAUSTED well before anything is actually wrong, but GeminiEmbeddings raised every upstream failure straight through. The only backstop was the worker's whole-task retry, whose default budget (3 retries, 60s backoff) is far too short for a quota window — 213 retain/consolidation operations dead-lettered during one backfill across five self-hosted instances. embeddings.py already has the machinery for this: EmbeddingRetryPolicy plus _call_with_retry/_acall_with_retry, tuned by HINDSIGHT_API_EMBEDDINGS_MAX_RETRIES/_INITIAL_BACKOFF/_MAX_BACKOFF/ _RETRY_BUDGET, wired into the LiteLLM backends since #3090. Wire Gemini into the same path rather than configuring the Google SDK's own HttpRetryOptions: the shared policy is the one that carries a wall-clock budget, and that budget is what keeps a degraded provider from turning a synchronous recall — which embeds its query inline — into a long stall. Attempt count alone cannot bound added latency. Retries wrap both the startup dimension probe (via to_thread, so a blocking backoff cannot stall the model loads running concurrently with initialize()) and _embed_batch, sharing one budget across the concurrent batches of a single encode(). _status_code_of now also reads `code`, where google.genai's APIError carries the status while leaving `response` as None — matching what llm_debug.status_code_of already does for the LLM providers. The lookup is range-checked so an unrelated `code` attribute cannot be mistaken for an HTTP status. Closes #4103.	2026-09-04 15:22:10 +02:00
Nicolò Boschi	7a52b0a6b7	fix(embeddings): let litellm-sdk invoke a Bedrock inference profile ARN (#4114) ... * fix(embeddings): let litellm-sdk invoke a Bedrock inference profile ARN (#4034) LiteLLM derives the Bedrock embedding request/response shape by pattern-matching a provider name (cohere/amazon/twelvelabs/nova) out of the `model` string, while the target it actually invokes comes from a separate `model_id` kwarg that defaults to `model`. `LiteLLMSDKEmbeddings` only ever exposed one string, so both were forced to the same value. That makes an application inference profile ARN unusable: the ARN is opaque, so provider detection fails on it, and orgs whose Service Control Policy denies `bedrock:InvokeModel` on the bare model id can't use the bare id either. Neither setting works, and there is no third one. Expose the split litellm already has: `HINDSIGHT_API_EMBEDDINGS_LITELLM_SDK_MODEL` stays a recognizable id for provider detection, and the new `HINDSIGHT_API_EMBEDDINGS_LITELLM_SDK_MODEL_ID` carries the ARN to invoke. Unset, nothing changes — no `model_id` kwarg is sent and litellm falls back to `model`. * docs(models): show how to point litellm-sdk embeddings at a Bedrock profile ARN Adds a plain Bedrock example to the embeddings config block, plus a tip for the application inference profile case: which of the two vars picks the request format, which one is invoked, and why they have to be separate. Notes that chat models need none of this (bedrock/converse/<arn> takes any model).	2026-09-04 13:46:50 +02:00
Nicolò Boschi	280f098202	feat(retain): inline images and files as first-class content (#4077) ... Makes images and files first-class raw content in `retain`. `content` accepts an ordered list of text/image/file blocks, the extractor reads each attachment in the position it occupies, and every read surface hands back the attachments behind what it returns. A plain string behaves exactly as before — text-only retain is byte-identical, because everything new sits behind an ATTACHMENTS block that is empty when a chunk carries none. Blocks are flattened at the API boundary into one canonical body with atomic placeholders, so `documents.original_text` stays plain text and content_hash idempotency, `update_mode=append`, chunk-delta re-extraction and `reprocess_document` keep working untouched. Bytes live in the existing FileStorage abstraction, content-addressed by sha256. Schema (one migration, both dialects): `attachments` for the blob, `document_attachments` for which documents reference it, and `memory_units.attachment_ids` for which attachments a *fact* came from — a column rather than a third table, because those ids behave exactly like `tags`. Provenance is per fact, not per chunk. Extraction runs one call per chunk, and a chunk holding a screenshot also holds the prose around it, so a chunk-level edge cited the diagram as evidence for the paragraph that never mentioned it. The extractor is asked instead, and a fact stated in the prose carries nothing. Extraction quality was measured against a real image-QA dataset with a raw-VLM ceiling arm before merging: transcribing structured attachments rather than summarizing them, and recording how each value is drawn, took the gap between "the model can read this off the image" and "memory can answer it" from 31.3% to 10.0% on the same 40 charts. The prose-article benchmark went 75% -> 100% over the same change, so it is not chart-specific tuning. Also here: * A vision slot (`HINDSIGHT_API_VLM_*`) so attachment-bearing chunks alone use a vision model and text-only chunks stay on a cheaper retain LLM. A vision call deliberately does not fail over to the retain chain's text models — that would reintroduce the silent omission the 422 gate exists to prevent. * The extension retain hook can now see each attachment (media type, size, kind, filename) and refusing a retain reclaims its bytes, which previously stayed fetchable forever. * A filename lives on the document edge, not the blob: the same PDF can be attached under a different name elsewhere, and content-addressing made the first name win for both. Known limitations, documented rather than hidden: store-owned memory backends get nothing (that retain path is Postgres-free and pre-dates this work), very dense pages are sampled rather than exhausted, and the Python client's ContentBlock is a plain dict where TypeScript gets the real union. Breaking for Go and Rust callers: `content` is now a union, so a bare string no longer satisfies it. Go gains a `TextContent()` helper; Rust uses `Content::Variant0(...)`.	2026-09-04 12:48:08 +02:00
Nicolò Boschi	30ce3b8d11	feat(llm): add Meta Model API as a first-class provider (#4081) ... * feat(llm): add Meta Model API as a first-class provider Meta Model API (https://ai.developer.meta.com) serves the Muse family over an OpenAI-compatible chat/completions endpoint, so it slots into the existing OpenAICompatibleLLM path exactly like deepseek / zai / atlas. Set `HINDSIGHT_API_LLM_PROVIDER=meta` to route fact extraction, reflection and consolidation through it. The base URL defaults to https://api.meta.ai/v1 and the default model is muse-spark-1.3 (1M context). Two Meta-specific behaviours are worth knowing, and are documented rather than worked around: - Muse Spark always reasons. `reasoning_effort: "none"` is rejected with HTTP 400 — only minimal/low/medium/high/xhigh are accepted, or omit it. Reasoning tokens are billed against the output budget, so the per-operation max-token limits need headroom. - Chat Completions documents `max_tokens`, not `max_completion_tokens`. Muse Spark is a reasoning model but not one of the OpenAI products the frozen `_supports_reasoning_model` name list recognises, so the parameter name comes from the provider default. A test pins that, and pins that a configured reasoning_effort still reaches the request (the #3449 drop list only covers OpenAI's own non-reasoning products). The provider is registered on the chat/completions path only. Meta also serves a Responses and an Anthropic-Messages endpoint; a `meta-responses` variant mirroring openai/openai-responses was considered and deliberately left out. Capability flags are left off deliberately: Meta has no batch endpoint, and its prompt caching is automatic (no key, flag, or breakpoints), so it does not fit the explicit `get_or_create_cached_prefix` contract `supports_prompt_caching` describes — the benefit applies for free either way. Changes: - engine/llm_wrapper.py: register "meta" in create_llm_provider(), LLMProvider.valid_providers, and the default base_url map - engine/providers/openai_compatible_llm.py: register "meta" in valid_providers, default base_url, and the API-key-required check - config.py: PROVIDER_DEFAULT_MODELS["meta"] = muse-spark-1.3 - tests/test_meta_provider.py: default model/base URL, API-key requirement, the max_tokens parameter name, and reasoning_effort pass-through - hindsight-embed control center: add Meta Model API to the provider wizard - docs: add Meta to llmProviders.json (drives the providers grid, the capability table and the default-models table) and config examples in developer/models.mdx + developer/configuration.md - README + .env.example (+ the bundled embed copy): document the new provider Also colours the docs provider grid, which was previously monochrome. Each tile now carries its brand colour, taken from the Simple Icons dataset — the same project the marks themselves come from, so a tile's colour matches its mark. Near-black brands (OpenAI, Ollama, Anthropic, ...) get a dark-theme override so they do not vanish against the dark surface. Providers with no Simple Icons entry (Groq, Fireworks, Atlas Cloud, Requesty, opencode-go, Nous, llama.cpp, LiteLLM) keep the neutral inherited colour rather than an invented hex. The fallback is `inherit`, so the other IconGrid caller (ClientsGrid) is unchanged. Meta uses its own mark (SiMeta) rather than the generic OpenAI-compatible glyph. Not verified against the live API — no Meta API key was available — so muse-spark-1.3 is deliberately absent from the "Tested Models" table, which means models verified to work. Worth probing first with a real key: Meta rejects recursive JSON schemas in structured output with HTTP 400. * docs(llm): mark muse-spark-1.3 tested against the live Meta API Verified end-to-end through Hindsight's own create_llm_provider() against https://api.meta.ai/v1 (HTTP 200, valid content, token usage parsed including reasoning tokens), so muse-spark-1.3 now belongs in the Tested Models table. Four behaviours confirmed live, all matching the published docs: - Structured output with a flat json_schema works, and classified the probe input correctly (world vs experience). - Recursive JSON schemas are rejected: HTTP 400 "Recursive JSON schemas are not currently supported". Audited every Pydantic response model in engine/response_models.py for self-reference through $defs — none is recursive, so no Hindsight path is affected. - reasoning_effort "none" is rejected: HTTP 400 '"reasoning_effort" does not support "none" with this model.' Other levels are accepted. - Reasoning tokens are substantial and come out of the output budget: a trivial prompt spent 87 reasoning tokens against 11 visible output tokens, and at max_tokens=64 the response comes back with no content at all. Hindsight's defaults leave ample room (retain 64000; consolidation and reflect unbounded), so this only bites an operator who lowers the cap — which is what the configuration note added with the provider already warns about. Also records in the max-tokens test that Meta accepts max_completion_tokens as well, so sending max_tokens is a choice between two working names rather than a correctness fix. * fix(llm): reflect failed outright on Meta — tool_choice is auto-only Found by running a real Hindsight instance against Meta Model API and exercising retain, recall, reflect and consolidation end to end. Reflect returned HTTP 400 on every call: only `"auto"` is supported for `tool_choice`. `"none"`, `"required"`, and named function choices are not currently supported Reflect's agent loop forces a retrieval tool on its first turns, so the whole reflect surface was unusable on this provider. The unit tests could not have caught it: they cover provider construction and parameter naming, not the tool-calling path. This is the opposite failure mode to the one `_drops_tool_choice_required` handles. LM Studio and Ollama accept the field and silently ignore it, so reflect answers badly (#1563/#1179); Meta rejects the request outright, so reflect answers not at all. The two need separate predicates, hence `_rejects_non_auto_tool_choice` alongside the existing check rather than a widening of it. The field is dropped for any non-auto mode. A named choice has already been narrowed to a single tool by the block above, so the call stays practically forced under auto — the same reasoning the DeepSeek branch relies on. "none" cannot be expressed by omission and would become "auto"; no caller reaches this path with it (only the gemini, claude-code and github-copilot providers handle NONE), so that is documented in place rather than given an untested tools-stripping branch. tests/test_meta_tool_choice.py covers required, named and auto, and asserts the carve-out does not leak to other OpenAI-compatible endpoints. The LM Studio and required-downgrade suites still pass unchanged. Also documents the latency finding from the same run: Muse Spark reasons before every reply, and reflect's 30s default deadline is too short for its final synthesis — it timed out four times before failing. Raising HINDSIGHT_API_REFLECT_LLM_TIMEOUT and HINDSIGHT_API_LLM_TIMEOUT to 300 makes reflect return a correct grounded answer in ~60s. Verified end to end on the fixed build: retain 26s (3 facts, entities and a March 2026 temporal range), recall 1.7s (3 hits, correctly ranked), reflect 60s (grounded answer), consolidation completed (4 observations, 15 links, 0 failed operations). * docs(models): give Meta Model API its own setup section with the required knobs The provider's settings were inline comments inside the shared 20-provider config block, which is the wrong place for something an operator must act on: three of the four are required, not tuning, and one of them (the reflect deadline) is the difference between reflect working and reflect returning nothing at all. Adds a "Meta Model API Setup" section alongside the other providers that need one, with the required knobs as a table that states why each is required — every one of them a consequence of Muse Spark always reasoning before it replies. Also records the model lineup, the contributor-tier trade-off, and the four things worth knowing up front: prompt caching is automatic (which is why the capability table shows none), there is no batch or embeddings endpoint, recursive JSON schemas are rejected, and calls are slow. The shared config block keeps the two timeout exports, since they are required to be set, and now points at the section for the reasoning.	2026-09-04 12:08:55 +02:00
Nicolò Boschi	d20893a83f	fix(api): paginate GET /observations/scopes and GET /webhooks (#4075) ... * fix(api): paginate GET /observations/scopes and GET /webhooks Both endpoints returned the whole collection: the scopes endpoint grouped every observation in a bank and shipped one row per distinct tag set, and the webhooks endpoint selected every webhook row. Neither is bounded by construction — a bank has as many scopes as it has distinct tag sets — so the payload and the per-row work grew with the data. Both now take `limit` (default 100, max 1000) and `offset` and return `total` / `limit` / `offset` alongside the page, the same shape `list_tags` and the other paged list endpoints use. The bound reaches the SQL: the scope histogram is grouped, ordered and paged in one query with a separate `COUNT(DISTINCT scope)` for `total`, and the webhook listing gets `LIMIT/OFFSET` plus a count query. Webhook rows now order by `created_at, id` so a page boundary cannot fall inside a group of rows sharing a timestamp. Consumers page with them: the control-plane webhooks view walks every page (it renders the whole list and its count), the two proxy routes forward `limit`/`offset`, and the CLI prints "Showing N of M webhooks" rather than presenting the first page as the whole list. Clients, OpenAPI spec and the docs skill regenerated. * fix(tests): page the in-memory store's observation_scope_counts stub The MemoriesExtension conformance test asserts every stub method accepts the interface's params, so widening the interface with limit/offset left the InMemoryMemories stub behind (test-api shards 2/3 and free-threaded). It now groups, orders and pages its own observation rows and returns the same {scopes, total, limit, offset} shape as its list_tags neighbour, rather than the bare list it used to hand back. * fix(control-plane): page the webhooks view instead of loading every page The first cut walked every page in loadWebhooks and rendered the lot, so the bound existed but nothing in the UI showed it — a bank with hundreds of webhooks still built one unbounded table, and a reader had no way to tell there was more than a screenful. It now fetches one 50-row page and exposes the pager entities-view uses: first/prev/next/last, "1 / 3", and a "51-100 of 120" range line, with the header count coming from `total` rather than the loaded page. The controls hide when everything fits on one page. Two paging hazards handled: a bank switch resets to page 1 (the page number belongs to the bank being left, and carrying it over lands on an offset the new bank may not reach), and creating a webhook jumps to the page it lands on — rows are oldest-first, so reloading the current page would close the dialog and appear to do nothing. Deleting the last row of the final page steps back a page rather than stranding an empty one.	2026-09-04 10:34:54 +02:00
Nicolò Boschi	9697c69008	perf(embeddings): issue a remote provider's batches concurrently (#4039) (#4043) ... Every remote embedding provider walked its batches in a plain `for` loop, and `generate_embeddings_batch` handed the whole list to that one call in a single executor slot. A retain therefore held exactly one embedding request open at a time, no matter how much text it had. That is a real ceiling regardless of what any given workload is bound by. Measured directly against TEI (bge-small-en-v1.5, one L4, ~430-token inputs, no API in the path), the same server sustains 903 texts/s at one in-flight request and 2,080 at eight; for hosted providers the longer round trip makes the serialization cost proportionally more. Scope note: #4039 attributed an observed retain throughput of ~674 texts/s to this serialization and predicted a 3.1x end-to-end. That attribution turned out to be wrong — the retain bottleneck was elsewhere — so this claims no end-to-end number. What it does is remove a client-side ceiling that binds as soon as whatever is currently in front of it moves. `Embeddings._encode_batched()` now owns both the batching and a bounded fan-out, so a provider only says how to embed one batch. Results are concatenated in input order regardless of completion order, a failing batch propagates (the earliest one, so the error does not depend on timing), and each batch runs under its own `contextvars` copy so per-bank cost attribution survives the thread hop. Converted the six providers the issue names plus `CohereEmbeddings` and `GeminiEmbeddings` — the latter matters most, since `gemini-embedding-2` returns one aggregated vector per request and is forced to `batch_size=1`, making every text its own serial round trip. Concurrency is a class attribute defaulting to 1 and raised for remote providers in the factory: `local` and `onnx` run in-process, have no round trip to overlap, and already batch internally. Set there rather than in eight constructor signatures because the bound is a property of the deployment's embedding service, identical for every remote provider. The requests go out on one pool per backend instance, not one per encode() call. A pool per call multiplies threads by every concurrent caller and makes the bound per-caller when it is meant to describe the service — four concurrent retains would put 4x max_concurrent_requests on the wire. That matters more now that the API can serve from several event loops in one process (#4067) on a free-threaded build (#4037), where those callers genuinely run at the same time. A single-batch call (a recall query) still never touches the pool at all, so the latency-sensitive path cannot queue behind a retain. One new knob, `HINDSIGHT_API_EMBEDDINGS_MAX_CONCURRENT_REQUESTS` (default 8). `HINDSIGHT_API_EMBEDDINGS_TEI_BATCH_SIZE` deliberately stays at 32: the sweep put batch 8 slightly ahead of batch 32 once fan-out exists (2,080 vs 1,904 texts/s), but a ~9% edge measured on one model, one accelerator and one input length is too thin to change the request profile of every existing TEI deployment, and 32 is TEI's own `--max-client-batch-size` anyway. The retain coalescer already kept 4 requests in flight, and the two layers multiply: sizing a hand-off at the backend's full capacity would put 32 requests on the wire, four times the measured optimum, into a server that answers overload with 429s. `resolve_max_batch_size` now divides the backend's concurrency across the coalescer's slots, so the slots keep the pipeline from stalling on the slowest request of a hand-off while the total in flight lands on the backend's own bound. `_RetryBudget.spend` takes a lock — the LiteLLM providers share one budget across what are now concurrent batches, and a read-modify-write race there would under-charge it and let retries run past the ceiling. On a free-threaded build that race is no longer hypothetical. Tests assert on the shape of what reaches the transport, not on wall-clock: observed peak concurrency (via a barrier, so the peak is a real observation and not a lucky interleaving), that the bound holds across two simultaneous callers rather than per caller, input order under out-of-order completion, failure propagation, and contextvar propagation into worker threads. Two of them are family guards over the module's AST — every non-in-process provider must route through `_encode_batched`, and every factory branch must wrap in `_with_request_concurrency`. The serial loop was in all six providers at once, so a per-provider test cannot catch the seventh provider nobody writes a test for.	2026-09-03 15:18:27 +02:00
Nicolò Boschi	ae44a47236	feat(api): serve from several event loops in one process (#4067) ... * feat(api): serve from several event loops in one process A single event loop runs Python on one thread, so every request's CPU work — RRF fusion, pydantic validation, JSON serialisation — serialises behind it no matter how much of the request is spent waiting on Postgres. On a free-threaded build that ceiling is removable: run several loops, each on its own thread. `--event-loops N` (HINDSIGHT_API_EVENT_LOOPS) starts N uvicorn servers over one shared listening socket. Default 1, so nothing changes unless it is asked for; on a GIL build it warns, because there the loops only take turns. Measured on the -py3.14t image, same image and same data, 24 concurrent recalls: 1 loop 58 rps p50 406ms p95 482ms 6 loops 162 rps p50 136ms p95 236ms Three things could not be shared, each found by it breaking: - **The app.** uvicorn runs a lifespan per server, and an asyncpg pool belongs to the loop that created it — a shared pool corrupts under load ("got result for unknown protocol state"). Each loop builds its own engine and pool. - **The connection budget.** The configured pool size describes a process, so it is divided across loops rather than multiplied; N full pools exhaust max_connections, which is the first thing that goes wrong in practice. - **Everything that belongs to the process rather than to a loop.** Exactly one loop is the primary and owns migrations, the worker poller and the maintenance loop. A second poller under the same worker id claims the same tasks instead of adding capacity, and each extra maintenance loop is a duplicate sweep. Also fixes a latent cross-loop bug this exposed: the span-recorder registry is process-wide, so every engine's LLMTraceRecorder was handed every loop's LLM calls and tried to write through a pool belonging to another loop. That logged "attached to a different loop" on every call and drove concurrent unsynchronised access into asyncpg's protocol objects — observed as a segfault of the whole server under load. Recorders are now pinned to their own loop; single-loop deployments never bind one and are unaffected. Claude-Session: https://claude.ai/code/session_018HDqrzHgqZqsGc7EDqoTEu * docs: document HINDSIGHT_API_EVENT_LOOPS Claude-Session: https://claude.ai/code/session_018HDqrzHgqZqsGc7EDqoTEu	2026-09-03 11:16:08 +02:00
Nicolò Boschi	614bfc96df	feat: run Hindsight on free-threaded CPython 3.14 (-py3.14t image) (#4037) ... * test(api): guard against silently losing free-threading A free-threaded CPython re-enables the GIL the moment it imports a C extension that has not declared `Py_MOD_GIL_NOT_USED`, and says so only with a RuntimeWarning. One new module-scope import therefore reverts the whole server to single-threaded execution while every existing test still passes and the process still serves traffic -- which is exactly how the three imports fixed in the previous commit went unnoticed. Adds a subprocess probe that imports the full API surface and asserts the GIL is still disabled, plus a second case pinning the diagnostic form: PYTHONWARNINGS="error:The global interpreter lock:RuntimeWarning" which turns the warning fatal at the offending import (naming the module and the import chain) while leaving unrelated RuntimeWarnings alone. The probe runs in a subprocess because the GIL can only be re-enabled once per interpreter, so an import already done in the pytest parent would mask a regression. Skipped unless `Py_GIL_DISABLED`, so it is inert on the 3.11 matrix and only bites on a python3.14t job. Verified both ways on python3.14t: passes on the fixed tree, and a negative control that adds `import psycopg2` to the probe fails with the module named in the assertion message. * feat(api): fail loudly when a free-threaded build loses the GIL A free-threaded CPython re-enables the GIL for the whole process the moment it imports a C extension that has not declared `Py_MOD_GIL_NOT_USED`, and says so only with a RuntimeWarning. Nothing crashes and nothing degrades visibly: the server starts, serves traffic, and passes its tests, having quietly reverted to single-threaded execution. One new module-scope import is enough. Adds `hindsight_api/_free_threading.py` and calls it eagerly from the package `__init__`, next to `apply_default_thread_limits()` and for the same class of reason: both configure how the process executes, and both are worthless once the libraries they govern have loaded. `HINDSIGHT_API_FREE_THREADING` selects the mode: strict (default on a free-threaded build) — the GIL re-enable warning becomes an exception, so the offending import raises with the module and full import chain in the traceback. A GIL already on at startup raises. warn — log and continue, for bringing up a deployment whose dependencies are not all ready. off — no guard; used by the migration subprocess, which imports psycopg2 on purpose. Every mode is a no-op on a normal build, so this is inert on 3.11. * ci(api): run the test suite on free-threaded CPython 3.14 Adds a `test-api (free-threaded 3.14)` job plus the two pieces of packaging it needs, so a regression that silently re-enables the GIL fails CI instead of quietly costing the deployment its parallelism. The job asserts free-threading before running anything -- a bad build or an already-taken GIL shows up as its own red step rather than as a mass of confusing downstream failures -- and runs pytest under PYTHONWARNINGS="error:The global interpreter lock:RuntimeWarning" so a regression fails at the offending import with the module named. It is deliberately NOT gated on `has_secrets`. It uses the mock LLM, so it needs no provider credentials and therefore also runs on fork PRs, which skip every secret-gated test-api job today. Packaging: * `overrides-freethreaded.txt` drops orjson. PEP 508 has no marker for "free-threaded build", so a never-true marker is the mechanism; the alternative is forking pyproject.toml for one interpreter. * `scripts/ci/build-freethreaded-quicktok.sh` builds quicktok with pybind11>=2.13 and `py::mod_gil_not_used()`. Building the published sdist unmodified is not enough: the extension would not declare free-threading support and would re-enable the GIL on import. Delete this step once the change is upstream and released. local-ml is excluded from the job. Importing `sentence_transformers` re-enables the GIL, so a process that loads the local models cannot stay free-threaded (torch, tokenizers, safetensors and transformers are each fine on their own -- measured, not assumed). `tests/conftest.py` now skips the `embeddings` and `cross_encoder` fixtures when that stack is absent, instead of collapsing every DB-backed test into a misleading "sentence-transformers is required for LocalSTEmbeddings" ImportError. On 3.11, where local-ml is installed, nothing changes. Measured on python3.14t (Linux/aarch64, Postgres 18 + pgvector, mock LLM): the non-LLM suite is 5954 passed / 1719 skipped. Of the residue, the migration-test errors are this harness missing the `embedded-db` extra (pg0), which the CI job installs; the remaining ~28 failures are not yet attributed to the interpreter and need a same-container 3.11 control run before any are called real. NOT yet validated: the job's `uv pip install --group dev` and quicktok script invocation exactly as written. The quicktok patch-and-build was verified by hand (producing a cp314t wheel that imports GIL-free and tokenizes correctly), but the container host died mid-run before the scripted forms were exercised end to end. Refs: initiative kp-ac23cdb434a144bf947371b6e6e4f5e8 * ci(api): drop the quicktok wheel build from the free-threaded job quicktok had no free-threaded wheel, so the free-threaded CI job patched and built one (pybind11>=2.13 plus py::mod_gil_not_used()) before it could run anything. #4022 moved token counting to toktok-rs, which publishes cp3XXt wheels and declares gil_used = false, so none of that is needed: a plain `uv pip install` from PyPI now yields a working free-threaded install. Removes the build step and scripts/ci/build-freethreaded-quicktok.sh. overrides-freethreaded.txt keeps only orjson, which still publishes no cp3XXt wheel and whose build script refuses free-threading outright. * fix(api): make the shared caches and budget manager loop-agnostic Two process-wide singletons held an `asyncio.Lock`. An asyncio.Lock binds to the loop that first *waits* on it, so with several event loops in one process (free-threaded uvicorn) the first contended acquire claims it and every other loop then fails with RuntimeError: <asyncio.locks.Lock object ...> is bound to a different event loop It passes a single request and collapses under load, which is the worst possible shape: recall returned HTTP 500 from every loop but one. * engine/bank_stats_cache.py — the TTL cache behind bank config resolution. * engine/db_budget.py — ConnectionBudgetManager, a `_default_manager` singleton. Both now use a threading.Lock. That is not a workaround: every critical section either guards is await-free dict work, so the lock is never held across a suspension point and cannot block a loop, and unlike asyncio.Lock it is loop-agnostic. The cache needed one thing more. Its in-flight map coalesces concurrent loads behind an asyncio.Future, and a Future belongs to the loop that created it, so a caller on another loop must never await it. In-flight slots are now keyed by (running loop, cache key): coalescing happens within a loop, while the cached DATA stays shared across all of them, which is the part worth having. `invalidate()` detaches the key on every loop, since the slots are per-loop. Measured on python3.14t, real recall against Postgres 18 + pgvector, 8 event loops in one process, 64 concurrent: before 0 successful requests, 57 cross-loop errors after 124-165 rps at 5.6-5.9 cores, p50 354-438ms, 0 cross-loop errors For reference the same workload on 3.11 (one loop, as uvicorn runs today) does ~40 rps at 0.96 cores with p50 ~1390ms. Verified on 3.11 and python3.14t: the bank stats/info cache suites (12 passed, 13 skipped) pass identically on both. Refs: initiative kp-ac23cdb434a144bf947371b6e6e4f5e8 * fix(api): replace the remaining process-wide asyncio primitives Four module- or class-level `asyncio` primitives were left after the cache and budget-manager fixes. Each binds to the loop that first waits on it, so with several event loops in one process the first contended acquire claims it and every other loop fails with "is bound to a different event loop" — under load only, which is why none of them showed up in tests: * llm_wrapper `_global_llm_semaphore` and `_per_op_llm_semaphores` (module scope, built at import before any loop exists) * cross_encoder `RemoteTEICrossEncoder._global_semaphore` (class attribute) * llamacpp `_shared_server_lock` (module scope) Adds `_cross_loop.py` with `CrossLoopSemaphore` / `CrossLoopLock`. The counter lives in a threading primitive, which is loop-agnostic, and waiting is a short async backoff so a loop is never blocked while it queues — unlike a bare threading.Lock, these are safe to hold across `await`, which llamacpp's server start/stop needs. The caps stay PROCESS-wide rather than becoming per-loop. That preserves the existing contract: `--workers N` has always meant N independent caps, one per process, so `HINDSIGHT_API_LLM_MAX_CONCURRENT=8` keeps meaning 8 in flight per process instead of silently becoming 8 x loops against the provider. Polling rather than a cross-loop future handoff is deliberate and documented: it only runs while a cap is saturated, costs at most 20ms of extra latency acquiring a slot, and carries none of the per-loop waiter-registry state that `call_soon_threadsafe` would need. These gate LLM calls and subprocess startup, so that is not measurable. The uncontended path does not yield at all — a test pins that, since it is on every LLM call. Also documents the whole class of bug in the code-review skill: a "Concurrency" standards section (which lock, and why the choice is ownership rather than style) and review step 11d with the greps to catch it. tests/test_cross_loop_primitives.py covers cross-loop use, that the cap really is process-wide, exclusivity held across an await, and the uncontended fast path. It also pins the failure mode of a plain asyncio.Semaphore, so the reason this module exists cannot quietly stop applying. These tests need no free-threaded build — two event loops in one process reproduce it on 3.11. * fix(api): expose CrossLoopSemaphore's cap instead of a private counter test_llm_per_op_concurrency asserted the configured cap by reading asyncio.Semaphore's private `_value`, so it broke when the per-operation caps became CrossLoopSemaphores. Adds a public `capacity` property and asserts on that. Reading the cap is a reasonable thing for a caller to want; making it public is better than swapping one private attribute for another. Caught by re-running the full 3.11 suite against the final tree — the earlier 3.11 control predated this commit, so it would otherwise have shipped as an unnoticed regression on the supported interpreter. * build(docker): add a free-threaded image target (tag -py3.14t) Adds `api-builder-freethreaded` and `api-only-freethreaded`: docker build --target api-only-freethreaded -t hindsight:py3.14t . Built as its own pair of stages rather than by parameterising the existing ones. Almost nothing is shared — the interpreter has to be installed rather than taken from the base image, the dependency resolution differs, and local ML is excluded — so parameterising would have complicated the supported 3.11 path to no benefit. Nothing above these stages changes. Notes on the shape, each of which cost a build to find: * There is no official free-threaded python image; the library/python tags ship the GIL build only. uv installs the interpreter into /opt/pythons and the runtime stage copies it alongside the venv. * Not `uv sync --locked`. The resolution must drop the dependencies with no cp3XXt wheel (overrides-freethreaded.txt) and `uv sync` takes no --override, so it resolves fresh against the same pyproject. That is precisely why this ships as its own tag instead of being assumed equivalent to the pinned image. * No `uv pip check` either: it fails by design here, because pyproject still declares orjson and quicktok-v1 for every other interpreter and pip check cannot know their absence is deliberate. * libpq-dev/libpq5 are needed because psycopg2-binary has no cp3XXt wheel and builds from source. Shipping it costs the image nothing: migrations.py runs alembic in a subprocess on a free-threaded build, so psycopg2 is never imported into the serving process. * local-ml is refused outright with an explicit error rather than silently producing a mis-tagged image, since importing sentence_transformers re-enables the GIL. This tag defaults to remote embeddings and reranking. The build asserts what the tag claims: it imports the whole API under PYTHONWARNINGS="error:The global interpreter lock:RuntimeWarning" and checks `sys._is_gil_enabled()` is False. A C extension that has not declared Py_MOD_GIL_NOT_USED re-enables the GIL on import and says so only with a warning, so without this an image could look free-threaded and run single-threaded. The runtime also sets HINDSIGHT_API_FREE_THREADING=strict so the container refuses to start in that state rather than being merely slow. Verified: image builds (1.94GB), starts, runs its startup migrations with the GIL still disabled, and serves a real recall (142 results, matching the 3.11 image on the same corpus) with zero GIL warnings in its log. * test(api): guard multi-loop safety on the ordinary 3.11 suite Every cross-loop bug found while bringing up the free-threaded server — the bank stats cache, the connection budget manager, the LLM concurrency caps, and dateparser's locale dictionaries — was found by running the server with eight event loops, and none of them needed a free-threaded interpreter to reproduce. asyncio.Lock/Semaphore/Future bind to the loop that first waits on them regardless of the GIL, so two loops in two threads reproduce the whole class on 3.11. This adds that as a normal test: it runs everywhere, in seconds, with no special build. That is what keeps the free-threaded CI job from having to be the only safety net. The cheap guard catches loop-binding on every PR; the expensive job is left to cover what genuinely needs the interpreter — a C extension silently re-enabling the GIL, and races that only appear under true parallelism. Each case corresponds to a bug that shipped. Verified as a negative control by restoring the pre-fix bank_stats_cache: on 3.11 the cache test fails with "got Future ... attached to a different loop", exactly as the eight-loop server did. The final case pins the premise itself — that a module-level asyncio primitive still breaks across loops — so if CPython ever changes that, the guards above get revisited rather than quietly becoming theatre. * docs(code-review): state that both 3.11 and free-threaded 3.14 are supported The Concurrency section explained which lock to use and why, but never said why a reviewer should care — so the rules read as advice about a hypothetical future interpreter rather than a property of the two builds the project actually ships. Adds a "Supported interpreters" section naming both: CPython 3.11 (the default image, the `.python-version` pin, what `uv.lock` resolves for) and free-threaded CPython 3.14 (the `-py3.14t` image target). Neither can be deferred to a follow-up. It calls out the two things that catch people. Anything process-wide is genuinely concurrent on 3.14t, because the GIL is no longer making check-then-act accidentally atomic. And free-threading is lost SILENTLY: importing a C extension without `Py_MOD_GIL_NOT_USED` re-enables the GIL for the whole process with only a RuntimeWarning, so the 3.14t image keeps working and merely performs like the 3.11 one. "It passed CI" is therefore weaker evidence than usual, which is why the free-threaded job asserts the GIL is off before running any test and the image build asserts it too. Also notes the thing that makes this tractable to review: most of what breaks is not free-threading-specific but multi-loop, and multi-loop reproduces on 3.11 as soon as two event loops exist in one process. So new shared state is expected to be covered by tests/test_multi_loop_conformance.py in the ordinary suite, not left to the free-threaded job. Extends review step 11d and the must-fix list with the interpreter-dropping cases — chiefly a new C-extension dependency with no cp3XXt wheel on the API import path. * ci(api): drop quicktok from the free-threaded overrides, record orjson's cost quicktok left the project in #4022, so overriding it out is dead weight. Records what the remaining override actually costs, measured on a 1024-dim embedding rather than assumed: with orjson 23 us/vector, literal 6,531 chars fallback (_repr_literal) 325 us/vector, literal 20,504 chars 14x slower to render and 3.1x more bytes on the wire per vector. Both spellings parse to the same float32 bytes, so this costs throughput and nothing else, and it is on the retain path only (memories/pg/writes.py, retain/link_utils.py) — recall never renders a vector this way. That is the number worth having before anyone decides whether orjson is worth chasing upstream: it makes the free-threaded image a poor fit for a retain-heavy deployment and a fine one for a recall-heavy deployment, which is exactly the workload the free-threading work was aimed at. * fix(api): turn the free-threading guard off inside the migration child The migration subprocess imports psycopg2 deliberately — that is the entire reason it exists. But the guard this branch adds is inherited by the child, defaults to strict on a free-threaded build, and therefore turns psycopg2's "the GIL has been enabled" warning into an exception. Every migration failed: RuntimeError: Migration subprocess failed (exit 1). ERROR __main__: Failed to run database migrations: The global interpreter lock (GIL) has been enabled to load module 'psycopg2._psycopg'... which took out 1765 tests on 3.14t — every fixture that migrates a schema. This is a stacking bug, not a bug in #4033: that PR sets ENV_MIGRATION_ISOLATION to "never" in the child to stop it recursing, which is all it needs because the guard does not exist there. The guard is this branch's, so disabling it in the child is this branch's job too. Also clears PYTHONWARNINGS for the child, for the same reason one step removed: the free-threaded CI job runs the suite with that warning promoted to an error, and the child must not inherit it. Found by running the full suite on both interpreters after the rebase — the free-threading-only failures went from 19 to 1765, which is what a broken shared fixture looks like rather than a broken feature. * test(api): patch the isolation seam directly, not the cached env var The migration orchestration tests opted out of the subprocess by setting HINDSIGHT_API_MIGRATION_ISOLATION=never. That never worked: the flag is read through get_config(), whose result is cached in a module global, so setting the env var after any earlier get_config() call has no effect. It passed on 3.11 by accident — "auto" resolves to "never" there anyway, because the interpreter is not free-threaded — and failed on 3.14t, where "auto" isolates and the patched step functions were never reached. Patches migrations._should_isolate_migrations instead, which says plainly what these tests need: the fan-out has to happen in this process, because what they assert is the call sequence and a subprocess would not see the patches. Worth folding into #4033: its tests are green on 3.11 for the same accidental reason, so the flag's opt-out is not actually exercised there. * build(docker): give the free-threaded image its own Dockerfile The free-threaded stages lived in docker/standalone/Dockerfile. They shared nothing with it that mattered: a different interpreter (installed rather than taken from the base image), a different dependency resolution, no control plane, no local ML. The only thing genuinely in common is start-all.sh, and the runtime hardening was duplicated rather than reused anyway — so "reuse" was buying nothing while making a 550-line file longer and threading a build arg through four stages of the supported 3.11 image. Moves them to docker/standalone/Dockerfile.freethreaded. docker/standalone/Dockerfile is now byte-for-byte what it was before this branch. docker build -f docker/standalone/Dockerfile.freethreaded -t hindsight-api:py3.14t . Also drops overrides-freethreaded.txt entirely. It existed to remove dependencies with no cp3XXt wheel: quicktok left in #4022 and orjson in #4040, and everything remaining publishes free-threaded wheels, so there is nothing left to override. The CI job installs plainly now too. DEPENDS ON #4040. Until that merges, orjson is still in the runtime closure and the free-threaded install fails on it — verified, that is exactly what the build does without the override this commit removes. * ci(docker): build, smoke test and release the free-threaded image The `-py3.14t` image existed but nothing built it outside my machine, nothing exercised it, and the release never published it. Closes all three. CI (test-api free-threaded job) now builds the image and runs docker/freethreaded-smoke.sh against it. The build already asserts the GIL is off after importing the whole API, so a mis-tagged image fails before the smoke test starts; the smoke test then covers what a build cannot: * the container reaches /health/ready — i.e. it ran its migrations, which on this image means the subprocess path, since psycopg2 would otherwise take the GIL for the life of the process; * it serves a real retain and recall; * the SERVER process still has the GIL disabled, asserted rather than inferred from the container working, because losing it is silent. The smoke test is separate from test-image.sh rather than a flag on it: this image ships no local models, so embeddings must be remote and test-image.sh assumes a provider API key. It carries a deterministic stub embedder inline so it needs no secrets and no network, which also means it runs on fork PRs — the free-threaded job is deliberately not gated on secrets. Release: adds the tag to the docker matrix. Every entry now names its Dockerfile, since the free-threaded image has its own. Two deliberate asymmetries: * `latest` never points at `-py3.14t`. It is not a drop-in for the default tag — no local models — so it must be asked for by name. * linux/amd64 only. The build installs the interpreter and compiles psycopg2 from source, so emulated arm64 is slow enough to be worth adding deliberately rather than inheriting by default. Also silences the GIL warning inside the migration child. The child is SUPPOSED to take the GIL, and left visible the warning surfaces in a `-py3.14t` container's log as "the global interpreter lock (GIL) has been enabled" — which reads exactly like the image has silently lost its free-threading when it has not. The smoke test now treats any occurrence in the log as a failure, which only works once the expected one is gone. Documents the tag and its constraints in installation.md (regenerated docs skill). Verified locally end to end: image builds, and the smoke test passes — starts, migrates, retains, recalls, `free-threaded: 3.14.7`, no GIL warnings. * ci(api): fix what the free-threaded CI job actually caught The job failed with 64 failures on its first real run. My local container runs had missed all of them, for two reasons worth recording: I never ran the suite with PYTHONWARNINGS set, and my local venvs were not built with --all-extras the way CI's 3.11 job is. 51 of the 64 were the job's own configuration. It ran the whole suite with the GIL re-enable warning promoted to an error, but the suite imports LiteLLM on purpose to test that provider, and LiteLLM pulls in fastuuid, which has no free-threaded build — so ~50 tests failed for doing exactly what they are meant to do ("NameError: name 'fastuuid' is not defined"). The filter now applies only to the step that imports the whole API to assert the GIL is off. That is where it belongs, and the property is still asserted three more times: in tests/test_free_threading.py (in a subprocess), in the image build, and in the image smoke test. 5 were a real 3.14 incompatibility in test code: `asyncio.get_event_loop()` no longer auto-creates a loop, so `get_event_loop().run_until_complete(...)` raises "There is no current event loop in thread 'MainThread'". Replaced with `asyncio.run`, which is the supported spelling and behaves identically on 3.11. 2 were tests that need the local-ml extra, which a free-threaded install cannot have (sentence-transformers re-enables the GIL) and CI's 3.11 job does have via --all-extras. Both now `importorskip` the thing they actually need — torch's global default dtype has nothing to assert without torch, and the reflect test's MemoryEngine construction reaches the local embeddings provider. The rest are pre-existing or already attributed: the xai_oauth cleanup test fails with PYTHON_GIL=1 on the same binary, so it is a 3.14 asyncio change rather than a free-threading one. * ci(api): close the last four free-threaded CI failures Down from 64 to 4 after the previous commit; these are the remainder. Two were the local-ml pattern again. tests/test_jina_mlx_import_error.py stubs mlx, but the path under test still reaches transformers for a tokenizer — so without the extra the assertion sees "No module named 'transformers'" instead of the message it checks. It now importorskips transformers, which is what it actually needs. One was a missing environment variable rather than a code problem: the github-copilot provider looks for CLI account metadata that no runner has, and falls back to a token. The 3.11 test-api job passes GITHUB_TOKEN and this job did not. Added. It is not a repository secret — Actions provides it to every run, forks included — so the job stays runnable on fork PRs, which is deliberate given the secret-gated test-api jobs skip there entirely. The last is tests/test_xai_oauth_llm.py::test_cleanup_closes_a_client_still_draining _from_a_recycle, which fails with PYTHON_GIL=1 on the same binary. It is a 3.14 asyncio scheduling change that a plain 3.14 upgrade would hit identically, not something free-threading introduces, and it is left unfixed and attributed rather than worked around. * fix(xai-oauth): close a retired client whose drain task was cancelled `cleanup()` cancels each in-flight drain task so shutdown does not block on a request that may never land, and left the actual close to `_close_when_drained`'s `finally`. From Python 3.12 that no longer works: the cancellation is delivered at the task's next await — which IS the `await stale.aclose()` in that `finally` — so the close never runs, and CancelledError is a BaseException, so the `suppress(Exception)` around it does not catch it either. The client was therefore never closed and leaked its connections on every shutdown that happened while a recycle was still draining. `cleanup()` now closes the retired clients itself, after the drain tasks are done. The list is captured before cancelling, because the `finally` pops each entry out of `_drained` on its way through whether or not the close happened. aclose() is idempotent, so a drain that completed normally costs nothing. Found by the free-threaded CI job, but it is NOT a free-threading bug: it reproduces identically with PYTHON_GIL=1 on the same interpreter, so a plain 3.14 upgrade would hit it too. 3.11 is unaffected — the older cancellation semantics let that `finally` await run. Two earlier theories were wrong and are recorded so nobody retries them: swallowing the CancelledError is not enough (the next await is cancelled again), and `Task.uncancel()` does not help either (`_must_cancel` still fires at the next await). The close has to happen outside the cancelled task. Adds a regression test asserting the drain task is gone AND the client is closed. Verified as a negative control: with the fix reverted it fails on 3.14t with "a retired client was left open after cleanup", and passes on 3.11 either way, which is exactly the interpreter split the bug has. * test(retain): stop asserting batch dispatch ORDER in the coalescer test test_batches_never_exceed_the_backend_batch_size compared the flattened backend calls to the input list, which pins the order in which batches reach the backend. The coalescer never promised that: it runs up to `max_concurrent_requests` calls at a time (`self._slots`), so which batch lands first is a scheduling detail. Under the GIL the interleaving happened to be stable, so the assertion held. On a free-threaded interpreter the batches genuinely race and it failed on order alone — every text present, every batch within budget, every caller's vectors correct: At index 8 diff: 'chunk-16' != 'chunk-8' Compares a Counter instead, which keeps the property that actually matters — every text dispatched exactly once, nothing dropped or duplicated — and leaves the per-caller assertion below it untouched, since that is what proves each caller gets its own vectors in its own position. This is a test that was over-specified, not an implementation that regressed. The batch-size assertion the test is named for is unchanged. Verified 8/8 under xdist on 3.14t, where it was failing intermittently, and on 3.11. * build(docker): install Rust in the free-threaded builder, for litellm The image build failed on amd64: Failed to build `litellm==1.99.0` Error: command ['maturin', 'pep517', 'build-wheel', ...] Caused by: No such file or directory (os error 2) litellm publishes only abi3 wheels (cp310-abi3), and abi3 — the stable ABI — does not apply to a free-threaded interpreter, so uv cannot use them and falls back to the sdist, which builds litellm's Rust extension. litellm is a hard runtime dependency (pyproject pins it per-platform), so this is not optional. It is the same constraint that kept toktok from working free-threaded before #4022: an abi3 wheel is invisible to a cp3XXt interpreter. Rust is confined to the builder stage — the runtime image copies only the venv and carries no toolchain. Drop this once litellm publishes cp3XXt wheels. Only amd64 hit it: my local builds were linux/arm64, where the resolution differed. That is a good argument for the CI job building the image at all, which is what caught this.	2026-09-02 22:23:06 +02:00
Nicolò Boschi	f747d96c38	feat(recall): fuzzy tag matching on tag_groups leaves (#4026) (#4028) ... * feat(recall): fuzzy tag matching on tag_groups leaves (#4026) Tags increasingly hold user-facing names, and tag filtering is exact array containment. A caller filtering by what a query mentioned passes `typsecript`, and the memory tagged `typescript` is dropped before ranking runs — so the recall returns empty even though ranking would have found it. Better ranking cannot fix that; the match itself has to tolerate the misspelling. A `tag_groups` leaf gains one optional field, `resolve`, defaulting to `exact` (today's behaviour). Set to `fuzzy`, its tags are matched against the bank's tags by similarity instead of literally. That is the whole API change: no new config, no new response field, no new TagsMatch values. Matching is trigram similarity at 0.45 via `entity_resolver._trigram_similarity`, already verified byte-identical to Postgres `similarity()` (#3107), so Postgres, Oracle and store-owned backends behave the same. Resolves: typescropt/typescript 0.57, kubernets/kubernetes 0.62, user:alcie/user:alice 0.47. Does not: mango/mongo 0.33, k9s/k8s 0.14. Known limit, pinned by a test: similarity is length-sensitive. A short tag has few trigrams and one edit destroys three of them, so kakfa/kafka scores 0.20 and does not resolve. Fuzzy matching is effective on descriptive tags and close to inert on very short ones — and that same property is what keeps different short words apart. Resolution runs above the SQL layer, rewriting the leaf into ordinary exact leaves so only those reach the query builders. The ~20 SQL call sites, the Python mirrors used on the graph path, the GIN(tags) index, the store protocol and the Oracle dialect are untouched. Per mode, for tokens t1..tn resolving to E1..En: any/any_strict becomes one leaf over the union; all/all_strict becomes an AND of one OR-leaf per token, so a memory must carry some spelling of each; exact becomes an OR over the cross product, one tag per token, bounded at 32 branches and checked before enumeration, with combinations carrying fewer distinct tags than tokens dropped. Failing closed: a tag that resolves to nothing stays in the filter as itself, leaving the leaf unsatisfiable. Returning an empty list would read as "no tag filtering" in the builders and hand back the whole bank. The vocabulary comes from the existing `list_tags` store method, so there is no schema change. A bank holding more than 5000 distinct tags is rejected with a 422 rather than resolved against a truncated vocabulary. Claude-Session: https://claude.ai/code/session_011KDT484YujNcBxHzbfzNbk * fix(clients): make tag_groups reachable through the wrapper SDKs `Hindsight.recall(tag_groups=...)` and `.reflect(tag_groups=...)` raised ModuleNotFoundError for every caller. The wrapper imported `hindsight_client_api.models.recall_request_tag_groups_inner`, which the generator does not emit: it produces one union model per tag_groups shape and names it after the first schema that used it, so the class is `MentalModelTriggerInputTagGroupsInner`. Nothing caught it because the wrapper's tests never passed tag_groups and the import sits inside the `if tag_groups is not None` branch, so it only fires when the feature is used. Fixed at both call sites, with mirrored regression tests on the Python and TypeScript wrappers asserting a tag group reaches the request body with the leaf's `resolve` intact — the pair the review checklist asks for, since a capability that exists in one wrapper and not the other is invisible to client-coverage-check (it validates request-body fields, not wrapper surface). Also thread tag_groups through the control-plane recall and reflect proxy routes and their client types. Both accepted every other tag filter and silently dropped this one, so no control-plane caller could use compound tag filtering at all — fuzzy or exact. Two follow-ups from reviewing #4026: - Reject `resolve="fuzzy"` in a mental-model trigger's tag_groups. A trigger's scope is read by two paths that resolve differently: the refresh runs through reflect, which resolves fuzzy leaves, while the staleness check and the scope watermark build SQL straight from the stored groups and do not. A stored fuzzy leaf would build content from the resolved tags while never being marked stale by them, and would drift as the bank's tag vocabulary changes. - Promote `entity_resolver._trigram_similarity` to `trigram_similarity`. Two subsystems now share it — entity resolution and fuzzy tag matching — so the leading underscore misrepresented a real contract between them. Claude-Session: https://claude.ai/code/session_011KDT484YujNcBxHzbfzNbk	2026-09-02 16:50:53 +02:00
Nicolò Boschi	a467838153	feat(api): run migrations in a subprocess, controlled by a flag (#4033) ... Alembic drives PostgreSQL through SQLAlchemy's sync engine, i.e. psycopg2, which has no free-threaded build: importing it re-enables the GIL for the life of the process. A server with HINDSIGHT_API_RUN_MIGRATIONS_ON_STARTUP=true would therefore spend the rest of its life single-threaded, having done the damage before serving a single request, with no symptom but being as slow as before. HINDSIGHT_API_MIGRATION_ISOLATION = auto | true | false. auto (the default) isolates only on a free-threaded interpreter; true isolates everywhere, keeping alembic's import graph and its sync engine out of a long-lived server process regardless; false is the historical in-process behaviour. On every interpreter shipped today auto resolves to no isolation, so this is a no-op until someone runs a free-threaded build. An unknown value raises rather than defaulting: getting it wrong means migrations quietly run in the wrong process, which is invisible until something else breaks. The boundary is the whole migration entrypoint rather than each create_engine call. Schema migration also reaches ensure_embedding_dimension and the vector and text-search extension helpers, each of which opens its own sync engine, so one child covers all four sites instead of one spawn apiece. _HINDSIGHT_MIGRATION_CHILD stops the child recursing. The payload travels on stdin, not argv: run_migrations_for_schemas is called with every tenant schema at once and is documented at 20k of them, which is hundreds of KB of JSON -- past ARG_MAX on macOS and near it on Linux, so it would have failed as E2BIG only on the largest deployments. The child inherits stdout/stderr rather than having them captured, because a full sweep runs the better part of an hour and capturing would show an operator nothing until it finished. Verified on python3.14t against Postgres 18 + pgvector, and again through a forced child on pg0: the full schema applies (23 tables, head d1e2f3a4b5c6) with psycopg2 absent from the parent's sys.modules and the GIL still disabled. Config flag documented in configuration.md, .env.example, the bundled embed template, and the regenerated docs skill.	2026-09-02 15:37:00 +02:00
Nicolò Boschi	bce43b8e14	perf(tokenizer): move token counting from quicktok to toktok-rs (#4022) ... Swaps `quicktok-v1` for `toktok-rs` (vectorize-io/toktok), a Rust BPE tokenizer whose ids are byte-identical to tiktoken's, then collapses the module's interface onto the two operations the engine actually performs. The swap itself is behaviour-neutral: compared side by side over 258 texts (~250 real files from `hindsight_api/` plus the special-token and mixed-script edge cases), quicktok and toktok produce identical counts AND identical ids on all three shared encodings. No token budget, chunk boundary or truncation point moves. Interface. `_SafeEncoding` existed to force `disallowed_special=()` onto `encode()`. Every caller of the object it returned was doing either a plain `count` or `decode(encode(x)[:n])` open-coded — which is `truncate_to_tokens`. So the module's whole public surface is now `count_tokens`, `truncate_to_tokens` / `truncate_many_to_tokens`, and `BUNDLED_ENCODINGS`; the tokenizer itself is private. That keeps #1883 fixed by construction rather than by convention: every route to the raising `encode()` went through the accessor that is now `_load_encoding`. Character-boundary truncation (toktok 0.1.3). Truncation was `decode(encode(text)[:n])`, cutting on a *token* boundary. Byte-level BPE splits one character across several tokens (under o200k_base "🧠" is three), so a cut could land mid-character and decode to U+FFFD: truncate_to_tokens("hello 🧠", 2).text -> 'hello �' (before) -> 'hello ' (now) The native call also never builds ids, never decodes, and returns the original string object untouched when nothing needs cutting. `batch_truncate` replaces the Python loop in the two callers that truncate a whole list: every reranker document (both LiteLLM cross-encoders) and every embedding input. Two user-visible consequences: * `llama3` and `qwen3` are gone — quicktok bundled five vocabularies, toktok bundles three. `HINDSIGHT_API_TOKENIZER_ENCODING=llama3` now fails at the first token count with the existing "Unknown tokenizer encoding" ValueError. Docs and both env templates updated, and `BUNDLED_ENCODINGS` (which had been lying about those two) now has a test that loads every name it advertises. * A negative budget used to slice a list with a negative index, silently dropping tokens off the end; the native call would raise. It clamps to 0. Wheels: cp311-abi3 covers 3.11-3.14, so 3.14 no longer compiles from source the way quicktok did. No musllinux wheels, which is irrelevant to the shipped images (all Python stages are glibc python:3.11-slim). numpy drops to an optional extra, so the tokenizer pulls in no dependency of its own. Measured on this repo's text: 2-7x faster than tiktoken on cl100k_base, 10-16x on o200k_base; counting an 81k-token document peaks at 1 KiB vs ~3 MB.	2026-09-02 12:52:08 +02:00
Nicolò Boschi	21c2160046	feat(entity-labels): add an open-vocabulary multi-valued label type (multi-text) (#4027) ... Entity labels could classify a fact against a fixed vocabulary ("value", "multi-values") or capture one free string ("text"). There was no way to extract *several* values that cannot be enumerated when the bank is configured — the names a thing is known by (canonical name plus abbreviations, acronyms and alternative spellings), ticket references a fact cites, product codes. "multi-text" is a list[str] field with no Literal constraint, so the extractor writes as many values as the content warrants and nothing has to be declared up front. Each value becomes its own key:value entity and, with tag: true, a tag — so a bank can derive a classification from content and then filter on it at recall without the caller supplying the vocabulary. Added to MapField as well as LabelGroup: the control plane renders top-level label groups through the map-field editor, so the two type unions have to stay in sync or the UI can emit a shape the server rejects. Client wrappers could not express this. The TypeScript wrapper's updateBankConfig had no entityLabels option at all, so TS consumers could not configure a controlled vocabulary of any type; the Python wrapper typed entity_labels as list[str], which passes through at runtime but no typed caller can satisfy. Both fixed, with the mirrored mapping regression tests the wrappers' parity rule asks for. Closes #4025	2026-09-02 12:51:29 +02:00
Nicolò Boschi	3531e82f9f	fix(llm): name the phase a stalled LLM request died in (#3881) (#3992) ... Issue #3881 reports ~50% of reflect calls stalling for exactly `llm_timeout` and failing with `APIConnectionError: Request timed out` on `scope=reflect_tool_call`. That report cannot be acted on, because nothing in the code distinguishes the four things it could be. `AsyncOpenAI(timeout=<float>)` is `httpx.Timeout(150)` -- connect, read, write and pool all 150 s -- and `APITimeoutError` stringifies to "Request timed out." for every one of them. Connect and pool stalls mean the request never went out; a read stall means it went out and was never answered. Different faults, different owners, identical log line. - `describe_llm_error` renders any LLM failure as `Class: message [cause chain]` and is used by reflect's per-iteration error log, so this reaches EVERY provider, not only the ones on a client we build. It exists because several of the exceptions that end a stalled call stringify to the empty string: the bare `TimeoutError` behind `[REFLECT ...] LLM error on iteration 2: (120002ms)` (see #3982) named neither the failure nor the provider. - `describe_transport_error` appends the httpx/httpcore `__cause__` chain to every `APIConnectionError` log line in the openai-compatible, openai-responses and anthropic providers, so the phase is named at the provider too. - `build_sdk_timeout` gives a per-phase `httpx.Timeout` to every client Hindsight constructs -- the three SDK clients plus the two providers that build their own (`codex_llm`, `xai_oauth_llm`) -- and caps the connect leg at `HINDSIGHT_API_LLM_CONNECT_TIMEOUT` (10 s). Passing a bare float had silently raised connect *above* the OpenAI SDK's own 5 s default to the whole request budget, so an endpoint that never completes its handshake burned 150 s instead of failing in 10. Read/write/pool keep the full budget; 0 disables the cap. litellm and gemini own their transports and take a single total deadline; claude-code and github-copilot drive a CLI, not a socket. - The `httpx`/`httpcore` log levels become `HINDSIGHT_API_LLM_HTTP_LOG_LEVEL` instead of a hardcoded WARNING at import. This one is process-global, so it covers every httpx user including litellm and the SDKs. At DEBUG, httpcore names the stalled phase (`connect_tcp` / `send_request_headers` / `receive_response_headers`) -- the one instrument that settles "was the request ever sent?" without a packet capture. - Permit wait time is measured in `_acquire_permits` and reported through a caller-bound sink, so reflect's `agent_N=Xms` trace entry gains a `q<N>ms` suffix and a queued call can no longer be misread as a slow provider. The `.queued` stage breadcrumb from #3002 covered only the worker path -- `set_stage` is a no-op outside a worker task, so a synchronous `POST /reflect` had no signal at all. A parity test enumerates the providers that own an httpx client and asserts each caps its connect phase, so a provider added later with its own client fails that test instead of silently inheriting the whole budget. No behaviour change beyond the connect cap. The reflect debug model (`LLMCall`) is deliberately untouched so this needs no client regeneration; the queued time is on the log line. Also regenerates the docs skill, which picks up the unrelated `--shm-size=1g` drift left by #3896. Claude-Session: https://claude.ai/code/session_017ufCz6qrNxn36Stug7ek8A	2026-09-01 17:37:32 +02:00
Nicolò Boschi	921ae824a4	fix(reflect): fail the run when a retrieval tool fails, and record refused refreshes (#4003) ... A mental-model refresh could replace a document built over months with "I don't have information about that" (#2894). When a reflect tool raised, reflect handed the exception back to the model as a tool result and let the loop continue; the model answered from whatever it had — usually nothing — and that answer was indistinguishable from a run over a bank that genuinely holds nothing on the topic. Non-empty prose, so every emptiness guard on the write path let it through, and the operation was recorded as completed. Reflect now fails the run instead: - A tool that RAISES is an infrastructure failure (the database, the embedder, the reranker), not something the model can retry its way out of, so it raises ReflectToolExecutionError. One failure in a parallel batch fails the run. A tool that RETURNS {"error": ...} for a malformed or unavailable call is the model's own mistake and is still fed back to it, unchanged. - A non-context-overflow LLM error that survives one retry re-raises rather than falling through to a forced final synthesis built on evidence the failed turn never finished gathering. Context overflow keeps its deliberate degradation: that is a prompt-budgeting problem with the gathered evidence intact. - OperationCancelledError passes through untouched on both paths, so a client disconnect stays a 499 (#2122) instead of becoming a 500. The line this draws is between "we could not look" and "we looked and there is nothing there". A retrieval that succeeds and returns nothing is an answer, and still rewrites the document. The refresh re-raises both reflect failures as a typed MentalModelRefreshError (refresh_failed_reflect_error, with reasons retrieval_failed / no_answer), so they reach the operation's typed `details` through the same failure-metadata hook every other refusal already used. Content, structured document and watermark are all left untouched, so the retry re-reads the same window. Every refused refresh — the new reflect-side ones and the existing _preserve_and_fail paths — now also appends a failure record to mental_model_history carrying the reason and the exception. Before this a failed refresh left no trace on the model at all: the History tab kept rendering the last SUCCESSFUL trace as though it were current, and the only record was prose on an async-operation row no mental-model view reads. Retention is applied per kind so a run of failures cannot evict the version history. Control plane: a new Errors tab on the mental-model modal shows those records as a timeline of events — reason, attempt count, time, exception — with a red dot on the tab when there are any. History goes back to being a version browser. Consecutive identical failures collapse into one event (the worker retries each refresh), but a successful refresh between two of them breaks the chain, so separate outages stay separate. Also fixes a metadata bug found while verifying this live: a refresh is retried on the same operation row, and the success path wrote no failure_reason to overwrite the failed attempt's — producing outcome=content_written alongside failure_reason=no_answer. The success now writes it as null explicitly. Claude-Session: https://claude.ai/code/session_01F3i5UVdZRK16AZ9oFQqsg4	2026-09-01 17:07:15 +02:00
Nicolò Boschi	ac41cee604	feat(extensions): add hindsight-extensions registry and unbundle Supabase (#3988) ... * feat(extensions): add hindsight-extensions registry and unbundle Supabase Extensions were only ever bundle-able or nothing: shipping one meant putting it in `hindsight_api.extensions.builtin`, where it becomes maintainer-owned forever, lands in every image, and — because `extensions/__init__.py` eagerly re-exported every implementation — drags its dependencies into core's import graph. That pipe is why a third-party IdP's JWT client was a direct dependency of every Hindsight install. Add `hindsight-extensions/` as the registry for extensions distributed separately from the server. Its README is the contract: slots and how config env vars map onto them, how to write an extension, the package layout and naming (`hindsight-extensions/<name>/` -> `hindsight-ext-<name>` -> `hindsight_ext_<name>`), and Docker packaging. Move `SupabaseTenantExtension` there as the first entry, published as `hindsight-ext-supabase-tenant`. All 54 of its tests move with it, plus two new ones asserting the documented `hindsight_ext_supabase_tenant:...` env value actually resolves through `load_extension`. Two decisions worth their comments: - The extension does NOT declare `hindsight-api-slim` as a runtime dependency. The server is the host process that imports it, not something it installs; declaring it would let `pip install` of an extension silently move the server version underneath a running deployment. It is a dev extra, resolved from the local checkout via `tool.uv.sources` (dev-only metadata, verified absent from the built wheel). - The Docker example installs with `uv pip install --python /app/api/.venv/bin/python`, matching docker-compose/custom-models: the image's venv was created by `uv sync` and ships no `pip`, so a bare `pip install` lands in user site-packages and is invisible to the server. Core changes: - `extensions/__init__.py` and `builtin/__init__.py` export interfaces only. Nothing needed the concrete re-exports — the loader imports by path — and dropping them is what lets an extension have optional dependencies at all. - `builtin/supabase_tenant.py` stays for one minor release as a module whose `__getattr__` raises the migration instructions. `load_extension` wraps a missing *attribute*, not a failed import, so the ImportError propagates with its message intact instead of surfacing as "class not found". - Drop the direct `PyJWT[crypto]` dependency: no core module imports `jwt` any more. Note this does not shrink the install — `mcp` pulls pyjwt transitively and `cryptography` is already pinned directly — so the win here is ownership and import graph, not bytes. Locks are not checked in for extensions: `tool.uv.sources` pins the whole api-slim tree, so every core dependency bump would leave them stale. CI runs `uv sync --extra dev` and retriggers on `core` changes, since these tests run against the server's interfaces. Docs point at the registry rather than restating it, and the Deploying section's Docker recipe was replaced — it named an image (`vectorize/hindsight-api`) and a PYTHONPATH volume-mount pattern that no longer exist. Also includes two one-line generated-file syncs in skills/hindsight-docs (quickstart, installation) that were already stale on main; regenerating the docs skill picks them up. * refactor(extensions): ship extensions by image, drop the compat shim Follow-up on review. Three changes to how an extension is distributed: - Delete `builtin/supabase_tenant.py`. An install pinned to the old path now fails at startup with ModuleNotFoundError rather than a guided message. The docs carry the migration instead. - Extensions are not published to PyPI. There is no wheel, no version and no release step: the unit of distribution is an image built on top of Hindsight that installs the extension's dependencies and copies the package onto PYTHONPATH. That drops the whole "declare hindsight-api-slim only as a dev extra" problem — nothing resolves dependencies against a running server any more. - The pyproject is now test-harness only (`package = false`, no build backend, no distribution metadata), and says so in a comment so nobody re-adds packaging to it. Docs say 0.9.3, not 0.10. Since the Dockerfile is now the distribution mechanism rather than an example, CI builds it — its final `import` step is the only thing proving the extension is reachable from the interpreter the server actually runs. It builds against `:latest-slim` via a HINDSIGHT_IMAGE build arg to keep the pull cheap. Verified against the real image, not just locally: docker build -f hindsight-extensions/supabase-tenant/Dockerfile \ --build-arg HINDSIGHT_IMAGE=ghcr.io/vectorize-io/hindsight:latest-slim ... -> load_extension('TENANT', TenantExtension) inside the container returns SupabaseTenantExtension with its config resolved from the env vars. Worth noting from that build: `uv pip install 'PyJWT[crypto]' httpx` reports "Checked 2 packages" — both are already in the base image transitively. The line stays because the extension should pin what it imports rather than rely on the server's transitive tree, but it costs nothing today. 56 extension tests pass; the 3 remaining core tests (which assert no implementation is re-exported and no core module imports jwt) pass. * fix(tests): import ApiKeyTenantExtension from its module, not the package Dropping the concrete re-exports from `hindsight_api.extensions` broke `tests/test_extensions.py`, which imported `ApiKeyTenantExtension` from the package inside a multi-line parenthesised import. A collection ImportError fails the whole shard, which is why all three test-api shards and all six LLM acceptance jobs went red at once on the previous push. I'd checked for this with a single-line grep, which cannot see a name inside a parenthesised import list. Re-checked with an AST scan over every package in the repo (this was the only occurrence) and by collecting the full suite: 7780 tests collect clean.	2026-09-01 14:47:37 +02:00
Nicolò Boschi	aee42254ab	fix(recall): boost the prioritised arm in rank space, not score space (#3956) (#3987) ... * fix(recall): boost the prioritised arm in rank space, not score space (#3956) `RECALL_STRATEGY_BOOSTS=graph:high` could exclude an entire retrieval arm from the cross-encoder rather than merely deprioritising it: on a ~15k-fact bank a reporter measured recall@20 falling 0.9667 -> 0.4000, with zero semantic-only candidates surviving the reranker cap on all 30 test queries. The stage-1 boost multiplied the arm's `1/(k+rank)` RRF contribution by a weight `w`, and that sort key feeds the hard RERANKER_MAX_CANDIDATES cut. RRF with k=60 is deliberately flat: across the 300-candidate cap window the score spans only 1/61 -> 1/360, a factor of 5.9. `high` used w=7, above that spread, so the sort degenerated into a lexicographic one -- boosted arm first, rank merely a tiebreaker -- and the boosted arm took every slot. The culprit is the `k` term: in score space the displacement reach is `r_max = w*(k+s) - k`, so at the head of the ranking the constant `w*k` dominates and the boosted arm's ~366th hit outranked the other arm's first. The levels were tuned against a bank with 336 merged candidates against a 300 cap (89% survival), where the cut could evict at most 36 candidates and the boost really was a reordering; nothing in the formula carried a pool-size term, so the calibration stopped holding as pools grew. Boost the rank instead -- `1/(k + rank/divisor)` -- which cancels `k`: the boosted arm's rank `r` beats another arm's rank `s` iff `r < divisor * s`. Displacement becomes proportional rather than an absolute offset, so it can never invert the head of another arm, and it no longer depends on the merged pool size or on `k`. Levels become divisors: low=2, medium=4, high=8. Replaying the reported shape (3563 merged candidates, 300 cap, 8.4% survival), old formula vs new on an identical pool: semantic-only kept top-20 semantic kept no boost 48 20/20 old graph:high 0 3/20 new graph:high 10 19/20 The boost still does its job: `high` protects the boosted arm to rank 267 against an unboosted baseline of 150. Also surface the cut in the recall trace as a `rerank_prefilter` phase (kept/dropped, the cap in force, active boosts, per-arm composition of the survivors). The boosts previously reached only the server log, so a trace -- where you look when ranking seems wrong -- gave no hint a boost was applied. Note the cap is now caller-supplied and budget-resolved, so it can be below 300 on a low budget, which made the old behaviour strictly worse than the figures above. Claude-Session: https://claude.ai/code/session_018HDqrzHgqZqsGc7EDqoTEu * chore(docs): regenerate docs skill for the --shm-size=1g install snippet Pre-existing drift, not introduced here. `hindsight-docs/docs/developer/` gained `--shm-size=1g` on the `docker run` snippet, but the generated `skills/hindsight-docs/references/` copies were never regenerated. `verify-generated-files` does not run on main pushes, so the drift stayed invisible until a PR touching hindsight-docs/** made the job run and regenerate everything. Committing the generated output unblocks CI. Claude-Session: https://claude.ai/code/session_018HDqrzHgqZqsGc7EDqoTEu	2026-09-01 14:09:42 +02:00
Nicolò Boschi	b12b77dbfc	fix(ci): stop one stalled Gemini call from outliving the caller, and isolate the third claim-test file (#3982) ... * fix(ci): stop one stalled Gemini call from outliving the caller, and isolate the third claim-test file CI has been red on every branch since this morning, in four jobs with three independent causes. None of them is a product bug on the branch that trips them. 1. test-typescript-client, test-python-client(-oracle), test-doc-examples (cli) — all reflect, all the same shape: ERROR gemini_llm - Unexpected error during Gemini tool call: TimeoutError: WARNING reflect.agent - [REFLECT ...] LLM error on iteration 2: (120002ms) INFO memory_engine - [REFLECT ...] Complete: 448 chars, 2 iterations | 121.981s Gemini sometimes accepts a request and never answers it. The per-request deadline is the only thing that ends such a call, and the abort was terminal: TimeoutError fell through to the generic handler, which re-raised it, so the caller paid the whole deadline AND got an error — reflect then answered from a degraded forced pass. These stalls are not new. What changed is what they cost: #3946 made Gemini honour the configured llm_timeout instead of its hardcoded 90s, so the same stall went from 90s to 120s. Yesterday's green run has one at 83.994s, comfortably inside the 120s these suites allow a test; today's has 121.981s, just outside it. Two halves, at the layers they belong to: * A deadline abort is now retried exactly once, in both Gemini call paths. Nothing came back, so nothing about the request is implicated, and on a healthy provider the retry lands in well under a second. One retry only — tracked separately from the API-error ladder — because a second stall says something about the provider, not about this request. * Reflect gets its own per-request deadline default, 60s (DEFAULT_REFLECT_LLM_TIMEOUT). It is the one interactive operation: a caller holds an HTTP request open while it makes several sequential LLM calls, so a per-call deadline equal to the whole global budget lets ONE stalled call outlive the caller. Retain and consolidation keep 120s — they run in the background against a queue, where the deadline is there to stop runaway generation. An explicit HINDSIGHT_API_LLM_TIMEOUT is still inherited: an operator who set a global deadline meant it, and being quietly capped below it would be the more surprising of the two behaviours. 2. test-api (3/3) — "expected only the oldest same-document retain, got []". tests/test_retain_document_serialization.py claims queue-wide against the shared public schema, so another xdist worker's poller can take its rows before it does; its own claim then comes back empty and reads as "the predicate excluded them". This is the third file with that defect: #3963 gave test_worker.py and test_claim_bank_serialization.py a private migrated schema for exactly this, and this one was missed. Same fix. Only the queue tests take `backend`; the end-to-end append tests build their own engine and are untouched. 3. Core LLM tests — two assertions that test something other than their property: * test_delta_editorial_fusion asserted `"keyword" in fused`. The refresh had kept every SEO rule and written them as "search terms" / "search intent": the guidance was there, only the token was missing. It now judges the fusion, per CLAUDE.md — the structural asserts (no duplicate paragraphs, based_on counts) stay direct. * test_refresh_with_tags_only_accesses_same_tagged_models required the refreshed content to mention four specific Alice details. It leaked nothing at all and was reported as a SECURITY VIOLATION for summarising her more briefly. Tag scoping guarantees that nothing outside the scope is reachable; which of Alice's own details get written up is the summariser's call. The absence half stays strict, the presence half is now loose. Tests: new tests/test_gemini_deadline_retry.py pins both call paths (retry once, then give up — not the whole ladder); TestReflectDeadlineDefault pins the three-way resolution; the existing per-op defaults test now says which operation gets which deadline instead of asserting they are uniform. Claude-Session: https://claude.ai/code/session_0134gt96Tyi2Yi55yDH5s4Rb * fix(ci): size the reflect deadline against the retry ladder, and rerun the model-behaviour job First CI run on this branch made the mechanism visible and showed both numbers were wrong. The retry works — the log has stalls answered in ~3s on the next attempt, with whole reflects landing at 64.0s and 69.9s where they would previously have been errors at 120s. But the stall RATE is far higher than assumed: 5 stalls in one doc-examples job, and 2 of those stalled again on the retry. 60s x 2 attempts is 120s, which is exactly the cliff the change set out to clear, so those two runs failed identically to before (121.904s, 121.437s). What has to fit inside the caller's patience is deadline x attempts, not one deadline. Sized as such: - DEFAULT_REFLECT_LLM_TIMEOUT 60s -> 30s, and _TIMEOUT_RETRIES = 2, so the worst case is 3 x 30 = 90s. Healthy reflect calls in these logs answer in 1-4s, so the headroom is still an order of magnitude, and a third consecutive stall is ~1 in 60 at the observed rate rather than the ~1 in 16 that a single retry left. Second: `Core LLM tests` fails on a DIFFERENT test most runs — five distinct ones across the two runs of this branch (a judge verdict on emotional dimension, whether the agent called done() rather than being forced, a ReflectNoAnswerError, plus the two fixed in the previous commit). Every test in that job asserts what a model CHOSE to do, so one run cannot separate a regression from the model landing differently, and a red there currently carries no signal. It now runs with --reruns 2: a real regression reproduces and stays red, a coin-flip settles. Deliberately not applied to the deterministic shards, where a rerun would hide the races those tests exist to catch. Claude-Session: https://claude.ai/code/session_0134gt96Tyi2Yi55yDH5s4Rb * fix(tests): state the emotional-dimension threshold before the checklist Last CI run left one failure, and the reruns made it diagnosable: three attempts, all failing the same way, so not a coin flip. It is a false negative. Extraction produced "Sarah seemed disappointed upon hearing about the delay" and "Marcus felt anxious about the upcoming interview" — two of the three emotional states, which is exactly what the criteria says it takes to pass — and 3/3 judges still voted not-met, reasoning that the response "omits the emotional states". Only the speaker's thrill was dropped. The wording invited it: three states listed first, "at least two of these should be present" trailing behind them. Judges anchor on the enumerated items and read the missing one as the answer. Now the threshold leads and the three are a checklist under it, with the failing condition spelled out (two or more stripped to the bare event). Claude-Session: https://claude.ai/code/session_0134gt96Tyi2Yi55yDH5s4Rb * fix(tests): judge the fact sentence, not the rendering that restates the bare event The previous commit's diagnosis was wrong. Reproduced locally against the real model and judge: the criteria wording was not the problem, the response shape was. Extraction is perfect. All three emotions survive: TestUser was thrilled about receiving positive feedback on their presentation. Sarah seemed disappointed upon hearing about the delay. Marcus felt anxious about the upcoming interview. But the test fed the judge each fact's full rendering, and every one of those ends with a trailing dimension that is a bare restatement of the event: ... was thrilled about ... | When: ... | Involving: TestUser | Received positive feedback on presentation. The judge reads the restatement as the evidence and answers accordingly — "stripped down the emotional states of 'thrilled' and 'disappointed' to the bare events, only retaining 'anxious'" — about a response that says "was thrilled" and "seemed disappointed" in as many words. The clause I added last commit ("FAIL only if two or more were stripped down to the bare event") gave that misreading something to match, so it made the failure more certain rather than less. The emotional dimension lives in the sentence, so judge the sentence: one per line, metadata dropped. The context block goes too — it restated the same three emotions, leaving the judge holding them once as background and once as the thing to look for, which is the shape build_judge_messages already documents as blurring the two. Verified locally against the real pipeline: 4/4 passes, where the previous wording failed 3/3 in CI and on the first local run. Claude-Session: https://claude.ai/code/session_0134gt96Tyi2Yi55yDH5s4Rb	2026-09-01 13:59:39 +02:00
Nicolò Boschi	d936d4931c	feat(webhooks): emit X-Hub-Signature-256 and a timestamped signature (#3986) ... Webhook deliveries signed only `X-Hindsight-Signature`, a vendor name for a construction that is byte-for-byte the one GitHub popularised: `sha256=<hex>` HMAC-SHA256 over the raw body. Every receiver therefore needed a Hindsight-specific shim to verify a signature it already knew how to check. Emit `X-Hub-Signature-256` alongside it, carrying the identical value. Same secret, same algorithm, same bytes, so duplicating it grants no new capability to an attacker, and existing consumers of `X-Hindsight-Signature` keep working. Preferred over a per-webhook configurable header name: that would add a config field (plus migration, API, control plane, clients, CLI coverage, docs) to let users type the one string this already sends, and would leave every SDK verifier asking which header the sender was configured for. Two adjacent gaps found while in here: - The body-only signature has no notion of freshness, so a delivery captured off the wire stays verifiable forever. Add `X-Hindsight-Signature-V2` (`t=<unix>,v1=<hex>` over `<t>.<raw body>`, Stripe-style), signed at attempt time so retries re-sign. The timestamp is inside the MAC, so receivers can trust it and reject anything outside a tolerance window. The existing headers keep their body-only meaning — `X-Hub-Signature-256` is body-only by convention and must not be redefined. - `http_config.headers` was spread *after* `X-Hindsight-Event`, so a webhook's custom headers could overwrite the event type a receiver keys off. Spread user headers first and set the Hindsight-controlled headers after, so neither the event type nor any signature can be clobbered. Content-Type stays overridable (some receivers insist on a vendor media type; the body is JSON regardless). Document the whole header set with a verification example, which the webhooks page previously did not cover at all. The two unrelated `skills/hindsight-docs/` hunks are pre-existing generated drift from #3896, picked up by re-running generate-docs-skill.sh. Closes #3207	2026-09-01 12:44:36 +02:00
Nicolò Boschi	c42e6323e9	feat(llm): per-provider Codex credentials directory (#3793) (#3983) ... * feat(llm): per-provider Codex credentials directory (#3793) Codex auth resolves `auth.json` from the process-wide `CODEX_HOME`, so every `openai-codex` provider a Hindsight process builds reads the same store. A multi-LLM chain of two Codex members therefore authenticates twice as the same ChatGPT account: when the preferred profile hits its usage limit, failover just retries it. There was no way to express "prefer profile A, fall back to profile B". Add `codex_home` alongside the existing per-member provider settings (`VERTEXAI_*`, `LITELLMROUTER_CONFIG`), which is all this needs — the routing half of #3793 already exists: - `HINDSIGHT_API_LLM_CODEX_HOME` for the primary - `HINDSIGHT_API_LLM_<n>_CODEX_HOME` for indexed members (also under the `RETAIN_` / `REFLECT_` / `CONSOLIDATION_` prefixes) Each falls back to `CODEX_HOME`, then `~/.codex`, so nothing changes for existing deployments. Refresh is already coordinated per auth-file path (`_path_scoped_lock`), so two profiles refresh independently and cannot overwrite each other's tokens. The field is server-level only, deliberately not in `_CONFIGURABLE_FIELDS`: it is a filesystem path to a credential store, and accepting it over the bank config API would let a bank point the server at an arbitrary file. Scope: this is the credential-store seam only. Failover keeps its existing generic semantics — any `Exception` from a member advances to the next, with no quota-vs-terminal classification and no cooldown, so a rate-limited primary is re-tried at the head of every request before the fallback serves it. Batch retain already stays pinned to the member that submitted it, so batch affinity holds. The `openai-codex` embeddings provider still reads `CODEX_HOME` only — it has no member chain to span. Closes #3793 Claude-Session: https://claude.ai/code/session_011KDT484YujNcBxHzbfzNbk * fix(llm): append codex_home instead of inserting it mid-signature `create_llm_provider` and `LLMProvider.__init__` still have callers that pass the older settings positionally, and two guard tests in test_llm_wrapper.py assert exactly that. Inserting `codex_home` before `vertexai_project_id` pushed every later parameter one slot along, so a positional `timeout` landed on `cache_affinity` (`assert 120.0 == 7.5` / `assert None == 7.5`). Move it to the end of both signatures and leave a comment saying why new parameters go there, since the mistake is invisible at the call sites that use keywords. Claude-Session: https://claude.ai/code/session_011KDT484YujNcBxHzbfzNbk * chore(docs): regenerate docs skill for the --shm-size docker examples Drift inherited from main: the `--shm-size=1g` flag was added to the docker run examples in installation/quickstart without re-running the skill generator, so verify-generated-files is red on every PR branched after it. Regenerated output only — no source docs changed here. Claude-Session: https://claude.ai/code/session_011KDT484YujNcBxHzbfzNbk	2026-09-01 12:43:58 +02:00
Nicolò Boschi	b75e941916	fix(worker): rotate slots across banks so bulk ingest stops starving them (#3861) (#3980) ... * fix(worker): rotate slots across banks so bulk ingest stops starving them (#3861) Claiming was a strict global FIFO on created_at, so a bank under sustained bulk ingest held every worker slot for as long as its queue lasted. Measured on a six-bank instance: one bulk bank owned 17 of 17 retain slots in 90.6% of daily samples, and a write to a bank with an empty queue timed out at 300s behind the backlog. The queue drains correctly once ingest stops — this is fairness, not correctness. Deficit round robin, with a quantum of one slot. Every operation costs exactly one slot, so DRR's deficit counter is always zero and drops out; what is left is the round-robin walk. The poller keeps a cursor over the bank id space per schema — one level below the tenant rotation it already had — and the claim takes one row for the first bank sorting after it. Both tiers are one statement: a `rot` CTE (bounded index seek past the cursor) unioned with the `fifo` claim that was always there, joined back and locked with FOR UPDATE OF ... SKIP LOCKED. Same query count as before. 0.10ms against 0.02ms for the bare FIFO claim, at 50k pending rows. The cursor is a *range*, not a set of known banks: the starved bank is by definition one this worker has never claimed for, so only a range can discover it. And `fifo` is what makes the rotation safe at both ends — it is the wrap (once the cursor passes the last bank, `rot` matches nothing and `fifo` claims the whole pool, so the end of a round costs neither a query nor an empty claim, and a single-bank deployment sits in that state permanently), and it is what keeps this work-conserving: a bank alone with work still takes every slot, so nothing is throttled and no slot is held open for an idle bank. Rejected along the way, both measured: an ordering predicate that re-scanned the bank's queue per candidate row (11s at 10k pending), and a separate seek before the claim (correct, but a round trip on every claim, for every schema, on every poll). No new index — `idx_async_operations_bank_status` and `idx_async_operations_bank_created_desc` already serve both branches. Oracle keeps the plain FIFO claim, deliberately: its ROWNUM rewrite for FOR UPDATE + LIMIT applies before ORDER BY, so a rotation there would land on whichever bank is scanned first — under bulk ingest, the one it exists to rotate away from. claim_tasks now returns ClaimedOperations (rows + the rotation's next cursor) rather than a bare row list, so learning where the rotation got to costs no second statement; callers updated. Claude-Session: https://claude.ai/code/session_017ufCz6qrNxn36Stug7ek8A * chore(docs): regenerate the docs skill for the worker-tuning paragraph hindsight-docs/docs/developer/api/operations.mdx is the source; the skill reference is generated from it by scripts/generate-docs-skill.sh, and verify-generated-files fails on the drift. Claude-Session: https://claude.ai/code/session_017ufCz6qrNxn36Stug7ek8A	2026-09-01 12:08:09 +02:00
Ashley Unitt	88f1472e52	docs(docker): reserve shared memory for embedded PostgreSQL (#3896) ... * docs(docker): reserve shared memory for embedded postgres * docs(docker): keep shared memory guidance concise	2026-09-01 12:03:16 +02:00
Nicolò Boschi	c507e70e34	fix(mental-models): skip the reflect loop when the scope holds nothing to read (#3875) (#3943) ... A refresh with nothing in scope is the reflect agent's worst case, not a cheap one. The forced retrieval turns all come back empty, and the evidence guardrail then refuses every `done` call — evidence is exactly what cannot be gathered — so the loop runs to its iteration limit and pays a forced synthesis on top. Creating a knowledge page enqueues its refresh immediately, so a bank created with its default pages spent its whole LLM budget on five worst-case reflects over an empty graph: the budget it needed to ingest the content those pages were waiting for. Ask first whether the model's own flags leave anything to retrieve. The check reads the resolved scope, not the bank: tags/tags_match, tag_groups and fact_types bound which memory units the agent's tools can return, and the window bounds both retrieval tools — open in full mode, the watermark window in delta mode, using the same `updated_at` predicates as the recall arms. With `exclude_mental_models` off, a sibling document with real content is a source (`search_mental_models` applies no time bound, so that holds in delta mode too); one still holding the `Generating content...` placeholder is not. It costs nothing on a refresh that goes on to reflect: the check decides on the `MAX(updated_at)` the refresh already runs for its watermark. That reading now travels out unclamped (`_MentalModelScopeWatermark`), because the clamp that stops a watermark regressing destroys precisely what the check needs. Only a scope with no readable memory pays a query, and only while sibling documents are in reach. The reflect call is gated rather than the function returning early, so the delta legs still run — a retraction is a reason to edit the document by itself. Full mode returns `content_preserved_no_new_facts` before the empty-candidate guard, which raises and would make the worker retry identical inputs; reusing the outcome the delta leg already reports for an empty window keeps this off the API surface.	2026-09-01 11:51:57 +02:00
Nicolò Boschi	f22c36250f	feat(reflect): send the operation schema on mental model delta refresh (#3937) ... * feat(reflect): send the operation schema on mental model delta refresh The delta call was the one pipeline call that asked for structured output without sending a schema. Retain's extraction passes `response_format` and a per-operation `strict_schema`; the delta call passed neither, so the prompt was the only description of the payload the model got — and #3901 was the predictable result, a model spelling new blocks as `{"id", "text"}` because every block in the document it was shown carries an id. The reason was real, not an oversight. Pydantic renders the eight-op discriminated union as `oneOf` + `discriminator`. OpenAI's strict subset accepts `anyOf` and rejects `oneOf`, and the Gemini SDK refuses both keys outright — `types.Schema` raises `Extra inputs are not permitted` while building the request, before anything is sent. So no schema could travel, and the call hand-parsed its JSON instead. Fix the serialization rather than the call site. `UnionSafeSchemaGenerator` renders a tagged union as `anyOf` and drops the discriminator block, which costs nothing: each variant keeps its `Literal` `op` field, so the variants stay mutually exclusive and the discriminator was only ever a routing hint. `OpenAIStrictSchemaGenerator` extends it, so the strict and soft paths agree. Every site that serializes a schema *into a request* now goes through `provider_json_schema()` — all nine providers plus retain's batch path. That is safe to apply that broadly because the output is byte-identical for a model with no tagged union, which is pinned by a test rather than asserted here. Gemini still hands the model class to its SDK for every schema the SDK already converts, and serializes by hand only for a union it cannot accept. `gemini_cache` is left alone: it hashes the schema for a cache key and never sends it. The delta call then follows retain exactly: `response_format`, `strict_schema` from `llm_strict_schema_reflect`, and `skip_validation=True` so the raw JSON still reaches `parse_delta_operation_list` — which drops one malformed op instead of failing the batch the way `model_validate` would. The lenient parser stays in front of the schema, the same way `_coerce_fact_response` sits in front of retain's. Note the soft path (strict off, the default) now appends the schema to the system prompt for this call: better instruction, slightly more prompt tokens. Tests: the union rewrite (both serializers, variants preserved), the no-op property for non-union models, and the Gemini SDK's rejection of the raw union — asserted as a live expectation so the hand-serialization branch gets deleted if the SDK ever grows support. Plus a plumbing test that the delta call sends the schema, which the `patch_llm_call` fixture had been documenting as true since before it was. Claude-Session: https://claude.ai/code/session_011n8KQc8sCe4cJJCd8Cv69n * ci: run the real-LLM Gemini evals in the core LLM job `HINDSIGHT_RUN_GEMINI_EVALS` gates four test files' real-provider evals, and nothing in CI ever set it. `GEMINI_API_KEY` was already on the core LLM job, so the gate looked satisfied and the tests reported as SKIPPED rather than as missing — 11 of them, including the whole `TestDeltaRefreshGeminiEval` class. That left the mental-model delta path with no end-to-end coverage anywhere: the one call that hands a provider a schema the provider has to accept, and the exact path this branch changes. The gap surfaced while verifying the structured-output change — the test named as its safety net turned out never to run. Unskips 11 tests in the job that already has the credentials and the `hs_llm_core` marker for them. Claude-Session: https://claude.ai/code/session_011n8KQc8sCe4cJJCd8Cv69n * fix(gemini): strip additionalProperties and require the union tag The newly-enabled Gemini evals caught two defects in the structured-output change on their first run — both invisible to every offline check. 1. Vertex rejected every delta request: 400 INVALID_ARGUMENT: Unknown name "additional_properties" at 'generation_config.response_schema': Cannot find field. Handing the SDK a dict is not the same as handing it the pydantic class. The class path drops keys the backend has no field for; the dict path maps them faithfully, so `extra="forbid"` on every op model arrived as `additionalProperties`, became `Schema.additional_properties`, and the request was refused. The SDK builds it without complaint, which is exactly why asserting on `t_schema` was not enough — the new test asserts on the serialized request instead. Stripping the key costs nothing: the parser still validates against the pydantic model, so no field can be invented. 2. `op` was not a required property. Pydantic omits it because each variant defaults it (`op: Literal["add_section"] = "add_section"`), and with the `discriminator` block gone, `anyOf` alone gives a reader nothing else to tell eight near-identical shapes apart. A grammar-constrained model that omitted the tag would emit an operation matching no variant — reproducing the validation failure this whole path exists to prevent. The union rewrite now requires the tag wherever it appears, which is the honest completion of dropping the discriminator rather than a separate concern. Neither reached a network call in local testing, and the first was live for one CI round-trip. Both now fail closed in unit tests. Claude-Session: https://claude.ai/code/session_011n8KQc8sCe4cJJCd8Cv69n * test(retractions): give the real-LLM fixture its Vertex credentials The retraction eval builds an LLMConfig from provider/api_key/base_url/model. Vertex AI authenticates by project + service account instead of an api_key, so that config raises before it can make a call: ValueError: HINDSIGHT_API_LLM_VERTEXAI_PROJECT_ID is required for Vertex AI provider. The fixture's own docstring warns that a skipped test rots — it was written for a key-based provider, and the CI job runs the evals under `vertexai`, which is the one case it did not cover. Nothing caught it because the test had never run anywhere. Pass the three Vertex fields through from config. Claude-Session: https://claude.ai/code/session_011n8KQc8sCe4cJJCd8Cv69n * test(retractions): the eval imported a function renamed out from under it `parse_markdown` became `split_markdown`; the rename missed this call because the only reference lives in a test that never ran, so nothing failed. Second layer of rot in the same test — the credentials gap hid this one until it was fixed. Checked the rest of its surface rather than fixing one line and paying another CI round-trip to find the next: `build_structured_retraction_prompt` still accepts every argument it passes and requires none it omits, and the other imports resolve. This was the only stale reference. Claude-Session: https://claude.ai/code/session_011n8KQc8sCe4cJJCd8Cv69n	2026-09-01 09:38:18 +02:00
Nicolò Boschi	7051c6e3b0	fix(embeddings): bound ONNX forward passes and import embed calls (#3891) (#3948) ... * fix(embeddings): bound ONNX forward passes and import embed calls (#3891) `OnnxEmbeddings.encode()` tokenized and ran `session.run()` over its entire input in one pass. It was the only provider that did: TEI (32), Cohere (96), OpenAI (100) and ZeroEntropy (100) all slice, and there was no `..._ONNX_BATCH_SIZE` to set. Since tokenization pads every text in a call up to the longest one in that same call, peak memory was `n_texts x max_seq_len x hidden` float32. Nothing above it bounded the input either: `_import_observations` embeds every observation in the bank in a single call, so a whole-bank import OOM-killed the process (43.3 GB RSS on a 4,138-observation bank) after the document and fact phases had already committed — leaving a bank with the right document and fact counts and zero observations. - `encode()` now runs `batch_size` texts per forward pass (`HINDSIGHT_API_EMBEDDINGS_ONNX_BATCH_SIZE`, default 32), packing similar-length texts together so one long text cannot pad a whole batch. Output is unchanged: both pooling modes mask padding, so batch composition cannot move a vector. - The `InferenceSession` is built with `enable_cpu_mem_arena = False` (`HINDSIGHT_API_EMBEDDINGS_ONNX_CPU_MEM_ARENA`, default off). The arena caches freed blocks and never returns them, so RSS held its high-water plateau for the life of the process. The reranker has disabled it since #1717. - The import phases sized by the bank rather than by a document — observations and mental models — embed in slices, so the bound holds for every provider. Reported with a full diagnosis in #3891, including the measurement that chunking costs no throughput (~31 min vs ~37 min to the OOM) and that the vectors are bit-identical. * test: make the ONNX batching tests observe order and padding width The fake session returned a constant vector, so neither the scatter-back after sorting nor the batch-invariance claim was actually exercised. The fakes now derive each vector from its own text's length and pad to the longest text in the call, so a misplaced result and a padding-dependent output both fail.	2026-09-01 09:34:03 +02:00
Nicolò Boschi	7729396e12	fix(llm): give every provider a real per-request deadline (#3898) (#3946) ... The Codex provider never read the configured LLM timeout. The factory did not pass one, and CodexLLM extends LLMInterface (not the LLMProvider base that assigns self.timeout), so the class had no timeout attribute at all -- the three call sites hardcoded httpx timeout=120.0. That literal is a per-socket-read timeout, and the body was fetched with a buffering client.post(), so a backend wedged into runaway generation reset it forever: one consolidation call was read for ~830 s (~12 MB of SSE deltas for a ~340-character answer) until the backend closed the connection, holding the reserved consolidation slot for the whole time. Three such stalls cost ~1.7 h on one bank. - LLMInterface now takes and stores `timeout`, so no provider can silently drop it, and the factory threads the resolved value to the five that were missing it: codex, gemini, anthropic, fireworks and llamacpp. - Codex reads the SSE body with `client.stream()` inside an `asyncio.timeout` that covers the request *and* the parse, so the configured timeout is a total deadline rather than an idle one, plus a body-size ceiling that abandons a fast runaway stream in seconds instead of buffering it. Both surface as CodexRunawayStreamError, an httpx.RequestError, so the existing retry/backoff path handles them unchanged. - Gemini's hardcoded 90 s and Anthropic's own 300 s default become the unconfigured fallbacks rather than the only values. Codex tests move onto a shared streaming stub since the provider no longer calls client.post(). The consolidation wall-clock ceiling the issue also asks for already landed in #3746 (unreleased); it is an idle ceiling defaulting to 7200 s, so it would not have ended an 830 s stall on its own. Claude-Session: https://claude.ai/code/session_018HDqrzHgqZqsGc7EDqoTEu	2026-09-01 09:15:06 +02:00
Nicolò Boschi	78d46a7181	fix(recall): honour min_scores.keyword on every text-search backend (#3882) (#3938) ... * fix(recall): honour min_scores.keyword on every text-search backend (#3882) `min_scores.keyword` was a no-op on four of the six text-search backends, including `native`, the default. A caller asking for `keyword >= 0.30` got rows scoring 0.2 back. `bm25_min_score` was added in #1947 as a VectorChord-specific gate: vchord's `<&>` operator ranks *every* document, so it needed the analogue of native tsvector's boolean `@@` match gate. Default 0, Oracle got it for symmetry, behaviour unchanged everywhere else — correct and complete for that purpose. #2422 then built the public `min_scores.keyword` floor on top of that same parameter and touched no file under `engine/sql/`. From `retrieval.py` the wiring looked finished, but only the vchord and Oracle branches ever read the value; `native`, `pg_textsearch`, `pgroonga` and `pg_search` accepted it and silently dropped it. An internal gate that defaults to off had been promoted to a public per-request floor without the backends being re-audited. - Push the floor into all six backends. pgroonga's `pgroonga_score()` and pg_search's `<schema>.score()` are only valid in the target list, and re-evaluating native's `ts_rank_cd` or pg_textsearch's `<@>` in WHERE would compute the score twice per row (and, for `<@>`, forfeit the index scan the ORDER BY relies on), so those four apply it by filtering the ordered LIMIT slice from the outside. Every arm orders by score DESC, so that keeps exactly the rows an inner predicate would. - Make the floor inclusive. `min_scores` is documented as inclusive and the semantic arm uses `>= min_similarity`, but vchord/Oracle used `>`. The new `bm25_score_gate()` helper resolves the overload: `> 0` at the 0.0 default (the structural match gate #1947 needed), `>=` once a caller sets a floor, which subsumes it. Default behaviour is byte-identical. The second half of #3882 is a documentation bug. "All inclusive, AND-ed" reads as a predicate over each returned result, but `semantic` and `keyword` prune only the arm they name: recall fuses four arms and returns what any of them surfaced, so a result may carry `null` for a stage that did not surface it, and graph/temporal results carry neither. That is deliberate — an intersection would discard the strong single-arm matches hybrid retrieval exists to find. Only `reranker` and `final` are per-result predicates, and they are what a caller wanting abstention should use. Note the two halves interact: once the pushdown is fixed, the union behaviour can only ever surface as a `null`, never as a below-floor number, because fusion copies `semantic` only from the semantic arm and `keyword` only from the BM25 arm. The reporter's `{"keyword": 0.2}` under a 0.30 floor was purely the pushdown bug. `MinScores`, the `RecallRequest` field, both MCP tool descriptions and the recall docs now say exactly that. Tests: `test_bm25_min_score_pushdown.py` asserts the floor reaches the SQL on all six backends, that it is inclusive, and that the 0.0 default is unchanged — SQL-shape assertions, so a new backend branch cannot repeat the omission on a machine with no vchord/pgroonga/pg_search/Oracle available. (`pg_search` gained a configurable function schema on main while this bug was open and would have inherited the same gap.) The backend list is hoisted out of `HindsightConfig.validate()` into `VALID_TEXT_SEARCH_EXTENSIONS` and the test parametrizes over it, so a sixth backend is covered the moment it becomes selectable rather than when someone remembers to update a second copy. Plus DB-level regression tests for the keyword floor and for the per-arm contract. Closes #3882 Claude-Session: https://claude.ai/code/session_011KDT484YujNcBxHzbfzNbk * fix(recall): inclusive keyword floor — update the vchord contract test, harden the regression test CI caught two things the local run could not. 1. `test_db_abstraction.py::test_build_bm25_arm_vchord_honors_custom_min_score` asserted `> 2.5`. That is the old exclusive gate this PR deliberately replaces, so the assertion is now `>= 2.5` with a comment recording why it changed. The test was doing its job; it encoded the behaviour the parameter had while it was vchord's internal match gate. 2. `test_keyword_floor_prunes_in_retrieval` asserted `len(kws) >= 2` so the floor would discriminate. On the three-fact corpus the keyword arm surfaces only one row for "animals", so the guard failed on its own precondition. Reworked to assert the contract without depending on corpus rank spread: a floor above every observed score must leave nothing keyword-scored (before the fix, native returned those rows with their real below-floor scores), and a floor at exactly the top score must keep that row (inclusivity, end to end). Neither a row count nor a score spread is something to assert on here — ranks can tie and the arm may surface a single row. Also: format the floor with `!r` rather than `:g`. `:g` truncates to six significant digits, so a caller echoing a `scores.keyword` value back as a floor could get a literal that rounds up past its own row and silently drops it — the exact round-trip the new inclusivity assertion exercises. Claude-Session: https://claude.ai/code/session_011KDT484YujNcBxHzbfzNbk * test(recall): drop the DB-level keyword-floor test; it asserts an environment property `test_keyword_floor_prunes_in_retrieval` failed in CI twice, on two different preconditions, for the same underlying reason: the BM25 arm surfaces nothing for this module's seeded fixture, so `scores.keyword` is `null` on every result and there is no floor to exercise. No other test in the repo asserts a non-null `scores.keyword`, so nothing else depends on that arm surfacing rows here. `search_vector` is a GENERATED ALWAYS column, so the fixture's raw INSERT does populate it — the cause is somewhere else in the test configuration and is worth a separate look, but it is not this fix. (The arm demonstrably works in a real deployment: the #3882 reporter's own responses carry keyword scores.) Deleted rather than skipped. The guard for this bug is `test_bm25_min_score_pushdown.py`, which asserts the floor reaches the SQL on all six backends deterministically and is what would have caught the original defect; a DB test that cannot observe a keyword score adds no coverage over it. Also fixes a vacuous assertion in `test_retrieval_floors_are_per_arm_not_per_result`: `any(keyword is None)` holds trivially when every keyword is None. It now asserts that not every result carries a score for both floored arms, which is the union property the test is named for. Claude-Session: https://claude.ai/code/session_011KDT484YujNcBxHzbfzNbk	2026-08-31 17:54:20 +02:00