mirror of https://github.com/vectorize-io/hindsight.git synced 2026-09-14 19:31:49 +08:00

Files

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

20 KiB

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)

# Start both API server and control plane UI
./scripts/dev/start.sh

API Server (Python/FastAPI)

# 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)

./scripts/dev/start-control-plane.sh
# Or manually:
cd hindsight-control-plane && npm run dev

Documentation Site (Docusaurus)

./scripts/dev/start-docs.sh

Generating Clients/OpenAPI

# 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

# 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.

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

Migration template (the script.py.mako template scaffolds this; fill in the bodies):

"""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:

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.

Run migrations locally:

# 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:

./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:

./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:

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.
Call the real pipeline (LLMConfig.from_env(), _get_raw_config()), e.g. extract_facts_from_text(...).

Assert with the judge, not string matching:

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:

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
Client types (hindsight-control-plane/src/lib/api.ts):
- Update the TypeScript type definitions for recall(), reflect(), retain() etc.
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:

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  /  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:

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.
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.
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.
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

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
```
# 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
```
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.

Use hierarchical config in MemoryEngine:

# 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"]

Use static config (non-hierarchical):

from ...config import get_config
config = get_config()
value = config.my_static_field

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)
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

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)

20 KiB Raw Blame History