fix(consolidation): respect bank mission over ephemeral-state heuristic (#525)

* Add Hindsight as git subtree + BCGU noise filtering tests

Adds hindsight server source as a subtree under hindsight-api/ so we
can iterate on server-side fixes directly.

test_bcgu_noise_filtering.py proves that a well-crafted
retain_custom_instructions (BCGU_RETAIN_MISSION) can suppress
talking-head noise at fact extraction time — eliminating the need for
client-side --filter-vision-noise preprocessing.

Tests cover:
- Default mode extracts 3 noise facts from talking-head frame (problem documented)
- BCGU mission produces 0 noise facts from same talking-head frame
- BCGU mission still extracts 2 high-value ChatGPT screen facts correctly
- Mixed doc (2 talking-head + 2 screen): 0% noise ratio with BCGU mission
- Pure talking-head doc: 0 facts extracted

All 5 tests pass in ~32s using gpt-4o-mini.

* fix(consolidation): respect mission context over ephemeral-state heuristic

Two related fixes for the consolidation engine when a bank mission is
configured:

1. **Mission override for ephemeral-state filter** (`prompts.py`):
   The system prompt previously instructed the LLM to discard any fact
   that looked like "ephemeral state" (e.g. current position, transient
   actions).  When a mission is active the mission itself defines what is
   valuable — timestamped screen actions, session events, tool interactions
   may all be mission-critical even though they look ephemeral.  Added a
   MISSION OVERRIDE block that explicitly tells the LLM the mission takes
   priority over the generic ephemeral-state guidance.

2. **Remove contradictory durable-knowledge nudge** (`consolidator.py`):
   The user-prompt builder was injecting "Focus on DURABLE knowledge that
   serves this mission, not ephemeral state" alongside the mission text.
   This phrasing contradicted missions that intentionally capture
   timestamped events.  Replaced with a neutral directive that simply
   signals the mission overrides general rules.

3. **JSON control-character sanitisation** (`consolidator.py`):
   LLMs occasionally embed literal ASCII control characters (0x00–0x1f)
   inside JSON string values, causing `json.loads` to raise a
   JSONDecodeError.  Added a try/except that strips control characters
   and retries the parse before re-raising, preventing spurious failures.

* refactor(consolidation): move sanitize_llm_output to llm_wrapper, reuse in consolidator

- Add `sanitize_llm_output()` to `llm_wrapper.py` as the single canonical
  function for stripping characters that break downstream systems
  (ASCII control chars 0x00-0x08/0x0B-0x0C/0x0E-0x1F/0x7F and Unicode
  surrogates). Tab, newline, and carriage-return are preserved.
- Reduce `_sanitize_text()` in `fact_extraction.py` to a thin wrapper
  that delegates to `sanitize_llm_output()`.
- Update `consolidator.py` to import and call `sanitize_llm_output()`
  directly instead of reimplementing the logic inline.
- Remove test_bcgu_noise_filtering.py (should not have been committed).

* fix(consolidation): apply sanitize_llm_output to observation text fields

sanitize_llm_output was imported but unused after the old _call_llm_once
path was removed. The batch flow uses structured Pydantic output so
there's no raw json.loads call — instead, apply sanitization via
field_validator on _CreateAction.text and _UpdateAction.text so control
characters are stripped before observation text reaches the database.

* fix(entity-resolver): correct mention_count for new entities in batch retain

When the same entity (e.g. "Bob") appears across N items in a single batch
retain, _resolve_entities_batch_impl deduplicates them into one name group
before inserting, then queued only ONE _EntityStat regardless of N. The
flush therefore always incremented mention_count by 1 beyond the INSERT
value — giving 2 for any number of mentions.

Two-part fix:
- INSERT with mention_count=0 so the post-transaction flush is the single
  source of truth for the count (avoids an off-by-one for N=1 as well).
- Append one _EntityStat per original mention (len(g.indices)) instead of
  one per unique name, so flush_pending_stats() adds the correct total N.

This makes the batch path consistent with the single-entity path, which
already accumulates one stat per mention via entities_to_update.
This commit is contained in:
Nicolò Boschi 2026-03-09 15:04:36 +01:00 committed by GitHub
parent f7a60f898d
commit 00ccf0b218
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 44 additions and 25 deletions

View file

@ -24,9 +24,10 @@ from datetime import datetime, timezone
from itertools import combinations
from typing import TYPE_CHECKING, Any
from pydantic import BaseModel
from pydantic import BaseModel, field_validator
from ...config import get_config
from ..llm_wrapper import sanitize_llm_output
from ..memory_engine import fq_table
from ..retain import embedding_utils
from .prompts import build_batch_consolidation_prompt
@ -45,12 +46,22 @@ class _CreateAction(BaseModel):
text: str
source_fact_ids: list[str] # memory UUIDs from the NEW FACTS list
@field_validator("text", mode="before")
@classmethod
def sanitize_text(cls, v: str) -> str:
return sanitize_llm_output(v) or ""
class _UpdateAction(BaseModel):
text: str
observation_id: str # UUID of the existing observation to update
source_fact_ids: list[str] # memory UUIDs from the NEW FACTS list
@field_validator("text", mode="before")
@classmethod
def sanitize_text(cls, v: str) -> str:
return sanitize_llm_output(v) or ""
class _DeleteAction(BaseModel):
observation_id: str # UUID of the observation to remove

View file

@ -29,7 +29,7 @@ Compare the facts against existing observations:
- Same topic as an existing observation UPDATE it (observation_id + source_fact_ids)
- New topic with durable knowledge CREATE a new observation (source_fact_ids)
- Cross-reference facts within the batch: a later fact may resolve a vague reference in an earlier one
- Purely ephemeral facts omit them (no create/update needed)"""
- Purely ephemeral facts omit them unless the MISSION above explicitly targets such data (e.g. timestamped events, session state, screen content)"""
# Output format — JSON braces escaped as {{ }} so .format() leaves them literal
_BATCH_OUTPUT_FORMAT = """

View file

@ -459,10 +459,12 @@ class EntityResolver:
entity_dates = [g.event_date for _, g in sorted_groups]
# INSERT ... ON CONFLICT DO NOTHING — no row lock on already-existing entities.
# mention_count starts at 0 here; flush_pending_stats() is the sole source of
# truth for mention counting (one stat per original mention in the batch).
inserted_rows = await conn.fetch(
f"""
INSERT INTO {fq_table("entities")} (bank_id, canonical_name, first_seen, last_seen, mention_count)
SELECT $1, name, COALESCE(event_date, now()), COALESCE(event_date, now()), 1
SELECT $1, name, COALESCE(event_date, now()), COALESCE(event_date, now()), 0
FROM unnest($2::text[], $3::timestamptz[]) AS t(name, event_date)
ON CONFLICT (bank_id, LOWER(canonical_name))
DO NOTHING
@ -489,7 +491,9 @@ class EntityResolver:
for row in existing_rows:
id_by_name[row["name_lower"]] = row["id"]
# Assign entity IDs back and queue for post-txn stats flush.
# Assign entity IDs back and queue one stat per original mention so that
# flush_pending_stats() increments mention_count by the true mention count,
# not just 1 per unique name.
for name_lower, g in sorted_groups:
entity_id = id_by_name.get(name_lower)
if entity_id:

View file

@ -48,6 +48,28 @@ _llm_max_concurrent = int(os.getenv(ENV_LLM_MAX_CONCURRENT, str(DEFAULT_LLM_MAX_
_global_llm_semaphore = asyncio.Semaphore(_llm_max_concurrent)
def sanitize_llm_output(text: str | None) -> str | None:
"""
Sanitize text by removing characters that break downstream systems.
Removes:
- ASCII control characters (0x00-0x08, 0x0B-0x0C, 0x0E-0x1F, 0x7F): break
json.loads and PostgreSQL UTF-8 encoding; tab (0x09), newline (0x0A), and
carriage return (0x0D) are preserved as they are valid in text and JSON.
- Unicode surrogates (U+D800-U+DFFF): Invalid in UTF-8, break LLM APIs
Surrogate characters are used in UTF-16 encoding but cannot be encoded
in UTF-8. They can appear in Python strings from improperly decoded data
(e.g., from JavaScript or broken files). Control characters commonly appear
in LLM output embedded inside JSON string values.
"""
if text is None:
return None
if not text:
return text
return re.sub(r"[\x00-\x08\x0b\x0c\x0e-\x1f\x7f\ud800-\udfff]", "", text)
class OutputTooLongError(Exception):
"""
Bridge exception raised when LLM output exceeds token limits.

View file

@ -15,7 +15,7 @@ from typing import Literal, cast
from pydantic import BaseModel, ConfigDict, Field, create_model, field_validator
from ...config import get_config
from ..llm_wrapper import LLMConfig, OutputTooLongError
from ..llm_wrapper import LLMConfig, OutputTooLongError, sanitize_llm_output
from ..response_models import TokenUsage
from .entity_labels import (
EntityLabelsConfig,
@ -66,25 +66,7 @@ def _infer_temporal_date(fact_text: str, event_date: datetime | None) -> str | N
def _sanitize_text(text: str | None) -> str | None:
"""
Sanitize text by removing characters that break downstream systems.
Removes:
- Null bytes (\\x00): Invalid in PostgreSQL UTF-8 encoding
- Unicode surrogates (U+D800-U+DFFF): Invalid in UTF-8, break LLM APIs
Surrogate characters are used in UTF-16 encoding but cannot be encoded
in UTF-8. They can appear in Python strings from improperly decoded data
(e.g., from JavaScript or broken files). Null bytes commonly appear in
OCR output, PDF extraction, or copy-paste from binary sources.
"""
if text is None:
return None
if not text:
return text
# Remove null bytes and surrogate characters
text = text.replace("\x00", "")
return re.sub(r"[\ud800-\udfff]", "", text)
return sanitize_llm_output(text)
class Entity(BaseModel):