fix(store): retype legacy belief_corroborations.belief_id INTEGER -> TEXT (#762)

[robotrocketscience]

Closes #762.

Problem

aelf lock <statement> fails with sqlite3.IntegrityError: FOREIGN KEY constraint failed on DBs created against the released v1.5.0 package whenever the statement is Jaccard-close enough to an existing belief to route through Store.insert_or_corroborate → record_corroboration.

Some legacy DBs shipped with belief_corroborations.belief_id INTEGER even though beliefs.id is TEXT PRIMARY KEY. Because every DDL in _SCHEMA uses CREATE TABLE IF NOT EXISTS, the canonical corrected shape never reached pre-existing stores — the FK silently misses on every hex-id insert.

Silent until the dedup path triggers, so users likely worked around by adding noise text rather than reporting.

Fix

New MemoryStore._maybe_retype_belief_corroborations_belief_id() one-shot migration:

1. Gate on PRAGMA table_info(belief_corroborations) — only run when belief_id reports type INTEGER (uppercased). Anything else stamps the marker and exits.
2. PRAGMA foreign_keys=OFF (outside any transaction, per SQLite altertable guidance).
3. Standard column-retype recipe in an explicit BEGIN…COMMIT: CREATE belief_corroborations_new with the canonical TEXT FK → INSERT … SELECT to copy rows → DROP belief_corroborations → ALTER … RENAME → recreate the index.
4. Re-enable foreign keys.
5. Stamp SCHEMA_META_CORROBORATIONS_BELIEF_ID_RETYPED.

Wired into __init__ before _maybe_consolidate_content_hash_duplicates so the latter's synthetic consolidation_migration rows write against the fixed shape.

Hex belief ids round-trip cleanly through the straight INSERT … SELECT: digit-only literals would have been coerced under INTEGER affinity, but hex strings are not valid INTEGER literals, so SQLite preserved them as TEXT in storage.

Verification

• 5 new tests in tests/test_belief_corroborations_retype_migration.py:
  • test_legacy_integer_schema_is_retyped_on_open — pre-condition is INTEGER, post-condition is TEXT + marker.
  • test_retype_is_idempotent_across_opens — re-open does not rewrite the marker.
  • test_fresh_store_stamps_marker_without_swap — fresh store gets canonical schema + marker.
  • test_record_corroboration_succeeds_after_retype — the fix(store): aelf lock fails FK on Jaccard-match — belief_corroborations.belief_id INTEGER vs beliefs.id TEXT (no migration) #762 repro: hex-id record_corroboration no longer raises.
  • test_existing_corroboration_rows_carry_over — pre-existing rows survive the swap with TEXT belief_id intact.
• Broader migration suite green: 52/52 across test_corroborations.py, test_content_hash_consolidation.py, test_insert_or_corroborate.py, test_speculative_hash_v2_migration.py, test_legacy_migration.py, test_v1_to_v1x_migration.py.
• Store suite green: 32/32 across test_store_crud.py, test_setup_migrate_order.py, test_cli_confirm.py.

Notes

• The migration is positioned to run before any other pass that writes to belief_corroborations, so synthetic-row writes (consolidation_migration audit rows) land on the fixed schema.
• The legacy-INTEGER seed used in the new tests inserts a row with PRAGMA foreign_keys=OFF; that mirrors the affinity-coercion path that lets some pre-existing rows survive on broken stores in the wild.
• aelf:confirm writes to feedback_history, not belief_corroborations (per test_cli_confirm.py:test_confirm_does_not_write_belief_corroborations), so the fix(store): aelf lock fails FK on Jaccard-match — belief_corroborations.belief_id INTEGER vs beliefs.id TEXT (no migration) #762 report's "presumably broken on old DBs for the same reason" speculation does not apply to confirm. Only the dedup / corroboration ingest path was affected.

[sourcery-ai]

Sorry @robotrocketscience, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[coderabbitai]

Warning

Rate limit exceeded

@robotrocketscience has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 26 minutes and 56 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d874195d-3431-4a56-8ef7-61185312fd56

📥 Commits

Reviewing files that changed from the base of the PR and between afe22a8 and 3ffad99.

⛔ Files ignored due to path filters (1)

• CHANGELOG.md is excluded by !**/CHANGELOG.md

📒 Files selected for processing (2)

• src/aelfrice/store.py
• tests/test_belief_corroborations_retype_migration.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/issue-762-belief-corroborations-migration

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

PR-size soft cap

This PR is over the advisory size threshold:

• 396 changed lines (limit: 200)
• 3 changed files (limit: 3)

Bigger PRs collide with more open work, which under the parallel-session workflow tends to produce repeated attn:merge-conflict cycles (see #602). When practical, split into smaller PRs that each touch a focused surface.

This is advisory only — nothing is blocked. If the size is intentional (large refactor, module removal, generated code), apply the size:override label and this comment will be removed on the next push.

[robotrocketscience]

[claim:review:pascal:2026-05-13T23:22:11Z]

[robotrocketscience]

LGTM. (Cannot formally --approve — shared GH author with PR opener; merge-train bot gates on signatures + green checks + ready-to-merge label, not GH review state.)

Reviewed:

• Migration logic in _maybe_retype_belief_corroborations_belief_id follows the SQLite-recommended column-retype recipe (CREATE _new → INSERT SELECT → DROP → RENAME → recreate index), wrapped in PRAGMA foreign_keys=OFF/ON outside the explicit BEGIN/COMMIT.
• New table shape (lines 1131-1141) matches the canonical _SCHEMA definition at store.py:257-265 exactly: belief_id TEXT NOT NULL REFERENCES beliefs(id) ON DELETE CASCADE, ingested_at TEXT NOT NULL (no default, consistent with canonical).
• Ladder order is correct: runs before _maybe_consolidate_content_hash_duplicates, so the synthetic consolidation_migration corroboration rows write against the fixed shape (body's explicit ordering rationale).
• Idempotency via SCHEMA_META_CORROBORATIONS_BELIEF_ID_RETYPED. Fresh stores and non-INTEGER shapes both stamp the marker without doing the swap.
• The "missing table" branch stamps the marker after _SCHEMA's CREATE IF NOT EXISTS has already created the canonical shape on this open — correct.
• Tests cover five distinct paths: legacy retype, idempotency across re-opens, fresh-store no-op, the record_corroboration repro from the issue, pre-existing row carry-over. Hex-affinity claim about pre-existing rows surviving the copy as TEXT is correct (SQLite NUMERIC affinity coerces only well-formed integer literals; 4fadaddd9b67b614 contains a-f so survives).
• "Confirm not affected" claim verifies — test_cli_confirm.py:172 asserts aelf:confirm writes to feedback_history, not belief_corroborations.

CI: pytest 3.12/3.13, CodeQL, all Staging Gate jobs green. mergeable=MERGEABLE. Discretion grep clean. Three atomic signed commits with conventional prefixes (fix / test / docs).

Two non-blocking notes for posterity:

• The FK=OFF toggle is defense-in-depth — no other table references belief_corroborations, so DROP would succeed under FKs ON anyway. Fine to keep.
• In the cosmically improbable case of a 16-char hex ID that happens to be all decimal digits (~1 in 3e12 for secrets.token_hex(8)), legacy INTEGER affinity would have coerced it on insert and lost leading zeros. Pre-existing rows in that corner case would already be corrupted on disk; the migration's straight copy preserves whatever's stored, which is the only sensible behavior.

Applying ready-to-merge.

[robotrocketscience]

[release:review:pascal:2026-05-13T23:24:46Z]

[github-actions]

merge-train: merged 3ffad99 → main via FF push.