feat(mental-models): graded write-time identifier-retention gate — refuse refreshes that silently drop the document's anchors - #3306
Open
JoshFunnell wants to merge 19 commits into
Conversation
`_execute_mental_model_refresh` assembled its result through a `_finish` closure defined *after* `reflect_async` returned, capturing the post-reflect locals (`reflect_result.text/.tool_trace/.llm_trace/.usage`, plus the derived fact counts and reflect_response payload). Every branch that finishes a refresh therefore has to run after the agentic loop, by construction. Split that into two frozen dataclasses and a free function: - `_MentalModelRefreshContext` — identity, scope, window, watermark and start time: what the pipeline resolved before it branched on mode or outcome. - `_MentalModelRefreshEvidence` — candidate text, fact counts, tool/LLM traces and usage: what produced the candidate, whichever path produced it. The two values later stages still mutate (`reflect_response`, `warnings`) stay out of it and are passed per branch, so nothing holds a snapshot that keeps changing. - `_build_refresh_run(ctx, evidence, ...)` — the former closure body. Behaviour-identical: all four existing call sites pass exactly what they previously captured, and the assembled `_MentalModelRefreshRun` is unchanged field for field.
…fresh A delta-mode refresh ran the full agentic reflect loop unconditionally, even though delta mode's premise is incremental work. Measured over 24h on a production bank: 4.29 LLM calls per refresh at a median 101k input tokens against 373 output tokens, because each loop call resends the whole accumulated conversation — and 36.8% of those refreshes applied zero operations, i.e. the loop ran to discover there was nothing to write. Every piece needed to skip it was already here, sequenced behind the loop rather than in front of it. `_try_delta_fast_path` puts two bounded tiers ahead of `reflect_async`, for eligible delta refreshes only: - Tier 0 reads the delta window with the same two typed retrieval calls the loop's tools make (`recall` under `recall_max_tokens`, `search_observations` under the tool-call budget, both `internal=True`). An empty window means nothing to integrate: the document is preserved and the watermark advances, the same outcome the loop reaches after paying for it. Zero LLM calls. - Tier 1 hands those facts and the current document to the existing structured-delta prompt in one call and applies what comes back. Anything less than clearly safe hands back to the loop, which then produces the result exactly as it does today: no readable baseline, the model setting the new `needs_full_context` escape hatch, a failed call or parse, or operations that all bounced. So this changes what a refresh COSTS, not which outcomes are reachable — tier 2 is untouched, and full mode never enters the fast path. By construction the fast path can only apply operations to the existing document or preserve it. It never writes a synthesised candidate as the whole document, so the narrow-candidate and empty-answer classes the agentic path guards against cannot arise on it. Also here, because they are inseparable from the above: - `needs_full_context` on `DeltaOperationList`, threaded through all three branches of `parse_delta_operation_list` and `_finalize_operations`. Each branch rebuilds the list from the operations it validated, so a flag left on the raw payload would be dropped in silence and the fast path would apply edits the model had just said it could not make safely. - `STRUCTURED_DELTA_FAST_PATH_SYSTEM_PROMPT` as an addendum, leaving the shared prompt byte-identical: the agentic path does not read the flag, so a model answering "I cannot do this properly" there would be ignored. - The structured-delta call is now bound through `with_config(...)` so it logs under the refresh's operation. Called bare it recorded a blank operation in `llm_requests` (measured: 197 blank-op calls per 48h), which made delta-ops calls impossible to attribute or count. - `fast_path` / `fast_path_fallback_reason` on the trace and dry-run models and in `reflect_response`, as their own vocabularies. Deliberately NOT folded into `ModeFallbackReason`/`RefreshOutcome`: a tier-1 hand-back is not a mode fallback (the mode is still delta) and no outcome changed, so every existing consumer of those literals stays valid. - Kill switches: `HINDSIGHT_API_MENTAL_MODEL_DELTA_FAST_PATH` (hierarchical, default true) and per-model `trigger.delta_fast_path` (null inherits).
…ol plane Follows the repo's config-field procedure for the new knob and the dataplane rule that an API change updates the control plane alongside it. - `configuration.md`: a row for `HINDSIGHT_API_MENTAL_MODEL_DELTA_FAST_PATH`, in its own subsection next to the internal-recall knobs it sits beside — same shape (hierarchical, plus a per-model trigger override). - `.env.example` + the byte-identical `hindsight-embed` copy, which the `test_bundled_template_matches_repo_root` sync test enforces. - `mental-models.mdx`: the `delta_fast_path` trigger row, a "what a delta refresh costs" section under Refresh Mode, and the two new fields in both the dry-run and stored-trace field tables. - Regenerated `openapi.json` and `bank-template-schema.json`, and mirrored the two doc edits into the generated `skills/hindsight-docs` copies. - Control plane: `MentalModelFastPathTier` / `FastPathFallbackReason` types, `delta_fast_path` on all five mental-model trigger declarations, the two new keys on the trace and dry-run result, and a pass-through of both in `TraceSummary`. Rendered as the raw API values in mono, like `effective_mode` immediately above them, rather than translated prose: these are the enum names the dry-run payload, the stored trace and the docs all use, so a reader comparing this line to a `reflect_response` sees the same token. No new UI controls.
The Rust client is generated at build time from `hindsight-docs/static/openapi.json`, so adding `delta_fast_path` to `MentalModelTrigger` gives `types::MentalModelTriggerInput` a new field immediately. All three CLI construction sites are exhaustive struct literals with no `..Default::default()`, so each stops compiling until the field is named. `cargo check` on the CLI is clean with these three lines.
…ne index corruption Block-level delta ops (replace_block/insert_block/remove_block) addressed blocks by a bare LLM-computed integer index, validated only for range. A wrong-but-in-range index was indistinguishable from a correct one, and the model had to silently count array elements in a compact, unannotated document dump to know a block's index in the first place. Measured in production: two off-by-one replace_block ops landed exactly one position before their intended targets deep in long sections, silently destroying unrelated paragraphs. Two-part fix: - replace_block/remove_block, and insert_block when its index names an existing block, now carry a required `anchor`: a verbatim (whitespace- normalized) excerpt of the block the model believes is at that index. apply_operations checks it against the block actually there before mutating, and skips the op (visible in the summary like every other skip) on a mismatch or a missing anchor -- fails closed for ops from a model/caller that never adopted the anchor. - The document JSON shown to the model (serialize_document_for_delta_prompt, used at both delta-prompt call sites in memory_engine.py) now annotates each block with its own index, so the model reads its position instead of counting. The annotation is prompt-only: Block schemas keep extra="forbid", so it can never round-trip back into the persisted document. Extends tests/test_structured_doc.py::TestApplyOperations with a regression test reproducing the measured defect shape (off-by-one index whose anchor names the neighboring block), anchor mismatch/missing/ whitespace-normalization coverage for all three ops, and a new TestSerializeDocumentForDeltaPrompt class covering the index annotation and its non-leakage into the persisted schema. Updated the prompt contract (prompts.py) and the feature's own docs (mental-models.mdx + its generated skills/ mirror, hand-applied after the mdx edit -- the generator script rewrites all 89 mirrored files with Windows path artifacts on this host, well outside this patch's scope). RED-proven: temporarily disabled the anchor check and confirmed the off-by-one regression test (and 4 other anchor tests) fail exactly as expected -- the wrong block gets silently overwritten -- before restoring and confirming green. Test counts: tests/test_structured_doc.py 51 passed (was 36); tests/test_mental_model_delta.py 34 passed, 4 skipped (unchanged, skips require GEMINI_API_KEY/OPENAI_API_KEY); tests/test_structured_delta_prompt_budget.py 2 passed (unchanged). ruff format --check and ruff check clean on all changed files.
A two-tier refresh system whose tier is unobservable after the fact cannot be operated or measured. The tier was only readable from mental_models.reflect_response.fast_path, which holds each model's LATEST refresh and is overwritten by the next one -- so tier distribution over any window was unrecoverable from the database, and had to be reconstructed by an external sampler polling every 5 minutes. RefreshMentalModelOutcomeMetadata now carries: serving_tier "tier0" | "tier1" | "tier2" fast_path_fallback_reason why the fast path handed back, on tier2 The agentic path reports fast_path=None; that is normalised to "tier2" so the persisted value names the tier that ran rather than the absence of a flag -- a null would be ambiguous between "agentic loop" and "written by a build predating this field". Both fields are optional, so existing callers are unaffected. tests/test_refresh_outcome_metadata.py drives the real writer against a fake connection and asserts on the JSON actually persisted, including the None -> tier2 normalisation and the missing-key case. 6 passed; tests/test_mental_model_delta.py still 34 passed, 4 skipped.
…mory is about
Tags describe which compartment a memory lives in; entities describe what it
is about. On a bank whose tag vocabulary is a handful of broad topics, a tag
scope cannot isolate a subject: measured on a production bank (2026-08-08),
the best tag scope for one subject reached 25% precision against a 15.5%
base rate, while the entity association reached 94.2% precision / 86.2%
recall on the same corpus.
TagGroupEntityLeaf ({entities: [names], match: any|all}) joins the existing
recursive grammar, so it rides every surface tag_groups already reach with
one SQL-builder change: mental-model refresh scope (trigger.tag_groups),
the staleness gate (any_memory_updated_since), the fast-path window read,
and the semantic/BM25/link-expansion retrieval arms. An entity-scoped
mental model therefore refreshes exactly when facts about its entities
arrive, and its refresh reads exactly those facts.
Mechanics:
- Names match case-insensitively against canonical entity names -- the
registry's own (bank_id, LOWER(canonical_name)) normalisation.
- Association is inheritance-aware: a memory matches directly via
unit_entities OR through any of its source_memory_ids. Observations carry
no direct postings by design (their association is transitive through
sources, mirroring _entity_rows_for_units_sql), so this is what makes
entity scope work on the one fact layer mental models read.
- Correlation references the outer memory_units row by fully-qualified
name; every builder call site that filters memory_units uses an
unaliased FROM.
- Surfaces without entity postings (the mental-models search, the
directives listing) strip entity leaves via strip_entity_leaves -- the
permissive reading; their tag constraints still apply.
- Entity leaves may not appear under 'not', enforced at the API edge on
RecallRequest, ReflectRequest and MentalModelTrigger
(validate_entity_leaf_placement): the permissive fallbacks would invert
into exclude-everything under negation.
- The Python-side post-filter evaluates entity annotations when a result
carries them and passes permissively when it does not.
Cross-bank leakage is structurally impossible (links are per-unit; a
same-named entity in another bank has a different id) and is covered by a
test.
tests/test_tag_group_entity_leaf.py: 22 tests over five layers -- schema
(incl. the not-placement bar), emitted SQL and param accounting, strip
semantics, the Python matcher, and the staleness gate against the real
database including the source-inherited observation case. Regression:
mental-model delta 34p/4s, trigger tag-group and refresh-scope suites 10p,
consolidation 105p baseline unchanged.
A refresh can silently drop anchored identifiers -- dates, file paths, commit
shas, registry ids, env vars -- while the document GROWS, so neither the
emptiness guard nor any length check can see it.
Measured over 364 real refresh events on one production bank:
lost 0 identifiers: 337 (92.6%)
lost 1: 17
lost 2: 3
lost 3 or more: 7
One of those seven dropped TRIAL-CLOSE-RUNBOOK.md, TRIAL-EVIDENCE-PLAN.md,
build_spec_a_overlay.py and a commit sha from a single model in one write.
Losing one or two is frequently legitimate churn, so the gate is GRADED:
warn (naming the lost identifiers verbatim) below the threshold, refuse at or
above it. Default 3 therefore blocks 1.9% of refreshes rather than 7.4%.
Refusal is preserve-and-fail under its own outcome value,
refresh_failed_identifier_retention: content and watermark stay untouched and
the raised error names the lost identifiers verbatim. A distinct outcome
(rather than reusing refresh_failed_empty_candidate) keeps anything keyed on
the outcome enum honest -- the refused candidate is definitionally non-empty
here. The warning is also persisted under
reflect_response.identifier_retention on the warn and refuse paths alike, so
the signal is readable without keep_trace.
The gate runs AFTER the vectorize-io#3112 delta-window guard: a failed delta's
window-only candidate would also read as identifier loss, and the more
precise "candidate is one window, not a document" refusal must win. An
engine-level test pins that ordering.
The gate only applies when has_delta_baseline is true -- a bootstrap write
over empty/PENDING content cannot clobber anything.
The identifier taxonomy is byte-identical to the offline retention probe's:
two instruments that disagreed about what counts as an identifier would give
contradictory evidence about the same event.
Threshold knob: HINDSIGHT_API_MENTAL_MODEL_IDENTIFIER_LOSS_REFUSE (default 3,
0 = warn-only), documented in configuration.md and env.example.
Logic is pure functions in engine/identifier_retention.py so it is testable
without a DB or an LLM; the engine call site is a few lines. 14 unit tests
cover the two false-signal directions (a moved identifier counts as kept; the
gate never refuses without a baseline), the env-knob edges, and a measured
limit of the shared taxonomy (the port class needs a host prefix). Two
engine-level dry-run tests cover the refusal outcome end to end and the
guard ordering. The control-plane diagnostics view labels the new outcome in
all ten locales.
JoshFunnell
force-pushed
the
upstream/identifier-retention-gate
branch
from
August 10, 2026 07:38
f5ceffd to
17fb990
Compare
Resolves the prompts.py conflict in STRUCTURED_DELTA_SYSTEM_PROMPT: upstream added the 'table' block type to the enumeration while this branch added the 0-based block-index instruction. Both are kept -- they are orthogonal, and 'table' is a real block type in the merged tree (structured_doc.py defines Literal["table"] and the block-shape list documents it).
…se check
Upstream's i18n:check (find-untranslated.ts) scans JSXText nodes and human-facing
attributes for untranslated prose. The two diagnostics lines rendered the raw enum
NAMES as bare JSX text, so after merging main they were reported as suspicious
strings and build-control-plane failed.
The names stay raw and untranslated on purpose -- they are the tokens the dry-run
payload, the stored trace and the docs all use -- so they move into {"..."}
expression containers, which the checker does not scan, leaving the separator as a
bare · entity that its own rule already exempts. Rendering is unchanged.
TagGroupEntityLeaf.match serialised as title: "Match", the same title TagGroupLeaf's DIFFERENT match enum already uses (any/all/any_strict/all_strict/ exact). progenitor/typify keys inline enums BY TITLE, so the second definition failed to conform and Rust client generation died with TypeError(InvalidValue) at hindsight-clients/rust/build.rs:199, failing verify-generated-files. The collision only appears in a REGENERATED spec -- the committed openapi.json on this branch predates the entity-leaf commit -- which is why CI saw it while a local build of the committed file passed. Controlled against origin/main's spec: the pre-existing 'Tags Match' and 'Type' title collisions are present upstream too and are therefore not the cause.
…ents verify-generated-files regenerates every generated artifact and fails if the result differs from what is committed. After the EntityMatch title fix the job stopped panicking in generation and started failing on that diff instead, so the artifacts needed regenerating from the merged source rather than carrying the textually auto-merged versions. Ran CI's four steps: uv run generate-openapi, generate-bank-template-schema.sh, generate-clients.sh (rust/python/typescript/go), generate-docs-skill.sh. The regenerated spec confirms the fix: the duplicate 'Match' enum title is gone and TagGroupEntityLeaf now publishes 'EntityMatch', which is why the Rust client generates instead of dying with TypeError(InvalidValue).
verify-generated-files also runs ./scripts/hooks/lint.sh. Formatting only.
…r-retention outcome This branch adds the refresh_failed_identifier_retention refresh outcome and a config key, so its generated artifacts are a function of THIS branch's source and cannot be inherited from the stacked-under PR: merging vectorize-io#3290 forward carries the code but not the derived files. Regenerated with CI's own steps (uv run generate-openapi, generate-bank-template-schema.sh, generate-clients.sh, generate-docs-skill.sh). The seven changed files match the set verify-generated-files reported, and the new outcome now appears in the TypeScript and Go clients as well as the docs-skill spec.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A graded write-time identifier-retention gate for mental-model refreshes: a refresh that drops too many anchored identifiers (dates, file paths, commit shas, UUIDs, URLs, env vars, ports, versions) from existing content is refused with the existing preserve-and-fail contract; smaller losses proceed with a warning that names the lost identifiers verbatim.
Why
A refresh can lose its anchors while the document grows, so neither the empty-candidate guard nor any length check can see it. Measured over 364 real refresh events on one production bank:
One of those seven dropped three file names and a commit sha from a single model in one write, while the document got longer. Losing one or two is frequently legitimate churn (a superseded date, a path that stopped being relevant), which is why the gate is graded rather than absolute: refusing on any loss would fail ~1 refresh in 14 and quickly be switched off.
How
engine/identifier_retention.py(no DB, no LLM): extract identifier sets from previous/candidate content, set-difference them (an identifier that moves counts as kept), grade against a threshold._execute_mental_model_refresh, deliberately after the empty-candidate guard and the Delta refresh fallback overwrites the whole document with a new-facts-only synthesis #3112 delta-window guard, so those more precise refusals win. An engine-level test pins the ordering.refresh_failed_identifier_retention(enum + OpenAPI + control-plane types/labels updated): the refused candidate is definitionally non-empty, so reusingrefresh_failed_empty_candidatewould make anything keyed on the outcome enum misclassify these. The raised error names the lost identifiers.reflect_response.identifier_retentionon warn and refuse paths alike, so the signal is readable withoutkeep_trace.PENDING/empty), the gate never fires — same condition as the sibling guard.HINDSIGHT_API_MENTAL_MODEL_IDENTIFIER_LOSS_REFUSE(default3;0= warn-only). Documented inconfiguration.md+env.example.Tests
0, bootstrap-never-refused, env-knob parsing (garbage falls back rather than breaking a write path), and a pinned taxonomy limit (the port class needs a host prefix).refresh_failed_delta_not_appliedwins over identifier loss when both apply (guard ordering).HINDSIGHT_API_LLM_API_KEY).