Skip to content

fix: re-ingest row inflation — dedup by content_hash (#219) - #283

Merged
robotrocketscience merged 5 commits into
mainfrom
fix/issue-219-content-hash-dedup
Apr 29, 2026
Merged

fix: re-ingest row inflation — dedup by content_hash (#219)#283
robotrocketscience merged 5 commits into
mainfrom
fix/issue-219-content-hash-dedup

Conversation

@robotrocketscience

Copy link
Copy Markdown
Owner

Summary

  • Adds insert_or_corroborate() to MemoryStore: all ingest call sites now check for an existing belief by content_hash before inserting. Duplicate content arriving from a different source gets a corroboration row on the existing belief instead of a new row.
  • _maybe_consolidate_content_hash_duplicates(): one-shot migration that collapses pre-existing duplicate content_hash groups on first open, summing alpha/beta, propagating origin and lock precedence, rewriting FKs, and doing a bulk DELETE. Runs in < 2 s on a 20 K-belief store with ~2 K duplicate groups.
  • _maybe_apply_content_hash_unique(): table-swap migration that adds UNIQUE(content_hash) to beliefs (SQLite has no ALTER TABLE ADD CONSTRAINT). Uses PRAGMA table_info to preserve any extra columns from prior ALTER TABLE migrations.
  • Fresh stores: _SCHEMA now declares content_hash TEXT NOT NULL UNIQUE so new DBs get the constraint from DDL.

Closes #219

Commits (5 atomic)

  1. feat: insert_or_corroborate helper on MemoryStore
  2. refactor: route ingest paths through insert_or_corroborate
  3. feat: consolidate duplicate content_hash rows on first open
  4. feat: UNIQUE(content_hash) on beliefs table
  5. docs: re-ingest dedup contract

Test plan

  • uv run pytest tests/test_insert_or_corroborate.py -v — 7 tests for the new helper (insert-new, duplicate-hash, corroboration row, count, alpha/beta unchanged, bad source_type, bad type on duplicate)
  • uv run pytest tests/test_content_hash_consolidation.py -v — 9 tests for the migration (alpha/beta sum, dupe deleted, only canonical survives, FK rewrite, synthetic corroboration, idempotence, schema_meta marker, origin precedence, lock precedence)
  • uv run pytest tests/test_content_hash_unique.py -v — 5 tests for UNIQUE constraint (fresh store rejects dupe raw INSERT, UNIQUE in sqlite_master DDL, existing store gets UNIQUE after migration, marker set, UNIQUE enforced post-migration)
  • uv run pytest --timeout=30 -q — full suite: 1849 passed, 8 skipped

@robotrocketscience robotrocketscience added the author-Setr PR coordination mutex label Apr 29, 2026
Adds MemoryStore.insert_or_corroborate(b, *, source_type, ...) which
checks get_belief_by_content_hash first; on a hit it calls
record_corroboration and returns (existing_id, False); on a miss it
calls insert_belief and returns (b.id, True). Validates source_type
against CORROBORATION_SOURCE_TYPES before any DB access. No callers
migrated yet. Seven unit tests in tests/test_insert_or_corroborate.py
cover insert, duplicate hit, count increment, alpha/beta stability,
and bad-source-type ValueError.
All live insert_belief call sites (ingest.py, scanner.py,
classification.py, cli.py, triple_extractor.py, mcp_server.py) now
go through insert_or_corroborate. Cross-source duplicates
(same content_hash, different belief_id) now record a corroboration
instead of silently inflating a second belief row. Adds three new
CORROBORATION_SOURCE_TYPES values (filesystem_ingest, cli_remember,
consolidation_migration) to cover the new call sites. Updates
test_corroborations.py enum check and test_ingest_jsonl.py
source-label test to reflect the corrected dedup behaviour.
migrate.py and benchmark.py are intentionally excluded.
#219. Adds _maybe_consolidate_content_hash_duplicates(), a one-shot
migration that runs on every store open until the
SCHEMA_META_CONTENT_HASH_DEDUP_COMPLETE marker is set.

For each group of beliefs sharing a content_hash the migration picks
the oldest row as canonical (created_at ASC, id ASC tiebreak) and:
- sums alpha/beta across all group members (each carries Bayesian evidence)
- propagates the highest-precedence origin (user_* > agent_* > unknown)
- propagates lock_level='user' if any member holds it
- takes the latest last_retrieved_at across the group
- rewrites FK references in feedback_history, belief_corroborations, and
  edges (src and dst) from duplicate ids to the canonical id
- drops belief_entities and belief_versions rows for duplicates (same
  content → same entities; canonical rows already exist)
- inserts one synthetic belief_corroborations row per duplicate consumed
  (source_type='consolidation_migration') to preserve the count signal
- deletes duplicate rows from beliefs and beliefs_fts in one bulk DELETE

All writes are inside a single transaction; a crash leaves no partial
state. On a 20K-belief store with ~2K duplicate groups the pass
completes in under 2 seconds.
Adds UNIQUE constraint to beliefs.content_hash via two mechanisms:

1. Fresh stores: _SCHEMA now declares content_hash TEXT NOT NULL UNIQUE,
   so any DB created from scratch has the constraint in DDL.

2. Existing stores: _maybe_apply_content_hash_unique() does a table-swap
   migration (SQLite does not support ALTER TABLE ADD CONSTRAINT):
   - Reads the current column list via PRAGMA table_info so any extra
     ALTER TABLE columns (hibernation_score, activation_condition, etc.)
     are preserved correctly.
   - DROP TABLE IF EXISTS beliefs_new (clears partial state from a prior
     failed attempt).
   - CREATE TABLE beliefs_new with UNIQUE added to content_hash.
   - INSERT INTO beliefs_new SELECT ... FROM beliefs.
   - DROP TABLE beliefs + RENAME beliefs_new TO beliefs.
   - Recreates idx_beliefs_session and idx_beliefs_origin.
   - Stamps SCHEMA_META_CONTENT_HASH_UNIQUE_APPLIED on completion.

Precondition: _maybe_consolidate_content_hash_duplicates must have
already run (no duplicate rows). If that invariant is violated a
sqlite3.IntegrityError propagates — it is a programming error.

Also fixes test_mcp_server.py _put_belief helper that used a shared
content_hash="hh" for all inserted beliefs; now uses f"hh-{id}" to
avoid IntegrityError under the new constraint.
Design doc covering insert_or_corroborate, the two one-shot migrations
(_maybe_consolidate_content_hash_duplicates and
_maybe_apply_content_hash_unique), all migrated call sites, new
corroboration source types, and the invariants upheld by the fix.
@robotrocketscience

Copy link
Copy Markdown
Owner Author

[claim:review:Gylf:2026-04-29T02:14:44Z]

@robotrocketscience

Copy link
Copy Markdown
Owner Author

[release:review:Gylf:2026-04-29T02:17:21Z]

@robotrocketscience
robotrocketscience enabled auto-merge (squash) April 29, 2026 02:17
@robotrocketscience
robotrocketscience merged commit 367b2eb into main Apr 29, 2026
8 checks passed
@robotrocketscience
robotrocketscience deleted the fix/issue-219-content-hash-dedup branch April 29, 2026 02:17
robotrocketscience added a commit that referenced this pull request Apr 29, 2026
## Summary

Spec memo for #290. Phase-3 of the #286 redesign — the foundational
typing axis that ranking, aging, and promotion all consume. **Doc-only —
no code change. Posted for ratification.**

Core moves:

1. **New orthogonal axis `retention_class`** with three live values:
`fact`, `snapshot`, `transient` (plus `unknown` for migration). Distinct
from the existing `type` enum, which describes *form* not *expected
lifetime*.
2. **Defaults per ingest path.** Filesystem/git/CLI-remember → `fact`.
Transcript/hook → `snapshot`. `transient` requires explicit operator
opt-in. Table in §2.
3. **Soft down-weight in ranking, not hard expiry.** Composite score
from #289 gains a `retention_age_multiplier(class, age_days)` term.
Per-class exponential decay with floor (fact ~300d half-life, snapshot
14d, transient 1d). Locks force multiplier = 1.0.
4. **Promotion: snapshot → fact** when `corroboration_count >= 3 AND
distinct_sessions >= 2 AND no contradiction`. Reuses #190 corroboration
recorder. Doctor-pass opt-in (`aelf doctor --promote-retention`, mirrors
#253). No demotion — contradictions handle correctness.
5. **Migration via `_maybe_classify_retention_class()`**, schema-meta
gated, heuristic by `source_kind`. `legacy_unknown` rows stay `unknown`.

## Decision asks (nine)

Bottom of the memo. Schema shape, defaults table, transient opt-in,
multiplier shape, lock override, calibration plan, promotion thresholds,
no-demotion, migration heuristic.

## What ships if ratified

Three PRs:

1. Schema + score-formula change + migration helper (~500 lines)
2. Per-ingest defaults wired across scanner / ingest / hooks (~250
lines)
3. Promotion lane via `aelf doctor --promote-retention` (~300 lines)

## Test plan

- [x] Memo cross-references #286, #196, #229, #190, #253, #283, #281
- [x] Code touchpoints listed in Provenance
- [x] No code change; nothing to test
- [ ] Ratification or override comment from maintainer

Refs #290, #286
robotrocketscience added a commit that referenced this pull request Apr 29, 2026
## Summary

Single-doc index for the open-issue tree. Sorts everything in `gh issue
list --state open` into five waves with explicit deps, soft-deps, and
cross-cutting hazards. Goal: prevent the rework patterns that already
bit us (#293's tests dying on #283's UNIQUE constraint, #256 stuck on
author rebase after multiple PRs landed on `store.py`, etc.).

Five waves:

- **Wave 0** — close-out. Seven stale items that should walk to zero
before new work starts (#223, #254, #281 re-scope, #286 → tracker, #287
dup, #280, #288 implementation tail).
- **Wave 1** — rebuild redesign (#288, #289, #290, #291). All four spec
memos in flight; ratify all four before opening any implementation PR.
Implementations must sequence (`context_rebuilder.py` is a
merge-conflict spot).
- **Wave 2** — phantom-prereqs T1→T2→T3 (#191/#256 stuck on rebase; #192
blocks on T2).
- **Wave 3** — v2.0 substrate decision tree. #196 gates everything
posterior-related. Bench-gated items (#197/#198/#199/#201/#229) wait on
#288's harness producing precision/recall numbers.
- **Wave 4** — v2.x materialization (#262#264#265). Sequential,
must not parallelise.
- **Wave 5** — long-tail retrieval / research (#154, #153). Hold; #154
refactor would rework #289#291.

Plus five cross-cutting hazards on the wall: `context_rebuilder.py` and
`store.py` as merge-conflict spots; the #283 UNIQUE constraint test
pattern; calibration-data bench-gate; substrate ratification before #290
implementation.

## Decision asks

Bottom of the doc — operator stamping queue (5 items) that unblocks Wave
1 implementation in sequence.

## Test plan

- [x] All open issues from `gh issue list` accounted for
- [x] All in-flight PRs cross-referenced in the spec / impl index tables
- [x] Discretion grep clean
- [ ] Operator review: confirm wave assignments + hazards list
robotrocketscience added a commit that referenced this pull request Apr 29, 2026
…ows (#223) (#300)

## Summary

Closes #223. Pre-#283 re-ingest could leave `feedback_history` rows
pointing at a `belief_id` that was later deleted (a fresh duplicate took
its place with a new id). The UNIQUE `content_hash` constraint and
`insert_or_corroborate` plug the source going forward, but the residue
still exists in user databases (issue reporter saw 100% orphan rate).

What ships:

- `store.count_orphan_feedback_events()` /
`delete_orphan_feedback_events()` — direct SQL helpers.
- `doctor.gc_orphan_feedback(store, *, dry_run=True)` returning
`OrphanFeedbackReport`.
- `aelf doctor --gc-orphan-feedback` flag. Dry-run by default; `--apply`
deletes.

Recovery is not attempted. `feedback_history` stores only `belief_id`,
not `content_hash`, so the original target's content is unrecoverable.
The pass deletes rather than re-links — the only safe op.

Mirrors the `--classify-orphans` pattern from #206.

## Test plan

- [x] `uv run pytest tests/ -q` → 1865 passed, 8 skipped on 3.13.
- [x] `pytest tests/test_doctor_gc_orphan_feedback.py -q` → 11 passed
(store helpers, doctor function, formatter, CLI dry-run, CLI apply).
- [x] Discretion grep on full diff → clean.

Refs #223. Source mechanism plugged by #283 / #219.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author-Setr PR coordination mutex

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Re-ingest creates 5.3x row inflation; FTS5 retrieval injects near-duplicate beliefs

1 participant