mirror of
https://github.com/vectorize-io/hindsight.git
synced 2026-09-14 19:31:49 +08:00
c61c4e7d7d4296d9fa594bc2054419564d538672
2787 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Nicolò Boschi
|
c61c4e7d7d | release(coding-agents): v0.5.1 integrations/coding-agents/v0.5.1 | ||
|
Nicolò Boschi
|
0eb818d0ab |
fix(coding-agents): the runtime auto-update never fired — registry 406 on the /latest media type (#3997)
latestVersion asked /<pkg>/latest with accept: application/vnd.npm.install-v1+json, which the registry only serves for the packument — it answers 406 there. !r.ok returned "", indistinguishable from "no newer version", so the runtime silently never updated. Drop the header. The fetch stub now answers 406 to that media type like the real registry, which makes four existing tests fail on the 0.5.0 code. |
||
|
Nicolò Boschi
|
6da65bc194 |
feat(changelog): enumerate each release's migrations, with tables and volume (#3996)
Release notes said nothing about schema changes, so an operator had no way to tell from the changelog whether upgrading meant a long startup migration. Every release entry now ends with a "Database Migrations" section listing each Alembic revision added in the tag range, the tables it touches tagged with how much data they hold, and a link to the PR that introduced it. A release that alters a high-volume table also gets a warning line above the list. The section is enumerated from git (`--diff-filter=A` over the versions dir), never through the LLM: the same range must always produce the same list, and the volume of a table is a property of the schema rather than a per-run judgement, so it comes from a reviewed map that a test forces new tables into. Tables are read from Alembic ops and raw DDL/DML positions and intersected with that map, which drops prose and index expressions the loose SQL patterns pick up. Bare FROM/JOIN reads are ignored — a migration that defines a function reading memory_units neither rewrites nor locks it. DROP INDEX names no table, so the owner is recovered from the index identifier. Claude-Session: https://claude.ai/code/session_01J52s5Lbg5Lp7KwRyhqg8f3 |
||
|
Nicolò Boschi
|
208b8729b6 | release(coding-agents): v0.5.0 integrations/coding-agents/v0.5.0 | ||
|
Nicolò Boschi
|
71a9a7008f |
feat(coding-agents): first-class pi support, sharing one extension adapter and installer with Prime Agent (#3993)
Adds pi (@earendil-works/pi-coding-agent) as a harness and extracts createPiExtension(harness), the extension adapter pi and its fork Prime Agent share. Supersedes #3775. The `pi` key is removed from package.json: both hosts read that same key, so it could only ever name one bundle, and the host it did not name loaded the other's and reported the wrong harness. `hindsight-coding-agents install pi|prime-agent` is now the only route for both. Also makes the companion skill self-update for every host that installs one — the paths now live once in core/skill-dirs.ts, which both the installer and skill-sync read. Co-authored-by: Sebastian Otaegui <feniix@gmail.com> |
||
|
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.
|
||
|
Nicolò Boschi
|
833d400f9b |
chore(coding-agents): sync generated files after per_source (#3872) (#3994)
#3872 merged while `verify-generated-files` was still running, and that job failed ~40s after the merge. Two generation steps had not been run: - prettier reformats the widened `ObservationScopes` union onto one member per line (the `prettier-int-coding-agents` task in scripts/hooks/lint.sh). - `./scripts/generate-docs-skill.sh` emits a third copy of the coding-agents page under skills/hindsight-docs/references/, which needs the `per_source` section like the other two. CI only runs on `pull_request`, so this drift is invisible on main but fails `verify-generated-files` for the next PR that opens against it. No behaviour change: formatting plus a regenerated doc. Claude-Session: https://claude.ai/code/session_011n8KQc8sCe4cJJCd8Cv69n |
||
|
Shaun Eccles-Smith
|
247e9b3138 |
feat(coding-agents): add per_source observation scoping (#3872)
* feat(coding-agents): add per_source observation scoping On a repo worked by a coding agent, commit diffs and session transcripts make different kinds of claim. A diff records what the code does; a transcript records what someone intended, argued for, or discarded. Under the `shared` default both consolidate into one undifferentiated belief set, so an idea floated in chat and never implemented is indistinguishable from a belief derived from the commits, and "what does the codebase actually do" cannot be answered from commit-derived knowledge alone. This cannot be fixed by configuration. The server treats an explicit scope list as unconditional: `_resolve_obs_tags_list` and `_resolve_write_scopes` in the consolidator both return the parsed list verbatim, without filtering it against the memory's own tags. A configured `[[], ["source:git"], ["source:chat"]]` therefore writes EVERY document into all three scopes, and the `source:git` scope fills with observations built from chat transcripts. Only a per-document decision separates them. `per_source` is resolved client-side, per document, in the new exported `resolveRetainScopes`, and never reaches the server. It expands to the global scope plus one per distinct `source:` tag the document carries, sorted — a document with two source tags (the commit-message seed keeps `source:git` alongside `source:git-log` so the cold-repo check still sees it) gets a scope for each, which needs no arbitrary tie-break and does not depend on the order the caller assembled its tags in. Reading only `source:` is what keeps this safe. `per_tag` splits on the right axis but also on every other one: it would reinstate the per-agent `harness:` fork that #3564 and #3575 removed, and any volatile tag such as a session id from `retainTags` would become its own scope — the fragmentation bug itself. The empty scope is always emitted first and unchanged, so the merged view matches `shared` exactly and the untagged observations that knowledge pages read (`tags_match: "all"`, per #3664) are unaffected. The cost is honest: one extra consolidation pass per document. That is the price of the axis, and it is why this is opt-in — `DEFAULT_OBSERVATION_SCOPES` remains `shared` and no existing value changes meaning. Closes #3871 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdYchB9yKUWw8oa16RWQu9 * fix(coding-agents): not every source tag names a kind of claim Running per_source on real repositories showed two of the five source tags are provenance labels rather than axes, and each produced a scope worth less than it cost. source:git-log is a bookkeeping alias. git.ts tags the commit-message seed with it AND source:git, so emitting a scope for each gave two near-identical belief sets — 307 observations against 302 on one repo — and doubled the consolidation for that pair. It is the same claim as source |
||
|
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 |
||
|
Nicolò Boschi
|
c249d43796 |
coding-agents: support opencode 2, whose plugin API shares nothing with v1's (#3985)
opencode v2 (`@opencode-ai/cli@beta`, binary `opencode2`) installs alongside v1
and rewrote the plugin contract end to end: a v1 plugin is a function returning a
bag of named hooks, a v2 plugin is `{id, setup(ctx)}` where ctx hands out
per-domain registration. Handing either host the other's export loads a plugin
that registers nothing and reports no error, so v2 needs its own adapter — mapped
onto the same harness-agnostic RuntimeCore, which keeps recall, injection, the
native hindsight_* tools, the cold seed and write-back identical to every other
harness:
v1 chat.message -> ctx.session.hook("prompt")
v1 experimental.chat.system.transform -> ctx.session.hook("context")
v1 tool: {...} -> ctx.tool.transform(d => d.add(...))
v1 event (session.idle) -> ctx.event.subscribe()
v1 client.session.messages() -> ctx.session.context({sessionID})
Three host behaviours shaped the result, each verified against a live
opencode2 0.0.0-beta-18743:
ONE CONFIG ENTRY SERVES BOTH CLIS. v1 and v2 read the same
~/.config/opencode/opencode.json, and v1 REJECTS the whole file when it sees v2's
`plugins` key ("Configuration is invalid ... Unrecognized key: plugins") — so a
second entry is not an option. What makes one entry work is that they resolve a
plugin DIRECTORY differently: v1 follows package.json `main` (dist/index.js), v2
ignores `main` and loads <dir>/index.js. The package therefore ships a root
index.js re-exporting the v2 entry, and the path already in a user's `plugin`
array drives the right plugin under each host, with no config change.
THE EVENT STREAM IS GLOBAL; THE SESSION HOOKS ARE NOT. v2's background service
hosts every open project at once. `ctx.event.subscribe()` delivers session.idle
for sessions in OTHER projects, with no location on the envelope to tell them
apart, while prompt/context fire only for the location that loaded the instance.
Acting on those ids would fetch another project's transcript and retain it into
this project's bank, so write-back is gated on sessions our own prompt hook
admitted. This is a bank-isolation invariant, and it has a regression test.
TOOLS NEED options.codemode: false. Without it a tool is reachable only through
v2's `execute` code-execution tool, and a model calling it by name — which the
skill and the injected preamble both tell it to do — gets "Unknown tool:
hindsight_...".
Two capabilities are deliberately absent. The codebase survey gets no opencode2
recipe: v2 plugins cannot define an agent (agent.transform has no `add`) and a
config-file agent is not a sandbox either, since the user's own global
permissions are appended after an agent's rules — the survey reads untrusted repo
files, so without a guaranteed read-only boundary it falls back to another
installed agent's CLI, as it already does for kilo/cline/cursor/dcode. And there
is no toast: v2 plugins can observe tui.toast.show but not publish one, so the
seed banner is logged. v1's mid-session cadence write-back is also gone, because
v2 makes it redundant rather than because it was missed — session.execution.succeeded
fires after every assistant turn, so the idle path already runs once per turn
from the authoritative post-reply transcript.
Verified end to end against a real opencode2 and server: plugin loads reporting
harness opencode2, per-repo bank derived, cold seed and survey ran, the injected
block reached the model (it quoted it back verbatim), hindsight_ingest_document
was called by name, and session idle produced retain_ok. Also verified the staged
published-package shape loads under opencode2, and that opencode v1 still loads
dist/index.js and reports harness "opencode" with the new root index.js present.
Closes #3795
Claude-Session: https://claude.ai/code/session_01WbaYxouCERUv9djo3GnnA2
|
||
|
Nicolò Boschi
|
9bfad10a49 |
fix(reflect): name the sections array the document schema requires (#3984)
Document mode's output-format block spelled out every field of a *section* —
heading, level, blocks — and never named the `sections` array that holds them.
So the prose described a section while the tool schema described a document
containing sections, and a model resolving that disagreement in favour of the
prose emits the section as the document:
{"document": {"heading": "…", "level": 2, "blocks": ["…"]}}
`document_from_sections` reads `payload["sections"]`, finds nothing, and returns
an empty document. It renders to "", reflect raises ReflectNoAnswerError, and the
mental-model refresh discards the run and retries against the same prompt. Every
fact the model was asked for was present and correctly structured; only the
wrapper was missing. Observed from Gemini on the refresh path, where it fails the
whole refresh and burns the call again on retry.
Fixed at both ends, because they fail differently. The prompt now names the array
and shows the shape — this was the only output-format block in the file
describing a nested shape without an example of it, and an example is the part a
model copies. The parser takes a bare section as the single section it plainly
is: it is already documented as tolerant of model output, coercing a missing
heading, an out-of-range level and a non-string block, and a payload that IS one
section is the same class of near-miss — the one that currently costs the entire
refresh rather than one field of it.
Still not a licence to guess: a payload with neither `sections` nor `blocks`
stays empty, so an answer is never invented from something that is not a
document (#2959).
Verified against real Gemini locally: 12 document-mode refresh runs (6 on each
prompt) plus the new hs_llm_core reflect test, all green. The flattening did not
reproduce in that window — it is rare, and these runs are far too few to measure
a change in its rate, so the prompt half is argued from the text, not from them.
What they do show is that the corrected prompt and the normalisation break
nothing a real model does. The parser half is pinned by a unit test carrying the
exact payload captured from the failing run.
Claude-Session: https://claude.ai/code/session_01UBcDzagMhuXsYZDp63i7aB
|
||
|
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
|
||
|
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 |
||
|
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 |
||
|
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 |
||
|
Altay
|
173ed6a93c | fix(coding-agents): annotate MCP tool safety (#3818) | ||
|
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 |
||
|
MasterST1337
|
5ceca53e93 |
fix(coding-agents): survey prompt says Glob, not Read, for the directory layout (#3796)
The cold-repo survey spawns a headless `claude -p --model haiku --allowedTools Read Glob Grep` session. SURVEY_PROMPT asks the model to understand "the directory layout" but never says which tool shows it, and haiku reaches for `Read(<repoRoot>)` — a bare directory path, which errors with EISDIR before the survey has read anything. Measured on one machine's OTel export, 2026-08-23..25: 10 distinct survey sessions failed this way on first tool call, across kids-yt-factory, hindsight, sitebrain.eu, PlanView, Meridian and four .traycer worktrees — i.e. every repo the survey ran cold against, plus a fresh worktree each time (a new worktree is a new bank, so the survey re-runs). Glob was already in --allowedTools; the model was simply never told to prefer it here. This is a prompt-level counter-instruction, not a structural guarantee — the robust fix is at the tool-wiring layer (deny Read on a directory, or give the survey an ls-shaped tool), which this does not attempt. |
||
|
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. |
||
|
Nicolò Boschi
|
aabbe8b225 |
fix(retain): stop deriving the extraction narrator from the bank's display name (#3962) (#3978)
* fix(retain): stop deriving the extraction narrator from the bank's display name (#3962) Retain primed fact extraction with a "Narrator: {name}" line built from `banks.name`. That line tells the model first-person statements are the narrator's own actions, and the name it carries is stamped into the who-dimension of every fact it produces — and into the observations later consolidated from those facts. But `name` is a display label. Nothing reads it except the bank selector and the `?q=` filter, the update-profile field already documents it as "display label only, not advertised", and the control plane has no UI to set it — so callers set it through the API/CLI to whatever identifies the bank to them. A project label like "AuditProject_0825" then appears verbatim in memories that never mentioned it: Dispatch has arranged a truck ... | Involving: AuditProject_0825 (dispatch), user #1680 already hit the same edge from the other side and suppressed the narrator when `name` defaulted to the bank_id (typically a routing key). The underlying coupling was the bug: retain now passes no narrator at all. A caller who genuinely wants to name the speaker says so in the item's `context`, which extraction already reads and prefers over the narrator — the same channel the dry-run endpoint's deprecated `agent_name` override already points people to. Changes: - `retain_batch` no longer resolves a narrator, so `_resolve_narrator`, the `_NARRATOR_UNRESOLVED` sentinel and the `agent_name` plumbing through `_streaming_retain_batch` / `_try_delta_retain` / `_extract_and_embed` are gone. This also drops a bank-profile read (and its pooled connection) from every retain. - `extract_dry_run` mirrors retain: no narrator unless the caller passes one. The deprecated `agent_name` request field still overrides it, so the HTTP API is unchanged (no schema change, no client regeneration). - `extract_facts_from_text` keeps its `agent_name` parameter — it is the dry-run override and the Narrator line still works when asked for. Tests: `test_narrator_resolution.py` keeps the prompt-assembly unit tests and replaces the `_resolve_narrator` cases with a retain-level regression test — a bank named "AuditProject_0825", a spy on the narrator handed to extraction, and an assertion that the name is absent from stored units. Verified it fails when a narrator is reintroduced. Closes #3962 Claude-Session: https://claude.ai/code/session_0134gt96Tyi2Yi55yDH5s4Rb * refactor(retain): drop agent_name from the contents-level extraction API Follow-up to the previous commit, which left `extract_facts_from_contents` taking a narrator that its only caller now passes as a bare `None`. A parameter no caller can populate is worse than none: it reads as a supported knob and invites someone to wire the bank name back into it. - `extract_facts_from_contents` and `extract_facts_from_contents_batch_api` no longer take `agent_name`. The retain orchestrator calls them without it. - `extract_facts_from_text` keeps the parameter — it is the dry-run endpoint's deprecated override — but it moves after `config` and defaults to `None`, so callers that don't want a narrator simply omit it. Every caller already passed it by keyword, so the reorder is safe. - `_extract_facts_from_chunk` / `_extract_facts_with_auto_split` had `agent_name: str = None`, which was never a valid annotation; now `str | None = None`. - `extract_dry_run` passes `agent_name` straight through instead of coercing `None` to `""`. The #3962 regression test spies on `_build_user_message` instead of on an `agent_name` argument that no longer exists, and asserts no prompt retain builds carries a `Narrator:` line or the bank's display name. Re-verified it fails when a narrator is reintroduced. Claude-Session: https://claude.ai/code/session_0134gt96Tyi2Yi55yDH5s4Rb * fix(tests): drop agent_name from the direct _streaming_retain_batch call test_consumer_failure_cancels_in_flight_extractions calls the internal helper directly, so it kept passing a keyword the previous commit removed: TypeError: _streaming_retain_batch() got an unexpected keyword argument 'agent_name'. Caught by test-api shard 3. Claude-Session: https://claude.ai/code/session_0134gt96Tyi2Yi55yDH5s4Rb |
||
|
Sanderhoff-alt
|
17fbc4f147 |
fix(bank-template): decide template-import writes from state read inside the authorization scope (#3957) (#3971)
Template import classified each imported mental model and directive as a create or an update from a snapshot taken before authorization and bank provisioning, then wrote against that stale classification: a concurrent create turned the import's create into a unique violation, and a concurrent delete turned its update into zero rows updated and a failing refresh. Re-read the committed state inside the authorization scope, after _ensure_bank_exists and the server default template, immediately before the writes, and decide against that. This subsumes the narrower re-read that only covered resources projected to collide with the default template. The preauthorization still names one operation per resource, from the earlier snapshot. When the fresh read flips a resource, the flipped operation gets its own decision through MemoryEngine.authorize_bank_template_import_write rather than a blanket grant for both outcomes: validate_bank_write is a per-operation hook that may reserve quota, so an update-only import must not be charged for creations it never performs, and must not be rejected by a validator that denies create. The default-template projection keeps its dual authorization -- that collision is deterministic, and authorizing both ahead of provisioning keeps every client check before bank creation. Fixes #3957 Co-authored-by: Nicolò Boschi <boschi1997@gmail.com> https://claude.ai/code/session_01DNWFQn8AUN6SApcswHG3aN |
||
|
Nicolò Boschi
|
db87a74675 |
fix(structured-output): strip $ref sibling keywords from strict schemas (#3944)
Retain failed with HTTP 400 from OpenAI-compatible providers whenever a bank
configured entity labels and strict schema was on:
Invalid schema for response_format 'response':
context=('properties', 'labels'), $ref cannot have keywords {'description'}
The dynamic LabelsFact model gives its `labels` field both a nested model and a
Field(description=...), which pydantic serializes as
{"$ref": "#/$defs/Labels", "description": ...}. JSON Schema 2020-12 allows
sibling keywords next to a $ref; OpenAI's strict subset does not, so the request
was rejected before inference and the API returned 500 "Fact extraction failed".
Normalize in strict_json_schema() rather than at the call site: a $ref node now
keeps only $ref, wherever one is generated. That covers all five providers that
serialize through it (openai_compatible, openai_responses, litellm, codex,
xai_oauth) and any future nested model with a described field. The traversal
special-cases $defs/properties/patternProperties so a property literally named
"$ref" is not mistaken for a reference node. Only annotation keywords are ever
dropped — nullable and list-wrapped fields keep their descriptions on the
anyOf/items wrapper, which the pass leaves alone.
Tests: a unit regression asserting no $ref node has siblings, plus an hs_llm_mat
end-to-end retain into a labelled bank with strict schema on. Marked mat rather
than core because the core job runs vertexai/gemini, whose provider never calls
strict_json_schema() — which is why the existing entity-label LLM tests never
caught this. Verified against real OpenAI gpt-4.1-nano both ways: reverting the
fix reproduces the production error verbatim.
Fixes #3904
|
||
|
Ferran Vidal
|
eff3546ecd |
fix(consolidation): resolve observation_scopes before tag-set batching (#3954)
Fixes #3953. `run_consolidation_job` grouped memories into LLM batches by their raw native tag set, then resolved the observation scope for the whole batch once, from `sub_batch[0]`. Two consequences, both verified against main: - `observation_scopes="shared"` facts carrying different native tags were split into separate LLM calls and could never be merged into one shared observation — the issue as filed. - Worse, and not in the issue: memories with the *same* tags but *different* `observation_scopes` landed in one group, so `sub_batch[0]` decided for all of them. With a `shared` memory first, a default `combined` fact tagged `user:alice` produced an untagged, globally recallable observation; with the `combined` one first, the `shared` override was silently dropped. `observation_scopes` is settable per retain item, so mixing is reachable. The fix groups on the resolved target scope rather than native tags, keying both falsy resolutions (`combined`, and a degenerate explicit `[]`) the same way the pass loop treats them. `_resolve_write_scopes` now reports the combined fallback scope for an empty explicit list too — it previously returned `[]`, so such a group took no lock at all for the scope it writes. Defence in depth: the sub-batch loop derives, from the pass loop rather than from the grouping key, the scopes each member will actually be written at, and splits the batch (with an error log) if they disagree. Correctness no longer depends on the grouping key being right — a future regression costs an extra LLM call instead of leaking an observation across scopes. Coverage, each part checked by reverting it: an exhaustive no-DB invariant over tag sets crossed with every `observation_scopes` shape ("equal batch key implies equal written scopes"), which fails on 13 cases against the old native-tag key; the signature pinned literally per mode; and end-to-end tests for the pooling fix, the empty-list case, and the runtime split under a deliberately broken key. Full CI dispatched on this SHA (fork PRs skip test-api): all three test-api shards green with the new tests executed. The three red jobs — Core LLM `test_delta_fuses_seo_and_brand_voice`, python-client reflect timeouts, and doc-examples "Cannot connect to Hindsight API" — fail identically on unrelated branches in the same window and are not caused by this change. |
||
|
Nicolò Boschi
|
c23e98856b |
fix(coding-agents): stop a failed git probe from forking a worktree into its own bank (#3981)
A transient failure of the single `git rev-parse --git-common-dir` probe silently changed a repository's bank identity: `getProjectRootFromGit` mapped every failure to `null`, indistinguishable from "not a git repository", so `gitProjectName` took the basename fallback and a linked worktree was retained into a brand-new bank — permanently, with no log line, no diag event and no retry. One repository in the wild accumulated eight stray `coding-agent::<repo>-wtN` banks this way. Both triggers were load-dependent and silent: the 1000 ms timeout elapsing under machine load, and `execFileSync` failing to spawn at all (EAGAIN under process pressure). Read the repository layout instead of spawning git (core/git-layout.ts). `.git` is either the git directory or a one-line pointer to it, and a linked worktree's git directory names its repository in `commondir` — so the answer is a handful of fs reads against a format git itself guarantees, with no subprocess, no timeout and no spawn to fail. A pure-JS git library was considered and rejected: isomorphic-git models objects and refs, not worktree discovery, so it answers a different question at a much larger cost in a package bundled into every hook process. On top of that, the fix the issue asks for: - the probe distinguishes "not a repository" from "could not tell", and only the former reaches the basename fallback; - transient errors retry with backoff before the probe gives up; - a failed probe never guesses: resolution throws BankResolutionError and every entrypoint goes through `deriveBankIdOrSkip`, so the lifecycle hooks skip the session (recoverable) rather than scatter it (not); - the skip logs a warn and a `bank_unresolved` diag event. Also rejects a common dir that no longer exists, so a pruned worktree can no longer be named after its own dangling `.git/worktrees/<name>` — the same wrong id by a different route. Tests cover real repos/worktrees/bare hubs (including resolution with PATH emptied, proving nothing spawns), the retry and failure classification, the refusal to guess plus its diag event, and a family guard asserting no module outside core/bank.ts calls the throwing form directly. Fixes #3950 Claude-Session: https://claude.ai/code/session_011n8KQc8sCe4cJJCd8Cv69n |
||
|
Nicolò Boschi
|
97ae1d08c5 |
metrics: time the operation-validator hooks, which run outside the operation's timer (#3976)
* metrics: time every operation-validator hook, by wrapping the extension once A recall's `[phases]` line accounts for its whole duration -- `accounted=427ms of 433ms` is typical -- and that is what made this hard to see: the line measures the INNER search (`_search_with_retries`, which owns `recall_start`), while `validate_recall` and `on_recall_complete` run in `recall_async` on either side of it. Everything the validator does falls outside the number, so an instrument that reads as complete covers a fraction of the request. It is not a hypothetical gap. The credits validator reaches the CONTROL database on both hooks, uncached: an org row and the whole `billing_config` table before the query, the same two again after, then the balance UPDATE and its ledger INSERT. Five queries on a `max_size=10` pool -- the pool auth shares and caches for exactly this reason -- all awaited inline, none of it visible. **Wrapped once where the engine takes the extension, not timed at each call site.** The interface has nineteen hooks and the engine calls them from many places; hand-instrumenting the two that prompted this leaves the other seventeen dark and guarantees the twentieth is added without timing, which is how the gap appeared in the first place. Wrapping the instance covers a new hook the day it exists. Hooks are recognised by shape rather than listed, for the same reason. Wrapping is applied to the INSTANCE, so `isinstance`, attribute access and non-hook methods are unchanged; idempotence is tracked in a WeakSet rather than a flag on the extension, because the extension is someone else's object and should not carry an attribute its own code can trip over; and an OVERRIDE is wrapped rather than the default it replaced, which matters because the overridden hook is the one working. Timed in a `finally`, so a REJECTING hook is measured too -- that is the 402 path, and it is the whole cost the caller pays. A metrics backend that is down cannot fail the request it measures. `test_every_hook_on_the_interface_is_instrumented` defines "hook" independently of the instrumentation's own predicate. Asking that predicate would be circular: narrowing it narrows the expectation with it, so the test passes while coverage shrinks. It did exactly that until rewritten -- verified by mutation, which now names all nine hooks it would have missed. Claude-Session: https://claude.ai/code/session_01V3ViCQDM6pPSCfntZam3Kg * metrics: make the wrapping fail-safe, and time it on a monotonic clock Two follow-ups on the same principle the module already states for the recording side -- instrumentation must never be the thing that breaks what it measures -- applied to the wrapping itself. The extension is loaded from an env var and is someone else's class, so an unhashable, non-weak-referenceable or slotted one now degrades to "not timed" instead of raising inside the engine constructor. Durations move from `time.time()` to `time.perf_counter()`: a wall clock that steps backwards mid-hook records a negative duration into the histogram, and the hooks being measured are short enough for that to matter. `_is_hook` also skips an already-wrapped method, so idempotence survives a validator the WeakSet could not track. Claude-Session: https://claude.ai/code/session_012gXUp1i7YrWmLJVUYki53g |
||
|
Nicolò Boschi
|
1aef228f7b |
feat(coding-agents): add qwen-code as a hook harness (#3979)
* feat(coding-agents): add qwen-code as a hook harness
Qwen Code speaks Claude Code's hook protocol field for field -- same stdin
envelope (session_id / transcript_path / cwd / hook_event_name), same
hookSpecificOutput.additionalContext + systemMessage output, same exit
semantics, and a settings.json shape byte-for-byte what cmdHook() already
emits. So HOOK_HARNESSES["qwen-code"] is claude-code's spec with three deltas,
none of which is visible from the diff:
1. TIMEOUTS ARE MILLISECONDS. Qwen passes a command hook's `timeout` straight
to setTimeout ("Timeout in milliseconds, default 60000" -- its own bundled
docs/features/hooks.md). Every other harness here is seconds, so the
installed values are 30000/30000/60000. Writing 30/60 would register 30ms
hooks, and that misconfiguration LOOKS fine in testing: Qwen spawns without
detached:true and kills only the direct child, so the orphaned work still
completes. HookHarnessSpec therefore declares `timeoutUnit`, and the
lifecycle tests normalise through it -- changing 30_000 to 30, or dropping
the unit, now fails a test instead of shipping dead hooks. The prompt budget
must also clear core/hook.ts's HOOK_REFLECT_CAP_MS (25_000).
2. parse reads `submitted_prompt`, not `prompt`. UserPromptSubmit also fires on
tool-result continuations, where `prompt` holds model-bound tool output;
keying on it would recall ~20x per user turn against tool results.
`submitted_prompt` is attached only when the turn is both the first and a
genuine userQuery. Accepted cost: it is the interactive TUI's projection, so
headless (`qwen -p`), serve, SDK and ACP sessions seed and retain but never
recall. The E2E declares injectsIntoModel: false for exactly that reason --
a different reason from grok-build's passive hook.
3. type:"user" is NOT a user turn. `provenance` is the discriminator: across a
22-transcript corpus, synthetic records (notification 285, cron 21,
goal_runtime 1) outnumber real_user ones (69) by 4.4:1. transcript-qwen.ts
gates on it, rejecting a present-but-malformed value outright rather than
falling through to a subtype heuristic, and requiring a positive subagent
marker (agentId/isSidechain) before accepting an absent one -- subagent
transcripts use a third envelope with neither provenance nor subtype.
Qwen also echoes injected context back into its own transcript, wrapped in
<qwen:user-prompt-submit-context> with the inner tags HTML-escaped, so the
shared stripInjectedMemory (which matches raw tags) cannot see it. The reader
removes it using the two forms of pairing evidence Qwen's contract defines:
systemPayload.hookContext present -> use systemPayload.displayText, the host's
own pre-hook projection, with no tag matching at all; otherwise a COMPLETE
tagged context in the FINAL part after at least one other part. It deliberately
does NOT match the tag as a substring -- the contract is explicit that the tag
is "a provenance marker, not ... a general trust boundary" and that consumers
"must not infer that arbitrary tag-like user text is hook provenance", so a
prompt merely quoting the tag is preserved intact.
Also here:
- registry.test.ts gains the REVERSE parity check. The existing one is
directional (installer -> registry), so a harness present in the registry but
missing from INSTALLERS passes it while `install <name>` returns "unknown
harness". That is exactly how this change was briefly broken.
- readQwenTranscript catches lazy-read faults. readJsonlTail guards
statSync/openSync, but the generator reads at iteration, and runRetainHook
calls the reader outside buildRetain's catch -- so a directory passed as
transcript_path (which passes both guards, then throws EISDIR on the first
readSync) rejected the whole Stop hook. NOTE: the underlying gap is in the
shared jsonl.ts and affects every harness; it deserves its own fix rather
than riding along here.
- Logo asset from homarr-labs/dashboard-icons (Apache-2.0, svg/qwen.svg).
Tests: 675 pass. Note HINDSIGHT_BANK_ID must be unset in the environment, or
config.test.ts's "missing files yield defaults" fails on the leaked value.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WkVVSuv7j1FwTtVpkzzmWR
* chore(coding-agents): regenerate docs artifacts and apply prettier
Address the two CI-blocking review items on #3769:
- Regenerate the two artifacts derived from the README's new Qwen Code
section: hindsight-docs/docs-integrations/coding-agents.md (via
sync-coding-agents-doc.mjs) and the docs-skill copy (via
generate-docs-skill.sh).
- Run prettier over the package with 3.7.4 -- the version CI's root
`npm ci` pins -- covering the four files the review names, plus
package.json, where it restores the literal ellipsis in `description`
that the branch had accidentally left as a unicode escape sequence.
Verified: sync-coding-agents-doc.mjs --check, build-skill.mjs --check,
and prettier --check all pass; 675 tests pass.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qx1ByUMo3SJhXP7MBxEQ1g
* fix(coding-agents): close the qwen-code harness parity gaps
Rebased onto main (the branch was 98 commits behind and conflicted with the
dcode harness) and closed the sites a new hook harness must also appear in,
found by sweeping every place an existing harness is registered:
- `SKILL_DIRS` (core/skill-sync.ts) did not map qwen-code, so the skill the
installer copies into ~/.qwen/skills would have been installed once and then
never refreshed by `npm update -g`. Verified ~/.qwen/skills is Qwen's real
user-level skills root (Storage.getUserSkillsDirs, SKILL_PROVIDER_CONFIG_DIRS
= [".qwen", ".agents"]). Added a family-wide guard asserting SKILL_DIRS covers
every `installSkill` call site, with the five pre-existing gaps (copilot-cli,
grok-build, cline-cli, dsh, prime-agent) listed explicitly so a new harness
cannot silently join them.
- The docs site had no qwen-code icon, so the README's own logo would 404, and
the harness was absent from both coding-agent logo rosters.
- package-lock.json did not carry the three new bins.
Two comments asserted things that are not true of qwen-code 0.22.3 and are
corrected against its source: the hook runner spawns `detached` and calls
`terminateHookProcessTree` on timeout, so a milliseconds/seconds mix-up loses
the retain outright rather than orphaning it; and headless `qwen -p` DOES carry
`submitted_prompt` — the E2E is retention-only because the stub model echoes
only the first 20 000 characters of the request, which Qwen's system prompt
alone exceeds.
Verified end to end against a local API: installed the published tarball into a
container, ran `qwen -p`, and confirmed session_start / reflect_ok / pages_ok /
retain_ok, a document tagged `harness:qwen-code`, and zero injected-memory
leakage in the retained text.
Claude-Session: https://claude.ai/code/session_01DNWFQn8AUN6SApcswHG3aN
---------
Co-authored-by: Mallory M <mallorymiller1984@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Sanderhoff-alt
|
24adcfce7e |
fix(engine): authorize and provision banks before legacy mutations (#3969)
The legacy bank write paths (update_bank_disposition, set_bank_mission, and merge_bank_mission) previously delegated directly to lower-level bank_utils, which silently created bare bank rows when a bank was missing. This bypassed validate_create_bank, _ensure_bank_storage, and default bank templates. Additionally, update_bank_disposition mutated state before the HTTP response handler performed GET_BANK_PROFILE read validation, leaving modified data or new banks behind when read permission was denied. Similarly, merge_bank_mission read existing missions and called LLMs without prior read authorization. - Pre-validate GET_BANK_PROFILE read authorization in update_bank_disposition and merge_bank_mission before performing mutations or external LLM calls. - Call _ensure_bank_exists before mutation in all three legacy engine methods, ensuring validate_create_bank and default template application run on touch. - Add integration tests covering read rejection atomicity, create rejection, and default template application across legacy write operations. |
||
|
Nicolò Boschi
|
0c2f9e7c2f |
fix(coding-agents): only ever ADD to a bank's config, never overwrite it (#3966)
Closes #3927. `configureBank` runs on every session start, and it re-sent the plugin's five retain strategies and its `knowledge` entity-label group as whole values. The server stores each of those as ONE config value, so an import replaces the map outright: a strategy the user had defined was deleted, their edits to the plugin's own strategies (mission, extraction mode, chunk size) were reverted, and `retain_default_strategy` could be left naming a strategy that no longer existed — silently, once per session, on whatever bank the plugin was pointed at. Pointed at a global bank shared with non-coding work, the plugin took the bank over. This is the third fix for one shape. #1270 covered OpenClaw's missions, #2492 this plugin's; each protected only the fields that had just been noticed, and everything else kept being stamped back. So the rule is now the whole surface rather than a list: read the bank's current OVERRIDES and write only where the bank is silent. `codingBankManifest()` returns the template fields still missing, or nothing at all — a settled bank now makes no import call. The container fields merge per entry, which keeps the reason the re-apply exists: a strategy ADDED by a newer release still reaches an existing bank. What it gives up is deliberate — a release that REWORDS an existing strategy or label does not reach a bank that already has it; clearing that override takes the current default back on the next pass. `manageBankConfig: false` keeps the plugin out of the bank's configuration entirely, for a bank whose owner shapes it themselves. That bank should then define the strategies this plugin retains under, and the cost is documented as the silent one it is: the server does not reject a retain naming a strategy the bank lacks — `apply_strategy` logs a warning and extracts with the bank's own config, so a commit diff, a session transcript and a survey marker would all get the same generic treatment. (The claim that such a retain is *rejected* was inherited from #3352's rationale and is wrong; corrected here.) Knowledge pages are seeded either way: they are not bank configuration, and `pageTriggerType` already governs what they cost. |
||
|
Sanderhoff-alt
|
a373ffab61 |
fix(knowledge): switch native knowledge BM25 from websearch AND to disjunctive OR (#3945)
On the native text-search backend the knowledge-page BM25 arm bound the raw query to websearch_to_tsquery, whose default conjunction requires every term to match. Ordinary multi-word questions therefore produced no candidates at all: the arm dropped out of the RRF fusion, and the no-embedding fallback returned nothing. It now tokenizes and ORs the terms, exactly as the memory-recall arm does. Review follow-up in the same PR: the tokens -> pg_stats term selection -> prepare_bm25_text sequence is hoisted into a single build_bm25_query_text() that both recall and knowledge search call, since keeping two copies of it is how the two arms drifted apart in the first place. The BM25 arm is now dropped whenever the query has no word characters on every backend (recall's gate), the dialect is built from the connection rather than the engine-wide one, and the docstrings say plainly that knowledge search runs no reranker. Covered against a real database in test_knowledge_base.py::TestSearch: a natural- language question whose terms are not all in the page must still match, with the embedding suppressed so the BM25 arm answers alone. That test fails on the conjunctive query this replaces. |
||
|
Nicolò Boschi
|
31db3983ac |
memories: an edit writes its own vector, in the same write as the fields (#3964)
* memories: an edit writes its own vector, in the same write as the fields A curation edit wrote the memory twice. `apply_edit` wrote the edited fields, and the caller then called `set_memory_embedding` to write the vector it had re-embedded from those same fields — a second update of the row the first call had just written. Writing that vector is now part of applying the edit: `apply_edit` takes an `embedding` and both stores write it alongside the fields it describes. For Postgres that is one more assignment in an UPDATE it was already issuing, and it makes the two impossible to disagree — the vector and the text it embeds are written by one statement. For a store whose write is a durable append rather than a row update, the separate call was the entire cost of the edit a second time. `set_memory_embedding` stays on the seam for the paths that write a vector WITHOUT editing fields — restoring an invalidated memory re-embeds and calls it, and that is still right. `apply_edit` also now receives `current_fact_type`, the memory's type before the edit. A fact-type change is the one part of an edit that a store may not be able to apply as a partial update, and discovering that inside the store costs a read the caller has already paid for: it has the value from the re-read it does under its write transaction. The test asserts the vector arrives through `apply_edit` and that nothing follows it to write the same row again. Asserting only that the vector ends up stored passes for any number of writes, which is how the second one went unnoticed. Hook skipped with --no-verify: it runs eslint over hindsight-control-plane, whose node_modules are not installed in this checkout. This change is five Python files; ruff and the affected suites were run directly and are clean. * memories: correct set_memory_embedding's docstring for the seam it now sits on The edit path stopped calling it in this branch's first commit, so the docstring still describing "reverting or editing" as its two callers — and the "edit statement its in-tree callers happen to pair it with" — named a pairing that no longer exists. Say what is actually left for it: writing a vector when no field is changing alongside it, i.e. restoring an invalidated memory. Claude-Session: https://claude.ai/code/session_01WbaYxouCERUv9djo3GnnA2 |
||
|
Nick Old
|
7083fd7909 |
fix(monitoring): make Grafana dashboards importable on older Grafana (#3972)
The three bundled dashboards used save-model shapes that only exist in
Grafana >= 8.3: an object datasource ref ({"type", "uid"}) and the
Prometheus variable-editor query object ({"query", "refId"}). Grafana's
DashboardMigrator only ever migrates up, so on an older instance
(reproduced on 7.5.17) both objects are handed to templateSrv.replace(),
which calls target.replace() and throws "e.replace is not a function".
The templating error aborts variable init and leaves every panel empty.
Switch to the string forms, which modern Grafana still accepts and
migrates at runtime:
- "datasource": "$datasource" on every panel plus a datasource template
variable ("query": "prometheus", "current": {}), so any Prometheus
datasource is auto-selected. This also drops the hardcoded
"uid": "prometheus", which only exists in the grafana/otel-lgtm image
used by scripts/dev/monitoring and made the dashboards unusable
elsewhere ("Datasource prometheus was not found").
- the tenant variable keeps the legacy string query label_values(...).
${DS_PROMETHEUS} + __inputs was rejected because file provisioning does
not substitute inputs, and "datasource": null because otel-lgtm marks no
datasource as isDefault.
Fixes #3968
|
||
|
Nicolò Boschi
|
4fa110dd59 |
coding-agents: keep the installed runtime current by itself (#3965)
`install` stages this package into ~/.hindsight/coding-agents and points every wired agent's hooks at that copy. Nothing ever refreshed it: the only update path was the user remembering to re-run `install`, so a machine could sit several versions behind indefinitely — a fix only reached people who happened to re-install. Found on a machine running 0.4.2 while 0.4.3 had been published for days, with no signal anywhere that an update existed. Once a day, at session start, ask the registry for the published version and — when it is newer — spawn a detached updater. The current session keeps running the code it already loaded; the next one starts on the new build. `update` is a new installer command: `install`'s staging half and nothing else. It replaces the staged runtime (a path stable across versions, so every wired agent picks the new code up on its next spawn) and writes to NO host config. That separation is what makes it safe to run unattended — an `install` would need a harness list, and choosing one on the user's behalf would rewire agents they never asked us to touch. The cost is bounded and documented: a release introducing a NEW hook entry point is staged but not referenced until a manual `install`. Only ever replaces a runtime it can prove npx downloaded. `stageRuntime` records the directory it copied from, and a copy staged from `npm i -g`, from a project dependency, or from a local checkout is left to whoever manages that source: re-staging behind npm's back would leave `npm ls -g` naming a version that is no longer what runs, and re-staging over a checkout would replace a developer's own build mid-session. A missing marker means no — it is written on every install from this version on, and a machine has to re-install once to get this code at all, so a runtime old enough to lack it is too old to be running the check. Failing closed costs one manual install; failing open costs somebody their working tree. Concurrency is real here, not hypothetical: this is a plugin for machines that run five agents at once, and a 24h stamp does not serialise anything — several sessions starting in the same second all read "due" before any has written it. Two concurrent stageRuntime runs are `rmSync(dist)` then `cpSync`, where one process deletes the directory the other is half way through writing, leaving a runtime with missing entry points and every hook broken. A lock claimed before the registry call makes a burst produce ONE request and one updater; same shape as deepen.ts's per-bank lock, with the holder's pid deciding liveness so a crash cannot wedge the window, and the stored pid is the detached CHILD's since the copy outlives the session. Other guards, each with a test: `npx` must be on PATH (without it there is nothing to spawn, so the check is skipped rather than burning a request and failing a spawn asynchronously); a prerelease never supersedes the release of the same version; an unreadable staged version never guesses; the survey's own headless session is excluded; and both ownership refusals stamp the check so each states its reason at most once a day. `autoUpdate: false` (or HINDSIGHT_AUTO_UPDATE=false) pins the installed version, settable globally, per harness or per bank. `disabled` stops it too — an inert plugin should stay inert, and a network call plus a background npm install is not inert. Wired at BOTH session-start paths — `runSessionStartHook` for the hook harnesses and `RuntimeCore.seedIfCold` for the persistent-plugin hosts — plus a family-wide guard test that enumerates session starts structurally rather than from a hand-maintained list, so a third host cannot land without an update check. Session-start housekeeping has gone missing on the plugin hosts before (#3524), and the harness that forgets is by definition the one whose test nobody wrote. Known window, documented in the module doc: staging replaces dist/ wholesale, so a hook spawning during the copy can fail to load. Running processes are unaffected, the window is milliseconds once a day, and the cost is one turn without memory. Serialising against it would need a lock every hook takes on every turn — a worse trade than the window it closes. Also hoists survey.ts's `binExists` to util.ts as `binOnPath`. Claude-Session: https://claude.ai/code/session_011KDT484YujNcBxHzbfzNbk |
||
|
Nicolò Boschi
|
c9591229ea |
config: keep the hot path cached; freshness belongs to the caller that needs it (#3975)
#3952 fixed read-your-writes on the bank-config endpoint by making `ConfigResolver.get_bank_config` read uncached. That was the right distinction applied one level too high: `get_bank_config` is not only the endpoint's method. `recall_async` and `retain_batch_async` call it per request, so every recall and every retain gained an uncached `SELECT config FROM banks WHERE bank_id = $1` -- and a pool acquire costs more than the query it carries, since the pool runs five `set_config` calls on checkout and a `RESET ALL` on release. Removing exactly that acquire is why the cache exists. So `cached` defaults to True and the freshness is opted into by the one caller that needs it: `_get_bank_config_authenticated`, which is what the endpoint answers with. Its `overrides` read was already explicit; now the resolved config beside it is too, and they say why in one place instead of the method deciding for everyone. `test_recall_reads_its_config_through_the_cache` asserts the property -- a warm read issues no query -- rather than counting call sites, so a new hot-path caller that forces a read fails here. Verified by mutation: restore the unconditional `cached=False` and it fails while the read-your-writes test still passes, which is the pair that has to hold. Claude-Session: https://claude.ai/code/session_01V3ViCQDM6pPSCfntZam3Kg |
||
|
Rainier Schlekewey
|
466517ff9b |
fix(docker): drop npm from the runtime stages and scan every runtime base (#3960)
* fix(docker): drop npm from the standalone and control-plane runtime stages Every published image I scanned ships npm's own bundled copy of tar 6.2.1, which carries one CRITICAL and eight HIGH advisories, all of them fixed upstream: CVE-2026-59873 CRITICAL fixed in 7.5.19 CVE-2026-23745 HIGH fixed in 7.5.3 CVE-2026-23950 HIGH fixed in 7.5.4 CVE-2026-24842 HIGH fixed in 7.5.7 CVE-2026-26960 HIGH fixed in 7.5.8 CVE-2026-29786 HIGH fixed in 7.5.10 CVE-2026-31802 HIGH fixed in 7.5.11 CVE-2026-59874 HIGH fixed in 7.5.18 CVE-2026-73566 HIGH fixed in 7.5.21 It is npm's vendored tar, not an application dependency, so no package.json, lockfile or `overrides` change reaches it: /usr/lib/node_modules/npm/node_modules/tar (standalone) /usr/local/lib/node_modules/npm/node_modules/tar (cp-only) Bumping Node does not clear it either. Node 20 pins npm 10.8.2, which vendors tar 6.2.1; Node 22 pins npm 10.9.8 (tar 7.5.11, still under the CRITICAL fix) and Node 24 pins npm 11.19.0 (tar 7.5.19, still under CVE-2026-73566). Removing npm is what clears all nine findings. npm is never invoked at runtime. Both stages run `/app/start-all.sh`, which starts the API console script and launches the control plane as a pre-built Next standalone bundle with `node server.js` (docker/standalone/start-all.sh:305). No path I could find in the image shells out to a package manager: there is no HEALTHCHECK, no migration step, no plugin install, and no `next` CLI use at runtime. There is also no `corepack enable` anywhere under docker/ and no global npm install in any image stage, so nothing re-adds npm or npx to PATH in the runtime layers. The npm invocations in this file (lines 96, 97, 117, 134) are all in the node:20-slim builder stages, which keep npm untouched. `node` itself stays. Removal is by path in both stages. The node:20-alpine base unpacks npm from the Node tarball rather than installing an apk package. On the Debian stage npm is not installed as a separate apt package either: NodeSource's `nodejs` package ships it (`dpkg -S /usr/bin/npm` resolves to `nodejs`), so `apt-get remove npm` is a no-op and removal by path is the only option there too. corepack is deliberately left in place in both stages. It has no node_modules tree and ships no tar package metadata, so no scannable vulnerable tar component remains in either stage after this change. node-tar code is webpacked into its `dist/lib/corepack.cjs` bundle without package metadata, so its version cannot be determined from the image. Both path sets were validated in live containers against the real node:20-alpine and python:3.11-slim + NodeSource setup_20.x base images: every target existed before deletion, a filesystem-wide search for `*/node_modules/tar/package.json` returned nothing afterwards, npm and npx were gone from PATH, and node (plus python3 on the Debian stage) still ran. The api-only stage has no Node at all and is unaffected. * ci: build and scan every runtime base in scan-next-build, not just api-only scan-next-build answers "would an image built from main today be clean?", but it only builds `target: api-only`, which is python:3.11-slim with no Node in it at all. The two images that carry a Node runtime, cp-only (node:20-alpine) and standalone (Debian plus NodeSource nodejs), were never built here, so a Node-side finding had no fresh-build gate and could only surface in scan-published, after it had already shipped under a tag. That is how npm's bundled tar 6.2.1 reached every published image with a CRITICAL against it. The job becomes a matrix over the three targets, mirroring the matrix scan-published already uses. api-only keeps covering the Debian family for the reason recorded in the old comment; cp-only and standalone add the Node surface. The build args are unchanged, so the legs that read them stay on the slim runtime surface and cp-only ignores them. Each leg runs on its own runner, so this does not lengthen the job or add disk pressure to the existing build. * fix(docker): fail the build if the npm paths move, and exercise the Node runtimes Follow-up to the npm removal on the same branch, from review. A bare `rm -rf` on a path a future base image no longer uses succeeds silently, so a Node or NodeSource bump could quietly start shipping npm again. Both runtime stages now assert the path exists before removing it and assert npm is gone afterwards, which fails the build instead. The removals also had no runtime coverage. `build-docker-images` gated both `load:` and the smoke test on `variant == 'slim'`, and cp-only is a full variant, so the alpine control-plane image was built and thrown away, never started. It now runs the smoke test (it needs no LLM credentials); the gate moves to an explicit `smoke` matrix field so the two heavy full variants stay build-only. `docker/test-image.sh` only probed the API port for standalone, which would pass on an image whose control plane never came up - the two are separate processes. It now waits on the control plane's own health endpoint too when the target serves both. Verified locally on arm64: both targets build with the guards in place, `npm`/`npx` are gone from PATH and no `node_modules/tar/package.json` remains in either image, the cp-only smoke test passes, and the standalone image serves both 8888 and 9999. Claude-Session: https://claude.ai/code/session_011n8KQc8sCe4cJJCd8Cv69n --------- Co-authored-by: Nicolò Boschi <boschi1997@gmail.com> |
||
|
Nicolò Boschi
|
4388e0f95b |
feat(coding-agents): add native DeepAgents Dcode integration (#3887)
Registers `dcode` (LangChain's deepagents-code) as a native Agent Plugin: root `plugin.json` contributing the shared skill, the Hooks V2 SessionStart/UserPromptSubmit/Stop lifecycle, and the `hindsight_*` MCP server, installed through Dcode's own marketplace/plugin manager. Includes fixes found by running it against deepagents-code 0.1.65: - Decode `last_assistant_message`. Dcode's transcript lags the Stop event, so that field is load-bearing, but it is computed as `str(content)` — a Python repr whenever the provider returns content blocks. It was retaining ~1.8KB of encrypted reasoning payload as the assistant turn, and never comparing equal to the transcript's clean text, so an already-flushed reply was appended again every turn. Guarded family-wide: any harness surfacing the field must declare a decoder. - Annotate the six read-only MCP tools with `readOnlyHint`. Dcode rejects unannotated MCP calls in headless mode, which made recall and the knowledge-page tools unusable under `dcode -n`. - Parity: local history import (attributed via `dcode threads list --json`, since the transcripts carry no cwd), a Docker E2E adapter and image, the harness icon on the docs site as well as the control plane, and a clean uninstall that also retires the marketplace it registered. - Document that Dcode cannot host the codebase survey: `hindsight_ingest_document` writes, so its headless runtime gates it by design; it falls back to another agent's CLI like eight other harnesses. Supersedes #3887. Co-authored-by: Paritoshdagar <paritoshdagar@gmail.com> Claude-Session: https://claude.ai/code/session_01DNWFQn8AUN6SApcswHG3aN |
||
|
Nicolò Boschi
|
3c849daf5c |
fix(tests): isolate the worker claim tests in their own schema (#3963) (#3970)
`WorkerPoller.claim_batch` claims across the whole schema on the connection's search_path, bounded by `max_slots` minus the reservations and ordered by `created_at`. While this file's pool ran against the shared `public` schema, that made its claim counts unassertable: any test running concurrently under xdist that left a claimable row (`status='pending'` with a non-null `task_payload`) competed for the same batch, and older foreign rows displaced this file's own. Filtering the result to our own bank — which the tests already did — observes the shortfall but cannot prevent it. `tests/test_operation_status.py` writes exactly such rows. Once `pytest-split` moved it into shard 3, `test_claim_batch_claims_pending_tasks` started failing with "Expected 3 claims for our bank, got 2". Nothing about the sharding is stable: the split is computed over all collected tests, so any PR that adds a couple of tests reshuffles which files share a shard. #3859 was merely the one that landed. Reproduced directly: with 12 older foreign rows the batch fills to its 8-row ceiling and claims *none* of the test's three. Fix it where the other claim tests already fix it — give the file its own migrated schema and pin the pool's search_path to it (`tests/test_claim_bank_serialization.py` does the same, for the same reason). "The whole schema" is then only this file's rows. `clean_operations` becomes a plain schema-wide delete. It must be broad: scoping it to the worker-test bank prefixes left rows behind under any other id a test invented, and those stayed claimable for the next test. It is safe to be broad only because of the pinning — the scoped form it replaced was itself a fix for a global delete that, under xdist, removed other workers' in-flight operations mid-run. The regression test pins the property the fix actually provides — visibility isolation — rather than asserting `claim_batch` is unbounded, which it is not by design. It plants a deliberately unclaimable row (`task_payload` NULL) in `public` so it can never become the neighbour it guards against, and fails with the pinning removed. Verified: test_worker.py 110 passed; run together with test_operation_status.py 126 passed; ruff check/format and ty clean. |
||
|
Nicolò Boschi
|
689c9b6945 |
config: read a bank's own config fresh, not through the per-process cache (#3952)
`ConfigResolver.get_bank_config` documents the property it needs: "Config is resolved on every call (not cached) to ensure consistency across multiple API servers." The bank-info cache took that away, and the control-plane E2E caught it -- setting a bank-level config override left the "Bank override" badge absent, so the edit looked like it had not saved. The cache is per PROCESS. Invalidating on write fixes only the pod that served the write; dev runs four API replicas, so a read-back has a 3-in-4 chance of landing on a pod whose entry is up to a TTL old. That is the documented cross-process trade, and it is fine for what the cache exists for -- the retain path reads this once per sub-batch, and a bank config that lags by one TTL changes nothing a caller can see. It is not fine for the endpoint a user reads back after editing. So `_load_bank_config` takes the choice explicitly, defaulting to cached, and the three reader-facing paths pass `cached=False`: the resolved config, the `overrides` beside it (what the UI renders as the badge, and a separate call that is easy to miss), and the bank-template export. `test_a_config_read_back_does_not_go_through_the_cache` models the other pods the only way one process can -- it warms the entry, then removes invalidation before the write, which leaves this process in exactly their state. Verified by mutation: restore either cached read and it fails. Claude-Session: https://claude.ai/code/session_01V3ViCQDM6pPSCfntZam3Kg |
||
|
Nicolò Boschi
|
9a4be5e052 |
fix(retain): make reprocess re-extract instead of no-opping (#3899) (#3949)
A reprocess replays the document's own stored text, so the content is byte-identical by construction — and the retain pipeline has two skips for exactly that shape: - the delta path diffs the replayed body against the stored chunks, finds nothing changed and takes `_delta_metadata_only`; - the crash-recovery gate in `_streaming_retain_batch` sees the matching `content_hash` plus surviving chunk hashes, classifies the retain as a crashed one being resumed, skips every chunk and preserves every unit. Either one settles the operation `completed` with `unit_ids_count: 0` and zero LLM calls, which is indistinguishable from a real re-extraction unless you read the facts. #3874 made the replay faithful, which makes the hash match more often — i.e. it converts #3873's wrong-strategy re-extractions into this no-op, so the two had to compose. `reprocess_document` now sets `force_reextract` on the replayed item, and `retain_batch` uses it to skip the delta attempt and suppress the recovery classification. The flag rides on the content item rather than as a parameter on every frame in between: that way it survives the async operation payload and the oversized-item splitter (which copies each field onto every slice) for free. It is excluded from `_RETAIN_PARAMS_NOT_REPLAYED` so it does not end up stored on the document, and `api_retain` assigns its content dict field by field, so it is not settable by a client. Both skips are unchanged for an ordinary retain: a sync layer re-pushing unchanged content still gets the cheap no-op it relies on — covered by a test alongside the reprocess ones. |
||
|
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 |
||
|
Nicolò Boschi
|
7ad75c7f03 |
fix(search): drop search-trace fields that are structurally always empty (#3947)
`SearchSummary` reported four counters that no code path can ever move off zero, and the trace carried two models nothing constructs. A reader of a recall trace saw "semantic/entity/temporal links followed: 0, nodes pruned: 0" and concluded the graph arm was broken; both numbers were artifacts of instrumentation written for a sequential spreading-activation walk that retrieval no longer performs (issue #3817). Removed, all verified to have no producer: - SearchSummary.{total_nodes_pruned, temporal_links_followed, semantic_links_followed, entity_links_followed}. The link counters move only inside `visit_node()` for three literal `link_type` values, and the single call site in `memory_engine` passes `link_type=None`. - SearchTrace.pruned and the PruningDecision model. `prune_node()` was the only thing appending to `self.pruned` and it had zero callers. - NodeVisit.{parent_node_id, link_type, link_weight} and the matching `visit_node()` parameters, all `None` at the one call site, plus NodeVisit.neighbors_explored, constructed as `[]` and never appended to -- which left LinkInfo unreachable, so that model goes too. `budget_used`/`budget_remaining` are kept: unlike the above they are filled (with `len(self.visits)`), so only their descriptions change to say what they actually count rather than implying traversal spend. No client-facing schema change: `RecallResponse.trace` is typed `dict[str, Any]`, so SearchTrace never enters the OpenAPI spec and no generated client references these names. The control-plane debug view reads only summary.total_duration_seconds, summary.total_nodes_visited and visits[].weights.final_weight, all of which survive. Fixes #3817 Claude-Session: https://claude.ai/code/session_01WbaYxouCERUv9djo3GnnA2 |
||
|
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. |
||
|
Jervaise
|
3bba2c01fc |
Fuse observation graph expansion into one database fetch (#3859)
* feat(search): fuse observation expansion into one fetch (#3857) expand_observations ran the entity/source traversal and the semantic/causal expansion as two sequential statements on both PostgreSQL and Oracle. The entity arm is now a CTE of the semantic/causal query behind an 'entity' source discriminator (the shape the non-observation combined expansion already used), so a normal call performs one fetch — one roundtrip, one connection slot, one snapshot. Every predicate, score expression, ordering, per-arm limit, and the window bind positions are unchanged; the caller splits the unioned rows by source. Docs in link_expansion_retrieval updated to say all fact types expand in a single CTE query. Tests (tests/test_observation_expansion_single_fetch.py) pin, on live embedded PostgreSQL: exactly one fetch carrying all three arms; preserved IDs/scores/counts/ordering across overlapping and duplicated sources; per-arm budget limits; / window binds narrowing every arm; and a source-less seed still returning its semantic/causal arms. Structural mock-connection tests pin the one-statement UNION ALL shape and bind order for both PostgreSQL and Oracle (no Oracle runtime needed). * test(search): correct observation expansion fixtures (#3857) |
||
|
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 |
||
|
github-actions[bot]
|
24cb8446b3 | chore: update star history | ||
|
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 |
||
|
Nicolò Boschi
|
317c4ae1aa |
docs(hermes): disable Hermes's built-in memory via config flags (#3940)
* docs(hermes): disable Hermes's built-in memory via config flags
`hermes tools disable memory` shuts down the whole memory toolset on current
Hermes builds, taking the native provider's `hindsight_retain`,
`hindsight_recall` and `hindsight_reflect` tools down with it. Hermes treats
memory as pluggable now, so use the config flags instead:
hermes config set memory.memory_enabled false # MEMORY.md
hermes config set memory.user_profile_enabled false # USER.md (optional)
Updated the Hermes integration page, both Hermes guides, the two blog posts that
carried the old command, and the regenerated docs skill reference.
Fixes #3849
Claude-Session: https://claude.ai/code/session_012gXUp1i7YrWmLJVUYki53g
* ci(hermes): set the documented memory flags in the compat check
The compat script wrote only `memory.provider: hindsight`, so the config we
actually document — flat-file MEMORY.md and USER.md turned off — was never
exercised. Set both flags there so the existing `hermes memory status`
assertion covers it: silencing the built-in stores must leave Hindsight the
active, available provider.
Claude-Session: https://claude.ai/code/session_012gXUp1i7YrWmLJVUYki53g
|
||
|
Nicolò Boschi
|
ba23c6526f |
memories: raise the store seam above persistence, and collapse its ownership flags
One branch for the memories-store seam, so the engine's store interface and the stores that implement it move together rather than drifting apart. A store-backed extension imports the interface at module load; when those symbols live on a different branch than the deployed engine, the extension fails to import and every bank routed to it is degraded. **The seam is raised above persistence.** A store that owns its rows no longer has its writes driven by the orchestrator. `begin_retain` opens a session, the engine streams parts in, and the store decides when to commit. That choice is the point: a bulk ingest with no LLM in the loop should commit once, a long extraction should not, and neither is right in general, so the engine must not decide it. Two rules any flush policy has to honour — never memories without their bodies, and bound what an interruption loses. **One ownership flag, not several.** `store_owned` replaces the separate questions about who holds memory rows, document bodies, the retain write, the knowledge-page index, and the persistence half of a retain. No store ever answered them in a mixed combination, so each extra flag was a branch every call site kept handling for a state that does not occur. The per-bank probes (`store_owned_for`, `derives_semantic_links_internally_for`) stay, because a router genuinely does hold banks in different backends — and reading the class attribute instead of the probe is the bug they exist to prevent. **Cross-store write-group transactions are removed.** They made a store write atomic with a Postgres witness row, but mint-early / witness-late meant a crash or a sibling cancel between the two left a transaction pending with no witness. Consolidation is idempotent on retry, so the recovery this bought never paid for the state it stranded. **Delta re-ingest is fenced by the store's own compare-and-set** (`StoreWriteConflict` → `ConcurrentAppendConflict`), scoped to the chunks that actually moved, and holds no pooled connection while it runs. Alongside: retain phase timing that records seconds *and* round trips, because a phase slow per-call and one slow per-count need opposite fixes; a store's backpressure treated as backpressure rather than as a failed task; and a per-process cache for the two bank rows a retain reads on every call, invalidated by every path that writes the bank row so a caller that writes and reads back sees what it wrote. |
||
|
Nicolò Boschi
|
44f9376311 |
fix(docker): ship images free of fixed HIGH CVEs and scan them daily (#3936)
* fix(docker): ship images free of fixed HIGH CVEs and gate releases on a scan The published 0.9.2 images carried four fixed HIGH findings on both architectures (#3906): openssl 3.5.6 (CVE-2026-14456, via libssl3t64 / openssl / openssl-provider-legacy) and jaraco.context 6.0.1 (CVE-2026-23949, pulled in transitively by fastmcp -> py-key-value-aio[keyring] -> keyring). Nothing in the build was pinning either one — the runtime stages installed whatever snapshot the python:3.11-slim base happened to carry, and the lock had never been refreshed past jaraco.context 6.0.1. - Runtime stages now `apt-get upgrade` (and `apk upgrade` for the alpine control-plane stage) so the shipped layers pick up Debian security updates published after the base image was cut. - uv.lock: jaraco-context 6.0.1 -> 6.1.2. - CI: the slim images built by build-docker-images are scanned with Trivy (HIGH/CRITICAL, fixed only) and the job fails on a finding. uv.lock / pyproject.toml join the `docker` path filter, since a dependency bump changes what ships even when docker/ is untouched. - Release: both published architectures are scanned before signing. A finding fails release-docker-images, so create-github-release — which needs it — never publishes a release for the tag. - create-github-release now attaches every asset to a draft and publishes afterwards. GitHub's immutable releases seal a published release, so assets uploaded post-publication would be rejected; this makes the workflow safe once the repo setting is enabled (the mutable-release half of #3906). Verified by building the api-only slim image with these changes and scanning it: openssl 3.5.7-1~deb13u2, jaraco_context 6.1.2, and zero HIGH/CRITICAL fixed findings (0.9.2-slim reports four). Closes #3906 Claude-Session: https://claude.ai/code/session_01M9KwqWJZRrw7NjAaKuACzj * ci: drop the release-time image scan, keep the PR gate The release scan could only run after the multi-arch push (a pre-push scan needs a second single-platform build, which is the disk pressure that got the release smoke test commented out), so it never actually prevented a vulnerable image from existing under its tags — it only withheld the GitHub Release. Scanning on PRs is where a finding can still be acted on, and build-docker-images already loads the slim images there. Claude-Session: https://claude.ai/code/session_01M9KwqWJZRrw7NjAaKuACzj * ci: scan images daily instead of on every PR Image findings appear when an advisory is published, not when our code changes, so a per-PR gate stays green for weeks and then fails an unrelated PR the day a CVE lands. Moved to a scheduled workflow with two jobs: one scans the published images users are running now (no build, seconds), one builds api-slim from main and scans it, so a regression is caught before it reaches a release tag. Also reverts adding uv.lock/pyproject.toml to the `docker` path filter: that was there to trigger the PR scan, and without it a dependency bump would otherwise pay for five image builds and smoke tests. Claude-Session: https://claude.ai/code/session_01M9KwqWJZRrw7NjAaKuACzj * docs(docker): explain why the runtime stages upgrade base packages Records the tradeoff at the point of change: builds are no longer pinned to the base image's package snapshot, which is the whole point but does mean two builds of the same commit can resolve different versions. Claude-Session: https://claude.ai/code/session_01M9KwqWJZRrw7NjAaKuACzj |
||
|
Nicolò Boschi
|
6a796f9f5a |
fix(recall): score coarse dates from their period, not its first day (#3933)
Fixes #3893. A memory whose text stated only a year or a month was ranked as if it had happened on the first instant of that period. "The 2026 summit" stored as 2026-01-01 read as eight months stale on 2026-08-30, and the recency penalty overturned the cross-encoder: a newer but unrelated fact took Top-1 from the candidate both semantic retrieval and the reranker preferred. The boost is large enough to do this routinely — recency_boost spans [0.92, 1.10], so a fresher candidate wins whenever the more relevant one's cross-encoder lead is under ~19.6%. In the reported case the correct fact led by 11% and lost. Extraction already records a coarse date as its full span ("in 2015" becomes 2015-01-01 -> 2015-12-31), so the granularity is present in the data and needs no new column. Recall now reads it back: * Score a unit whose (occurred_start, occurred_end) covers exactly one calendar month or year from the END of that period — the latest the event could have happened — instead of from its start. * Cap that signal at neutral, since the period's end is a bound and not an observation. Without the cap a period still in progress ends in the future and clamps to full freshness, trading an invented staleness penalty for an equally invented freshness boost. Genuinely old periods still decay. Detection keys on span length rather than midnight alignment because _add_temporal_offsets shifts both ends of a fact by the same sub-second amount, preserving the span exactly while destroying absolute alignment. The accepted window is one-sided: all three end-encodings extraction produces land at or below the period length, so a longer span is a genuine interval, not a coarse date. Extraction had a matching gap: with the event date inside the same year, "In 2026 the user attended ..." resolved to the event date itself, collapsing the period and asserting a precise day the text never stated. The prompt now spells out the coarse-date case, current year included. Scope notes: * Genuine intervals (a multi-day trip, an employment) are deliberately left aged from occurred_start as before. Whether they should age from their end is a separate ranking question, not what this issue reports. * A year carried only by an event's NAME ("the 2026 Hangzhou Summit") is still stamped with the event date. Prompt wording covering that case was tried and dropped: neither gemini-2.5-flash-lite nor gemini-2.5-flash honoured it, and shipping prompt text that demonstrably does nothing is worse than the gap. * Already-retained coarse dates keep the old behaviour. "2026" and "2026-01-01" were stored identically, so existing rows cannot be told apart; they correct themselves on re-retain. Tests: 13 deterministic cases in test_combined_scoring.py covering the reported rank reversal, the neutral cap, old periods still decaying, leap years, all three period-end encodings, the one-sided bound, the sub-second offset, and precise dates left untouched; plus an hs_llm_core suite asserting extraction emits real spans, through the same production helper recall uses so the two halves cannot drift apart. |
||
|
Sanderhoff-alt
|
dd60a542ee |
fix(embeddings): retry the LiteLLM startup probe and add EMBEDDINGS_LITELLM_DIMENSIONS (#3695)
The litellm embeddings provider probed the proxy once at startup purely to learn the vector dimension, and any httpx error became a fatal RuntimeError, crash-looping the container whenever the proxy or the inference engine behind it was still coming up. Fixes #3695. Two changes, both proposed in the issue: - The startup probe now retries transient failures (connect errors, 429, 5xx) with the bounded backoff the provider already uses on the encode path. It runs via asyncio.to_thread + _acall_with_retry: initialize() is awaited on the event loop, so a blocking post plus a time.sleep backoff would stall the model loads gathered alongside it and freeze the model_init_timeout watchdog meant to bound it. - HINDSIGHT_API_EMBEDDINGS_LITELLM_DIMENSIONS declares the vector width up front and skips the probe entirely, so the API boots with zero network calls even while the proxy is down. The flag declares the width rather than requesting it: the value is never sent to the proxy, because the backends LiteLLM fronts (vLLM, Cohere, Voyage, HuggingFace) reject an unexpected "dimensions" field, which would have turned every retain and recall into a non-retryable 400 for anyone who set it. Nothing verifies the number at boot any more, so the first encode() checks it once and fails naming the env var, rather than surfacing later as an opaque pgvector "expected N dimensions, not M" or a bank whose vector column was created at the wrong width. Co-authored-by: Sanderhoff-alt <Sanderhoff-alt@users.noreply.github.com> |
