skills/vectorize-io__hindsight
1
0
Fork 0
mirror of https://github.com/vectorize-io/hindsight.git synced 2026-09-14 19:31:49 +08:00
Code Issues Packages Projects Releases Wiki Activity
Files
5d46f9c8c8eb4fb96f549aa63abe1191b82a7840
vectorize-io__hindsight/CLAUDE.md
T
Nicolò Boschi b5034cb690 test(system): blackbox system-test coverage across the epic (#4216) 
* test(system): stories 02-09 for the recall surface

Eight stories over the retrieval pipeline, all driven through the published
client against a real server. Part of #4214.

- 02 every retrieval arm runs, and fusion records what each one found. Asserted
  through `trace=True`: a blended final score cannot distinguish "both arms
  agreed" from "one arm did all the work". Also pins that `graph` returns
  nothing for a single-document bank, and that temporal is a scoring component
  rather than a fourth arm despite the docs' "four strategies".
- 03 `max_tokens` spends the budget in rank order. The middle case looks wrong
  and is the point: at a tight budget the *second*-ranked fact comes back,
  because the top one does not fit and must skip only itself (#3688).
- 04 `budget` and `max_tokens` are independent dials — depth asserted via the
  numeric budget in the trace, since any corpus small enough to read finds
  everything at every level.
- 05 the `assistant` -> `experience` rename between the extraction schema and
  the read model, which nothing in either schema hints at.
- 06 tag scoping, and that fuzzy matching is opt-in via a `tag_groups` leaf
  rather than the plain `tags` parameter. Includes the short-word cliff: the
  same single transposition clears the 0.45 trigram floor in `typescript`
  (0.467) and misses it in `music` (0.333).
- 07 the two clocks. `fact_kind="event"` is load-bearing and silent — a date on
  a `conversation` fact is discarded with no error — and recency decays from the
  event date, which `query_timestamp` re-anchors.
- 08 an absurd date must not deny the whole bank: a future-dated fact makes
  `days_ago` negative, and the unclamped exponential fails every recall rather
  than mis-ranking one row.
- 09 `min_scores` floors two different things. `reranker`/`final` filter
  results; `semantic`/`keyword` gate their own retrieval arm, so flooring one
  alone removes nothing because the other arm supplies the fact back. The no-op
  is pinned deliberately — the parameter reads like a quality filter and is not.

`payloads.Fact` gains `fact_kind` for story 07.

Each story declares the consolidate step explicitly rather than sharing a
helper: the suite's rule is that no LLM call is answered by default, and hiding
one behind a fixture would be the first exception.

* test(system): stories 10-15, documents and lifecycle

- 10 a retain is a document: id, verbatim text, chunk composite id, provenance
- 11 replace, including the pure-deletion case where the revision only *stops*
  saying something and nothing new arrives to overwrite it
- 12 append accumulates instead of re-running the removal diff; re-sending a
  turn does not duplicate its fact
- 13 delete takes only its own facts, and — the #3429 shape — two banks sharing
  a document_id do not take each other down
- 14 reprocess is not a silent no-op: the extraction answer changes between the
  retain and the reprocess, so the new facts can only appear if the stored text
  was genuinely re-read
- 15 curation: edit reaches the read path and is stamped edited_at; invalidation
  stops answering recalls while keeping reason and timestamp for an audit

Found while writing these, filed rather than worked around: #4218 (list rows
are untyped dicts while single-fetch siblings return models) — the reason this
file mixes item["id"] with attribute access.

* test(system): stories 20-27, observations and consolidation

- 20 an observation is written *over* the facts, not instead of them, and its
  cited evidence resolves to facts that are still present
- 22 new evidence for an existing claim merges into it (proof_count grows, one
  observation) rather than growing a near-duplicate sibling
- 23 contradictory evidence supersedes: create + delete, so a recall never hands
  an agent both sides with equal confidence — while the historical *fact*
  survives the synthesis that summarised it
- 25 clear_observations empties the derived layer and leaves every source fact,
  and deleting the last document behind an observation retires it too
- 26 trigger_consolidation reports its operation and whether it deduplicated,
  so a double-fired trigger cannot quietly run twice
- 27 prefer_observations drops the facts an observation superseded, and
  include_source_facts returns them as keyed provenance instead

Restores `answers_with` to the rulebook, trimmed in #4212 for having no
consumer: consolidation replies must cite source_fact_ids the server minted
during the retain that triggered them, which a literal payload cannot know.

* test(system): story 30, mental models, plus the machinery to drive reflect

A mental-model refresh runs the full reflect loop in the worker, so the suite
had to learn to drive an agentic conversation:

- `tool=` matcher on rules. The reflect loop sends the same system prompt every
  turn and varies only the tools it offers, so the tool list is the one thing
  that separates one rung from the next; prompt substrings cannot.
- `returns_tool_call` restored (it has consumers now), plus
  `calls_the_offered_tool`, which climbs whichever rung it is on. A story about
  what reflect concludes should not have to enumerate the ladder, or break when
  a rung is added.
- `reflect.reflect_loop` wraps both into one call.
- Two anchors, not one: the search turns and the answering turn use different
  system prompts, and anchoring only the first leaves the final turn unmatched.

Also fixed two things in the harness that this exposed:

- The miss report showed only the user message. A refresh sends the bare source
  query there, so the report named no anchor and showed nothing useful; it now
  carries the whole conversation and the tools offered.
- `wait_until_settled` waited out the worker's retry backoff on an operation
  that had already recorded an error, turning a fast loud-miss into a 90s
  timeout. It now reports the error immediately, and the test server runs with
  worker retries off — against a deterministic stub a retry cannot change the
  answer.

* test(system): stories 35 and 40, knowledge pages and reflect

- 35 a page is a mental model with a place in a tree: both ids resolve, the tree
  carries the name someone browses by, search finds it, and deleting it leaves
  the facts. Also pins the defaults that make a page different from a bare model
  — delta refresh after consolidation, over observations, excluding models — the
  kind of thing a refactor flattens silently.
- 40 reflect answers from what it searched. The second test is the load-bearing
  one and is written inside out: only the answering turn is scripted, so if the
  server could ever reach an answer without searching first, no search turn
  would arrive and the test would pass. It asserts the unscripted search turn
  did arrive, then clears it — the one place in the suite where an unmatched
  call is the subject rather than a gap.

Quality is deliberately not asserted here: the stub supplies the answer text, so
these cover the mechanism. Judging what the model actually says needs a real
model and stays with the hs_llm_core judge tests.

* test(system): stories 43, 50 and 80 — directives, bank config, isolation

- 43 a directive is only real if its text reaches the model. Storing, listing
  and returning it prove nothing; these assert it arrives in the reflect prompt,
  under the MANDATORY heading (delivered as a rule, not as context), that a bank
  without directives ships no such section, and that deleting one stops it being
  sent. Whether the model obeys stays with the judge tests.
- 50 config updates are additive. The clobber failure is invisible — no error,
  the bank just reverts every other field to default — so the assertions are
  about the neighbours, not the field that changed.
- 80 bank isolation across all four verbs, on two banks that deliberately share
  a document_id (#3429). Read, count, update and delete are separate statements
  with separate predicates, so getting three right proves nothing about the
  fourth; each is checked on its own.

Restores LLMStub.calls plus prompts_for(step): prompt assembly is deterministic
even where the model's reading of it is not, so 'was the directive sent' can be
asserted directly instead of judged.

* test(system): stories 70 and 90 — entities/graph and multilingual

- 70 entities are what stitch documents together. Two documents sharing only
  'Alice' merge into one entity, and a query for a word appearing in just one of
  them reaches the other purely through that link. This is the traversal story 02
  could not exercise: remove it and the fact vanishes while everything else
  still passes.
- 90 non-Latin content round-trips byte-for-byte, entities keep their own names,
  mixed-script text keeps its embedded Latin names, and a Latin query reaches
  the Chinese sentence containing one. Emoji cover the astral-plane case.

Story 90 first failed on the *stub*, not the product: the lexical embedder
tokenised `[a-z0-9]+` only, so Chinese text produced no tokens at all and both
query and memory collapsed onto the same fallback vector. A test double that
cannot represent CJK cannot test CJK, so the tokeniser now emits one token per
CJK codepoint — roughly what a real analyser does at the unigram level. ASCII
tokenisation is unchanged, so the pinned scores in stories 01 and 09 still hold.

* test(system): story 60, async retain and idempotent operations

An async retain returns a receipt and moves the whole 'did it land?' question
onto the operation record, so the record has to answer it alone: a terminal
status, and result metadata saying what was stored — 'completed' on its own
cannot distinguish work done from work skipped.

The idempotency tests are the load-bearing ones. A caller retries when a request
times out and cannot tell a lost request from a slow one; without a
caller-supplied operation_id the safe retry does not exist. Both directions are
pinned: the same id replayed after completion stores one memory, not two, and
two different ids over identical content stay two operations — deduplicating on
content instead of the caller's id would be its own silent data loss.

* test(system): code-review fixes in the harness

- _slots returned a two-item tuple, which the project bans outright; it is a
  _Slots dataclass now. Introduced in #4212 and missed by that review.
- returns_tool_call has no consumers again — calls_the_offered_tool covers every
  reflect turn — so it comes back out. Second time this method has been added
  and removed; the rule holding is that surface ships when something uses it.

* test(system): story 21, consolidation failure and recovery

Consolidation is the one background job that both reads and deletes, so a
failure partway through is the most dangerous moment in the system. The correct
behaviour turns out to be boring, which is the point: a model returning nonsense
loses the synthesis and not one fact, the round completes rather than wedging
the bank, and failed_consolidation surfaces the backlog.

Recovery is explicit and worth writing down: trigger_consolidation does NOT pick
failed facts back up — they stay claimed, so a scheduled round will not re-feed
a poison input forever — and recover_consolidation is the deliberate door.

Test-env changes this needed, each because a production default is noise for a
single deterministic server: bank stats cached 60s (a test asserting on a
counter reads a value from before its own action), and the maintenance start
jitter that spreads sweeps over a minute to stop a fleet stampeding one
database.

Two things checked and NOT filed as bugs, both of which looked like one:
failed_consolidation appearing stuck was the 60s stats cache, and a failed
refresh sitting at status=pending was the worker's retry backoff.

* test(system): story 52, whole-bank export and import

Everything in a bank points at everything else by id, and every one of those
references is minted by the source instance — so an import has to rewrite them
all, in one pass, without missing a layer. Miss one and the import still
succeeds: the counts are right and only the provenance is broken. That is the
bug that has been fixed twice here, so the assertions chase references rather
than counts, and check that an imported observation cites facts that exist in
the destination.

Also pinned: the derived layers are opt-in both ways, the page's backing model
exists in the destination rather than naming one left behind, and the imported
bank actually answers recalls (rows arriving is not the same as rebuilt indexes).

The first draft failed on a test bug worth recording: the destination's own
auto-consolidation runs over the imported facts like any other write, so an
observation appearing there proved nothing — it might have been carried or
invented locally moments later. Every import now silences the destination's
consolidation first, which makes each observation necessarily an imported one.

* test(system): stories 31, 32 and 64 — staleness, refresh safety, webhooks

- 31 the watermark that makes 'always current' work. Both failure directions
  matter: a watermark that never advances refreshes forever at full cost, and a
  staleness check that never fires leaves a model quietly frozen while still
  answering confidently. Also pins that going stale does not blank the answer.
- 32 the two wipe guards. A refresh whose scope matches nothing must keep the
  answer it has — writing 'nothing retrieved' through as 'no content' destroys
  work the bank cannot re-derive. And a dry run touches neither content,
  watermarks, nor history, or it is not a dry run.
- 64 webhooks, asserted against a real receiver rather than the server's own
  delivery log, which only proves it tried. The signature is recomputed the way
  a receiver would — a signature over the wrong bytes is a header that looks
  right and verifies nowhere. Both SSRF cases from GHSA-ggrr-69wp-fj54 are
  pinned as security properties, refused at registration.

The stub gains a webhook receiver: it is the only endpoint a hermetic test can
offer. The test server allowlists exactly that host, so a webhook aimed at any
other private address still fails and the guard stays genuinely under test.

* test(system): stories 42, 44, 45 — disposition, structured output, tag groups

- 42 disposition and mission reach the reasoning prompt. Whether a trait changes
  the *answer* is a question for the judge tests; that the knob is connected at
  all is deterministic, and its failure is silent — the API accepts the setting,
  returns it on read, and the agent behaves identically. Opposite dispositions
  are asserted to produce different prompts, which a constant string would fail.
- 44 structured output is a second extraction call over the prose answer, not a
  constraint on it. Pins that the prose is unchanged, that nothing pays for the
  extra call without a schema, and — per #4230 — that a failed extraction is
  currently indistinguishable from an empty one.
- 45 tag_groups and/or/not and nesting, on a corpus where each wrong operator
  returns a different non-empty set rather than nothing.

Three harness bugs surfaced by giving a bank a reflect mission, all of which
would have broken every reflect story the first time anyone set one:

- The "reflect" anchor was the default role line, which a mission *replaces*.
  Re-anchored on the CRITICAL preamble, present on every reflect turn.
- The final turn is a free choice among every search tool plus the finish tool,
  and calls_the_offered_tool always took the first — so it searched, was offered
  the same choice again, and never terminated. Finishing is now claimed by its
  own rule registered first, and carries the answer in its argument (calling it
  bare ends the loop with "the done tool returned no answer").
- The search rules and the prose turn share a system prompt; only the search
  turns carry tools. Rules can now require tools, so the ladder's rules stop
  swallowing the turn meant to write the answer.

* test(system): stories 24, 54, 92 — observation scopes, templates/preview, chunking

- 24 observation_scopes decides which observations a multi-tag memory feeds. The
  isolation is the point: a lesson tagged for a student and a teacher must not
  produce one observation belonging to neither, and consolidation's all_strict
  matching is what keeps one party's observations out of another's. Both
  documented spellings are pinned — [[]] is one global scope, [] is *zero* and
  falls back to combined, one character apart with no error either way.
- 54 templates carry configuration and not memories, leave untouched fields
  unset rather than freezing today's defaults, and the prompt preview matches
  what a real retain actually sends. Also pins that retain_custom_instructions
  is only consulted in `custom` extraction mode — stored, returned on read, and
  silently unused otherwise.
- 92 chunking: contiguous indexes, reassembly loses nothing at the seams, one
  extraction call per chunk (fewer means a piece was never read, more means
  paying twice), and the stored document stays byte-identical because it is what
  a reprocess re-reads.

Fixed a real hole in the harness: the client was a path dependency installed
non-editable, so the venv held a *copy* taken whenever it was last built. The
suite had been testing a stale snapshot — preview_prompt exists in the repo's
client and was simply absent from the installed one. Now editable.

Two template tests are skipped rather than rewritten against raw HTTP: the
import endpoint cannot be called from any SDK (#4232). Reaching around the
client to make them pass would hide exactly the defect a client-driven suite
exists to surface.

* test(system): make the open defects fail instead of documenting them

The suite was green while five filed issues sat unfixed, because it
accommodated every one of them: comments saying "this returns a dict, see
#4218" and then dict access; a skip on the import round trip; and — worst — a
test asserting the *current* wrong answer for #4230, which would have failed the
day someone fixed it and taught the next reader to delete it rather than read it.

A comment is a code-review note, not a gate. These now assert the contract we
want and fail until the product honours it:

- #4217 the recall trace must report the caller's query_timestamp, not the
  moment the trace was built
- #4218 list rows must be typed like their single-fetch siblings
- #4221 every wrapper convenience method must have an async twin — written over
  the whole family, so the next one added without a twin also fails
- #4230 a failed structured-output extraction must be distinguishable from an
  empty one
- #4232 a bank template must round-trip through the SDK (the two skips removed)

Deliberately not xfail: an expected-failure marker keeps the run green, so
nothing forces the question, and it outlives the bug by months.

They sit in one file because they are temporary — when an issue lands, its test
moves into the story it belongs to and the file shrinks. When it is empty,
delete it.

* test(system): fold the open-defect tests back into their stories

All five issues are fixed on main, so the temporary defect file has done its job
and is deleted. Each contract moves to where a reader would look for it:

- #4217 trace anchor -> story 07 (temporal)
- #4218 typed list rows -> story 10, plus ~70 call sites across the suite that
  had been reading rows as dicts and now use attribute access
- #4221 async wrapper parity -> a new story 51, since it is a contract about the
  published client rather than about the server
- #4230 structured-output failure is reported -> story 44
- #4232 template round trip -> story 54 (the two skips were already removed)

Each keeps a line naming the issue it came from, so the history stays readable
without the file that tracked it.

Note for anyone converting dict access after a typing change: only *list rows*
became models. Trace payloads, score components and mental-model history entries
are still plain dicts, and a blanket regex over `x["field"]` rewrites those too —
it did here, and turned ten passing tests red before being walked back.

* docs(review): require a system story for new user-facing capabilities

The system suite only stays useful if it grows with the product, and nothing was
asking for that. Adds step 6b to the code-review skill and a pointer in
CLAUDE.md's Testing section so the rule is visible before code is written, not
only at review.

The trigger is deliberately about *composition*, not size: a change needs a
story when it adds a capability someone can name, or when it makes two existing
capabilities meet for the first time. That second case is the one the ~500-file
api-slim suite structurally cannot cover, and where every bug this suite was
built for actually lived.

Step 6b also carries the review checks the suite's own conventions depend on,
each learned by getting it wrong here: go through the published client (reaching
around it hid that import_bank_template was uncallable from every SDK, #4232),
declare every LLM call, await background work rather than disabling it, assert
the whole deterministic payload, and make an unmet contract *fail* rather than
be documented — neither xfail nor a test pinning today's wrong answer.

Also drops `extra_env` from start_hindsight_server: a parameter no caller ever
passed, found by this review.
2026-09-10 10:09:31 +02:00

452 lines
20 KiB
Markdown
Raw Blame History

# CLAUDE.md
This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
## Project Overview
Hindsight is an agent memory system that provides long-term memory for AI agents using biomimetic data structures. Memories are organized as:
- **World facts**: General knowledge ("The sky is blue")
- **Experience facts**: Personal experiences ("I visited Paris in 2023")
- **Mental models**: Consolidated knowledge synthesized from facts ("User prefers functional programming patterns")
## Development Commands
### Local Development (API + UI)
```bash
# Start both API server and control plane UI
./scripts/dev/start.sh
```
### API Server (Python/FastAPI)
```bash
# Start API server only (loads .env automatically)
./scripts/dev/start-api.sh
# Run all tests (parallelized with pytest-xdist)
cd hindsight-api-slim && uv run pytest tests/
# Run specific test file
cd hindsight-api-slim && uv run pytest tests/test_http_api_integration.py -v
# Run single test function
cd hindsight-api-slim && uv run pytest tests/test_retain.py::test_retain_simple -v
# Lint and format
cd hindsight-api-slim && uv run ruff check .
cd hindsight-api-slim && uv run ruff format .
# Type checking (uses ty - extremely fast type checker from Astral)
cd hindsight-api-slim && uv run ty check hindsight_api/
```
### Control Plane (Next.js)
```bash
./scripts/dev/start-control-plane.sh
# Or manually:
cd hindsight-control-plane && npm run dev
```
### Documentation Site (Docusaurus)
```bash
./scripts/dev/start-docs.sh
```
### Generating Clients/OpenAPI
```bash
# Regenerate OpenAPI spec after API changes (REQUIRED after changing endpoints)
./scripts/generate-openapi.sh
# Regenerate all client SDKs (Python, TypeScript, Rust)
./scripts/generate-clients.sh
```
### Benchmarks
```bash
# Accuracy benchmarks
./scripts/benchmarks/run-longmemeval.sh
./scripts/benchmarks/run-locomo.sh
# Performance benchmarks
./scripts/benchmarks/run-perf-test.sh # System perf (mock LLM + pg0)
./scripts/benchmarks/run-perf-test.sh --scale tiny # Quick smoke test
./scripts/benchmarks/run-consolidation.sh
# Results viewer
./scripts/benchmarks/start-visualizer.sh # View results at localhost:8001
```
## Architecture
### Monorepo Structure
- **hindsight-api-slim/**: Core FastAPI server with memory engine (Python, uv)
- **hindsight-control-plane/**: Admin UI (Next.js, npm)
- **hindsight-cli/**: CLI tool (Rust, cargo, uses progenitor for API client)
- **hindsight-clients/**: Generated SDK clients (Python, TypeScript, Rust)
- **hindsight-docs/**: Docusaurus documentation site
- **hindsight-integrations/**: Framework integrations (LiteLLM, CrewAI, LangGraph, Pydantic AI, AG2, Claude Code, etc.)
- **hindsight-dev/**: Development tools and benchmarks
### Core Engine (hindsight-api-slim/hindsight_api/engine/)
- `memory_engine.py`: Main orchestrator for retain/recall/reflect operations
- `llm_wrapper.py`: LLM abstraction supporting OpenAI, Anthropic, Gemini, VertexAI, Groq, MiniMax, Ollama, LM Studio, LiteLLM, Claude Code, GitHub Copilot
- `embeddings.py`: Embedding generation (local sentence-transformers or TEI)
- `cross_encoder.py`: Reranking (local or TEI)
- `entity_resolver.py`: Entity extraction and normalization
- `query_analyzer.py`: Query intent analysis
**retain/**: Memory ingestion pipeline
- `orchestrator.py`: Coordinates the retain flow
- `fact_extraction.py`: LLM-based fact extraction from content
- `link_utils.py`: Entity link creation and management
**search/**: Multi-strategy retrieval
- `retrieval.py`: Main retrieval orchestrator
- `graph_retrieval.py`: Graph retrieval abstract base class
- `link_expansion_retrieval.py`: Link expansion graph retrieval
- `fusion.py`: Reciprocal rank fusion for combining results
- `reranking.py`: Cross-encoder reranking
### API Layer (hindsight-api-slim/hindsight_api/api/)
- `http.py`: FastAPI HTTP routers for all REST endpoints
- `mcp.py`: Model Context Protocol server implementation
Main operations:
- **Retain**: Store memories, extracts facts/entities/relationships
- **Recall**: Retrieve memories via 4 parallel strategies (semantic, BM25, graph, temporal) + reranking
- **Reflect**: Disposition-aware reasoning using memories and mental models.
### Database
PostgreSQL with pgvector. Schema managed via Alembic migrations in `hindsight-api-slim/hindsight_api/alembic/`. Migrations run automatically on API startup.
Key tables: `banks`, `memory_units`, `documents`, `entities`, `entity_links`
### Adding Database Migrations
Hindsight runs the same Alembic tree against PostgreSQL and Oracle 23ai. Each
migration file dispatches through `run_for_dialect`, which calls either
`_pg_upgrade` or `_oracle_upgrade` based on the live connection. A pytest lint
(`tests/test_migration_shape.py`) fails CI if a migration omits the dispatcher.
1. **Create a new migration file** in `hindsight-api-slim/hindsight_api/alembic/versions/`:
- File name format: `<revision_id>_<description>.py` (e.g., `f1a2b3c4d5e6_add_new_index.py`)
- Use a unique hex revision ID (12 chars)
- Set `down_revision` to the previous migration's revision ID
2. **Migration template** (the `script.py.mako` template scaffolds this; fill in the bodies):
```python
"""Description of the migration
Revision ID: f1a2b3c4d5e6
Revises: <previous_revision_id>
Create Date: YYYY-MM-DD
"""
from collections.abc import Sequence
from alembic import context, op
from hindsight_api.alembic._dialect import run_for_dialect
revision: str = "f1a2b3c4d5e6"
down_revision: str | Sequence[str] | None = "<previous_revision_id>"
branch_labels: str | Sequence[str] | None = None
depends_on: str | Sequence[str] | None = None
def _pg_schema_prefix() -> str:
"""Schema-qualifier for raw SQL on PG (multi-tenant search_path)."""
schema = context.config.get_main_option("target_schema")
return f'"{schema}".' if schema else ""
def _pg_upgrade() -> None:
schema = _pg_schema_prefix()
op.execute(f"CREATE INDEX ... ON {schema}table_name(...)")
def _pg_downgrade() -> None:
schema = _pg_schema_prefix()
op.execute(f"DROP INDEX IF EXISTS {schema}index_name")
def _oracle_upgrade() -> None:
# Oracle 23ai equivalent. Use op.get_bind().exec_driver_sql for forms
# that Alembic core does not model (vector/text indexes, partitions).
op.execute("CREATE INDEX ... ON table_name(...)")
def _oracle_downgrade() -> None:
op.execute("DROP INDEX IF EXISTS index_name")
def upgrade() -> None:
run_for_dialect(pg=_pg_upgrade, oracle=_oracle_upgrade)
def downgrade() -> None:
run_for_dialect(pg=_pg_downgrade, oracle=_oracle_downgrade)
```
**Dialect-only migrations.** If a change genuinely doesn't apply to one
dialect (e.g. enabling `pg_trgm` is PG-only), omit the unused slot:
```python
def upgrade() -> None:
run_for_dialect(pg=_pg_upgrade) # oracle slot intentionally absent → no-op
```
Make the asymmetry deliberate. Don't leave an Oracle slot empty just because
you didn't think about it — copy-pasting a PG migration without the Oracle
half is exactly how schemas drift.
3. **Run migrations locally**:
```bash
# Set database URL and run migrations for the base schema plus all tenants
uv run hindsight-admin run-db-migration
# Run on a specific tenant schema
uv run hindsight-admin run-db-migration --schema tenant_xyz
```
## Key Conventions
### Code Quality
**Before writing code, read `.claude/skills/code-review/SKILL.md`** for the full coding standards (Python style, type safety, TypeScript style, general principles).
**Always run the lint script after making Python or TypeScript/Node changes:**
```bash
./scripts/hooks/lint.sh
```
Dead-code detection runs in CI (the `check-unused-code` job) at two levels:
- **Blocking:** unused imports (ruff `F401`) and variables (`F841`) — `lint.sh` auto-removes
them and `verify-generated-files` fails on any leftover diff; and **knip** for orphaned
control-plane files / unused (or unlisted) `package.json` dependencies.
- **Advisory:** whole unused Python functions (vulture) and unused control-plane *exports*
(the shadcn/ui surface is kept on purpose) — surfaced, not gated.
Run both locally with:
```bash
./scripts/hooks/check-unused.sh
```
**After completing any implementation work, run `/code-review`** to verify your changes against project standards (missing tests, dead code, type safety, etc.). Fix any "must fix" issues before considering the task done.
**MANDATORY: Run `/code-review` before pushing code or creating a pull request.** Do not push or create a PR until all "must fix" issues are resolved.
### Testing
Most tests are deterministic (MockLLM, pure functions) — assert directly.
**A user-facing capability needs a blackbox story in `hindsight-system-tests/`.**
Those drive a real `hindsight-api` process through the published Python client — no
engine access, no SQL — and exist to catch the bugs the ~500-file api-slim suite
structurally cannot: the ones in the seam *between* steps, where consolidation wipes
the facts under it or a transfer drops its evidence. Add a story when a change adds a
capability someone can name, or makes two existing capabilities meet for the first
time; the composition is the part nothing else tests. See that package's README, and
`.claude/skills/code-review/SKILL.md` step 6b for the review checklist.
**Tests that verify LLM behaviour use a real LLM + an LLM-as-judge.** When the thing under test is *how the model interprets a prompt* (classification, attribution, dimension preservation, instruction-following), MockLLM can't simulate it and exact string/enum asserts flake across providers and runs. Use this pattern instead:
1. Mark the test module `pytestmark = pytest.mark.hs_llm_core` (single-provider; CI runs it in the core-LLM job). Use `hs_llm_mat` only for provider-matrix acceptance tests.
2. Call the real pipeline (`LLMConfig.from_env()`, `_get_raw_config()`), e.g. `extract_facts_from_text(...)`.
3. Assert with the judge, not string matching:
```python
from tests.llm_judge import assert_meets_criteria
facts_summary = "\n".join(f"- [{f.fact_type}] {f.fact}" for f in facts)
await assert_meets_criteria(
response=facts_summary,
criteria="The first-person user statements are classified 'world' and attributed to the user, not the agent.",
context="What the input said and who was speaking.",
)
```
Rules of thumb:
- **Judge anything non-deterministic** — including `fact_type` classification and speaker attribution. Do NOT hard-assert `fact_type == "..."`; pass a `[fact_type] fact` summary to the judge instead. Structural facts that ARE deterministic (counts, presence of a field, that a substring was injected into a prompt) stay as direct asserts in fast unit tests.
- **Split the test surface**: cover the deterministic mechanics (prompt assembly, suppression logic) with fast non-LLM unit tests, and the model-following behaviour with one `hs_llm_core` judge test. (Example pair: `test_narrator_resolution.py` + `test_narrator_context_override.py`.)
- The judge model is independent of the test provider (defaults to Gemini); never judge with the same call you're testing.
### Memory Banks
- Each bank is an isolated memory store (like a "brain" for one user/agent)
- Banks have dispositions (skepticism, literalism, empathy traits 1-5) affecting reflect
- Banks can have background context
- Bank isolation is strict - no cross-bank data leakage
### API Design
- All endpoints operate on a single bank per request
- Multi-bank queries are client responsibility to orchestrate
- Disposition traits only affect reflect, not recall
### Control Plane API Routes
When adding or modifying parameters in the dataplane API (hindsight-api), you must also update the control plane routes that proxy to it:
1. **API Routes** (`hindsight-control-plane/src/app/api/`):
- `recall/route.ts` - proxies to `/v1/default/banks/{bank_id}/memories/recall`
- `reflect/route.ts` - proxies to `/v1/default/banks/{bank_id}/reflect`
- `memories/retain/route.ts` - proxies to `/v1/default/banks/{bank_id}/memories/retain`
- Other routes follow the same pattern
2. **Client types** (`hindsight-control-plane/src/lib/api.ts`):
- Update the TypeScript type definitions for `recall()`, `reflect()`, `retain()` etc.
3. **Checklist when adding new API parameters**:
- Add parameter extraction in the route handler (destructure from `body`)
- Pass the parameter to the SDK call
- Update the client type definition in `lib/api.ts`
- Update any UI components that need to use the new parameter
### Harness Attribution (which coding agent wrote a document)
`hindsight-integrations/hindsight-coding-agents/` stamps the coding agent on every
document it retains, so the control plane can show its logo instead of another
`key=value` chip:
- `metadata.harness = "<id>"` — the authoritative field
- tag `harness:<id>` — the same value, so the documents list can filter on it
The ids are defined by that integration's HookSpecs
(`src/harness/hook-lifecycle.ts`) plus the persistent-plugin entrypoints
registered in `src/harness/registry.ts`, whose id is their
`createPluginEntry(...)` argument — currently `antigravity-cli`, `claude-code`,
`cline-cli`, `codex`, `copilot-cli`, `cursor-cli`, `devin-cli`, `grok-build`,
`kilo`, `opencode`, `opencode2`.
The control plane resolves the value in
`hindsight-control-plane/src/lib/harness-logo.ts` (metadata wins over the tag) and
renders it with `components/ui/harness-logo.tsx` in the documents table and the
document detail dialog. **Adding a harness to the integration means adding it to
that registry in the same change**: copy its icon from
`hindsight-docs/static/img/icons/` (or take it from the agent's own brand assets
when the docs site carries none) into
`hindsight-control-plane/public/img/harness/` and add one entry. Don't register
ids nothing writes — a test asserts the registry matches the emitted set, plus an
explicit list of retired ids kept so already-retained documents keep their logo.
An unregistered harness is not an error: it renders no logo and still shows as
ordinary metadata.
### Coding-agents docs are generated from one README
`hindsight-integrations/coding-agents/README.md` is the single source for that package's
configuration. **Never edit `skill/SKILL.md` or the docs page by hand** — both are generated:
```bash
cd hindsight-integrations/coding-agents && npm run skill:build # README -> skill/SKILL.md
node hindsight-docs/scripts/sync-coding-agents-doc.mjs # README -> docs page
```
The regions the skill copies are marked `<!-- skill:begin -->` / `<!-- skill:end -->` in the README;
the agent-only half (tools, crediting, corrections) lives in `skill-src/preamble.md`.
`src/docs-freshness.test.ts` fails on a stale skill or an undocumented `RawConfig` field. Run
`./scripts/hooks/lint.sh` BEFORE regenerating — prettier re-pads the README's tables, and a
generated file built from unformatted source fails the byte comparison in CI.
### Adding New Integrations
Every new integration in `hindsight-integrations/` must satisfy all of the following before it can be merged:
1. **Tests are required** — tests must simulate or exercise the external system (mock the framework's interfaces and verify the integration actually calls Hindsight correctly). Pure unit tests of helper functions are not sufficient.
2. **CI job** — add a test job in `.github/workflows/test.yml` following the existing pattern (e.g., `test-crewai-integration`). The job must build, install deps, and run `uv run pytest tests -v`. Also add the integration to `detect-changes` outputs so it only runs when its files change.
3. **Release process** — add the integration name to the `VALID_INTEGRATIONS` array in `scripts/release-integration.sh` so it can be released via the standard release workflow.
4. **Follow project code standards** — Python style, type safety, no raw dicts for structured data, no multi-item tuple returns (see `.claude/skills/code-review/SKILL.md`).
If any of these are missing, the integration is incomplete and must not be pushed or merged.
### Changelogs
Never add "Unreleased" entries to changelogs (e.g. `hindsight-docs/src/pages/changelog/**`). Changelog entries are written by the release script (`./scripts/release-integration.sh`) when a version is actually cut. If a bug fix or feature needs documenting before release, describe it in the PR/commit — the release tooling will surface it in the published changelog section.
### Adding New API Configuration Flags
Configuration follows a hierarchical system: **Global (env vars) → Tenant (via extension) → Bank (database)**.
Fields must be categorized as either **hierarchical** (can be overridden per-tenant/bank) or **static** (server-level only).
#### Adding a New Configuration Field
1. **config.py** (`hindsight-api-slim/hindsight_api/config.py`):
- Add `ENV_*` constant for the environment variable name (e.g., `ENV_MY_SETTING = "HINDSIGHT_API_MY_SETTING"`)
- Add `DEFAULT_*` constant for the default value
- Add field to `HindsightConfig` dataclass with type annotation
- **Mark as configurable** by adding to `_CONFIGURABLE_FIELDS` set if the field should be overridable per-tenant/bank via API
- Add initialization in `from_env()` method
```python
# Configurable field (can be overridden per-tenant/bank via API)
_CONFIGURABLE_FIELDS = {
...,
"my_setting", # Add here for configurable
}
# Static field - just don't add to _CONFIGURABLE_FIELDS
```
2. **main.py** (`hindsight-api-slim/hindsight_api/main.py`):
- No change is needed for ordinary environment-backed config fields. The CLI starts from `_get_raw_config()`,
so new `HindsightConfig` fields are carried through automatically.
- If the new field should be overridable by a CLI flag, add the argparse option in `_parse_cli_args()` and include
that field in the `dataclasses.replace(config, ...)` call near the "CLI override" comment.
3. **Use hierarchical config in MemoryEngine**:
```python
# Config is resolved automatically per bank via ConfigResolver
config_dict = await self._config_resolver.get_bank_config(bank_id, context)
value = config_dict["my_setting"]
```
4. **Use static config** (non-hierarchical):
```python
from ...config import get_config
config = get_config()
value = config.my_static_field
```
5. **Documentation** (`hindsight-docs/docs/developer/configuration.md`):
- Add to appropriate section table with Variable, Description, Default
- Mark if it's hierarchical (can be overridden per-bank)
6. **Env template** (`.env.example`):
- Add the variable to the appropriate section, commented if optional, with a
short inline comment describing it (mirror the documentation entry).
- This file is the single source of truth for the env template:
`scripts/dev/setup.sh` copies it to `.env`, and `hindsight-embed` ships a
bundled copy (`hindsight-embed/hindsight_embed/env.example`) that seeds
embed/profile configs. After editing `.env.example`, re-copy it to the
embed package (`cp .env.example hindsight-embed/hindsight_embed/env.example`)
or the `test_bundled_template_matches_repo_root` sync test will fail.
#### Hierarchical vs Static Guidelines
**Hierarchical** (per-bank overridable):
- LLM settings (provider, model, API key, base URL)
- Operation-specific settings (retain mode, chunk size, etc.)
- Feature flags that vary by customer/bank
**Static** (server-level only):
- Infrastructure settings (database URL, port, host)
- Global limits (max concurrent operations)
- System-wide feature flags
## Environment Setup
```bash
cp .env.example .env
# Edit .env with the LLM provider/model and credentials for your setup
# Python deps
uv sync --directory hindsight-api-slim/
# Node deps (uses npm workspaces)
npm install
```
Common LLM settings:
- `HINDSIGHT_API_LLM_PROVIDER`: openai, anthropic, gemini, groq, minimax, ollama, lmstudio
- `HINDSIGHT_API_LLM_API_KEY`: API key for providers that require one
- `HINDSIGHT_API_LLM_MODEL`: Model name (defaults are provider-specific)
Optional (uses local models by default):
- `HINDSIGHT_API_EMBEDDINGS_PROVIDER`: local (default) or tei
- `HINDSIGHT_API_RERANKER_PROVIDER`: local (default) or tei
- `HINDSIGHT_API_DATABASE_URL`: External PostgreSQL (uses embedded pg0 by default)
- `HINDSIGHT_API_ENABLE_BANK_CONFIG_API`: Allow per-bank config *writes* (default: true; reads are always allowed)
Reference in New Issue View Git Blame Copy Permalink