fix(value_compare): keep non-finite numeric slots out of the comparator (#1227)

CHANGELOG/v4.md

@@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - **The FTS5 lane required every query token to be present, so it returned nothing for most real prompts ([#1177](https://github.com/robotrocketscience/aelfrice/issues/1177), [#1158](https://github.com/robotrocketscience/aelfrice/issues/1158)).** `_escape_fts5_query` quoted each whitespace token and joined them with spaces, which FTS5 reads as an implicit AND. No single belief contains every word of a natural-language question, so the lane went silent on exactly the query shape a `UserPromptSubmit` prompt has. Measured over 503 distinct logged prompts against a live 44,584-belief store, with the arms reported separately because pooling them is misleading in both directions: the conjunctive form returned **zero hits for 28.7% of user turns and 100% of harness blocks** (75.7% pooled). `search_beliefs`, `search_beliefs_scored` and the federated peer search now OR the three lowest-document-frequency tokens: **0.0% zero-hit on both arms**, holding **91.3%** of the conjunctive lane's top-20 hits on the queries where it returned anything at all. **The trim is what makes it affordable.** A full OR — what the issue's Mechanism section specifies — matches on the common terms too and costs **43.67 ms p50** on the user arm against the conjunctive lane's 1.01 ms, and it buys nothing: its top-20 agreement is 91.4% against the 3-token trim's 91.3%, and a 5-token trim also sits at 91.4%. Three tokens is the knee at **4.91 ms**, not a round number. Rarity is document frequency in the store's own corpus rather than a stopword list, which reads wrong and measures right — on a memory store about retrieval, `retrieval` carries a df of 2,443 and `how` only 516, so `how` survives the trim; the trim is candidate generation feeding bm25, which then ranks. **No migration and no new dependency:** document frequencies come from two per-connection TEMP virtual tables (an `fts5vocab` over `beliefs_fts`, and an empty probe table declared with the same tokenizer), so nothing is written to the database file and the path is safe on a read-only DB; an SQLite without `fts5vocab` degrades to a full OR rather than to an exception. Two implementation facts were established by measurement rather than assumed, and both would have been silent defects. Query tokens are resolved through FTS5's own tokenizer instead of `aelfrice.bm25.tokenize_stemmed`, because the two disagree on underscores and diacritics and the Python path resolves only **62.9%** of tokens against 99.7% — unresolved tokens collapse to df 0 and masquerade as the rarest, which is the failure #1158 records for the IDF-clip lane. And the expression carries the *original* tokens rather than the stems it ranked by, because the porter stemmer is not idempotent: **416 of that store's 15,208 terms** stem again to something else (`abus` → `abu`), and re-emitting the stem would silently stop matching those documents. `_escape_fts5_query` is retained for callers that do want every token present. This is the recall half of #1177 only — the FTS5-for-numpy substrate swap is **not** approved and stays parked.
 
+- **A git SHA in a belief could crash contradiction detection ([#1227](https://github.com/robotrocketscience/aelfrice/issues/1227)).** `float()` does not raise on overflow, it saturates to infinity, and the exponent branch of the numeric-slot pattern matches abbreviated commit SHAs — `592e701` is a hex string that happens to hold one `e` between digits, and parsing it as scientific notation yields `inf`. `_format_number` then narrowed with a bare `int(x)`, which raises `OverflowError` on infinity and `ValueError` on NaN. Found on a live 44,584-belief store, where three beliefs carried such a literal and the comparator died partway through a scan; reachable through `aelf search` under `AELF_SHOW_CONFLICTS=1`, which is the only wiring `_slot_conflict_preextracted` currently has and defaults off — so it was latent rather than live. Fixed in two places on purpose. The extractor no longer admits a non-finite value as a slot at all: an overflowed SHA is a parse artifact, not a measurement, and comparing against it manufactures a disagreement with a number no belief asserts. The formatter guards its own narrowing regardless, so any caller reaching the comparator by another route cannot resurrect the crash. Each half is pinned by tests that fail when only the other is present, since either fix alone leaves the other path live, and the extractor test carries a control asserting an ordinary numeric in the same sentence is still extracted — without it, a fix that dropped every numeric slot would pass. This also matters ahead of [#1175](https://github.com/robotrocketscience/aelfrice/issues/1175), whose build-first item proposes moving this comparator onto the retrieval injection path: there the hook's never-raise contract would have swallowed the exception and degraded retrieval silently rather than failing loudly. **Not covered:** the extractor still matches a bare `\d+e\d+` token as a numeric slot when it does not overflow, so a short SHA can still become a finite false slot; narrowing that pattern is a behaviour change with a wider blast radius and is left to its own decision.
 - **ARCHITECTURE and PHILOSOPHY asserted posterior decay that has no production caller ([#1218](https://github.com/robotrocketscience/aelfrice/issues/1218), routed from [#1162](https://github.com/robotrocketscience/aelfrice/issues/1162)).** `scoring.decay`, `type_half_life` and `TYPE_HALF_LIFE_SECONDS` are imported by no module under `src/` — retrieval takes `posterior_mean`, `partial_bayesian_score` and the gamma/zeta scorers and nothing else — so **nothing ever moves a stored `(alpha, beta)` toward the Jeffreys prior**. Three statements described it as shipped, and the PHILOSOPHY lock story rested on the lock short-circuit *inside* that function: an exemption from a mechanism that does not run. The fix is not deleting the word decay but saying which decay ships, because the conflation is what made the claims read as plausible. **The filed issue's own replacement text was too generous and is corrected here:** it describes retrieval-time `_apply_temporal_decay` as live, but that path is reachable only through `retrieve_v2` behind `temporal_sort`, which defaults to False and is set by nothing in `src/`, while the production hooks call `retrieve()`. So it is not on the default path either. What *is* live and default-on is entity-persistence demotion ([#1096](https://github.com/robotrocketscience/aelfrice/issues/1096)), which acts on ranking position in the L1 rerank — verified reachable from `retrieve()`, which passes the lane resolver-driven rather than hard-off. A fourth statement not in the issue ("beliefs are still mutated for decay and feedback") carried the same error and is corrected alongside. The cross-link asked for by the third acceptance criterion is enforced rather than written down: a test asserts the code fact the prose now rests on, and fails in **both** directions of #1162's pending disposition — wiring `scoring.decay` trips the no-caller assertion, deleting it trips a negative control — with the failure message naming the two files to update. It parses `src/` rather than grepping it, since `decay` appears as a local variable for the *ranking* factor in `_apply_temporal_decay`, which is precisely the conflation being undone. It deliberately does not assert the docs' wording: a text match on prose breaks on rephrasing and says nothing about whether the claim is true, and making the suite read `docs/` would have required adding `docs/**` to CI's `code` path filter — taxing every docs-only PR with the full pytest matrix, which `test_ci_path_filter` correctly flagged.
 - **Re-asserting a statement you had retired was swallowed, and `aelf lock` reported success anyway ([#1215](https://github.com/robotrocketscience/aelfrice/issues/1215)).** [#1210](https://github.com/robotrocketscience/aelfrice/issues/1210) gave `get_belief` a `valid_to` filter; `get_belief_by_content_hash` still had none, and it is the lookup every ingest path resolves through. So a re-assertion of retired content matched the **tombstone**: `insert_or_corroborate` wrote a corroboration row against the retired belief and returned `was_inserted=False`, the INSERT never ran, and nothing became visible. Reproduced end to end — after `aelf retire`, a second `aelf lock` of the same sentence printed `upgraded existing belief to lock` while the row stayed at `valid_to` set, `aelf locked` listed nothing, and search found nothing; the store held exactly one row, invisible. That is the residual case [#1164](https://github.com/robotrocketscience/aelfrice/issues/1164) did not cover: its fix correctly moved the lock upgrade onto the *resolved* id, which is why `lock_level` did land — on a row nothing can read. The lookup now excludes retired rows by default. Two callers opt back in, both because they are **UNIQUE-constraint guards rather than reads**: `content_hash` is `NOT NULL UNIQUE` ([#219](https://github.com/robotrocketscience/aelfrice/issues/219)), so a tombstone still owns its hash and an insert that cannot see it trips the constraint — verified, not assumed: reverting the opt-in in `wonder_ingest` raises `sqlite3.IntegrityError: UNIQUE constraint failed: beliefs.content_hash` when the next wonder pass revisits a GC'd phantom. That constraint is also why "insert a fresh row alongside the tombstone" was never on the table, and the fix is a policy decision rather than a filter. **Ratified policy, tiered by who is asserting:** an explicit user assertion (`aelf lock`, `aelf remember`, and their MCP twins) **revives** the belief — `valid_to` cleared, FTS row restored, back in search — at the posterior it was retired at, with a `reassert:revive` audit row so the transition is not silent in either direction. Background capture (transcript, commit, filesystem, wonder, claude-memory mirror, migration) leaves the tombstone retired and records nothing, because an agent re-observing text it already scanned must not undo the user's curation — and under the old behaviour it was doing exactly that, accruing corroboration rows on a belief the user had removed. Revival deliberately does **not** move the posterior: the re-assertion is recorded as a `belief_corroborations` row, which is where that signal belongs. Tests assert the invariant per tier, each with a negative control on live content — without it, a bug that made capture a no-op outright would satisfy both "did not revive" and "wrote nothing".
 - **Nothing demoted a potentially-stale belief, because the pass that does it had no importer ([#1207](https://github.com/robotrocketscience/aelfrice/issues/1207), under [#1162](https://github.com/robotrocketscience/aelfrice/issues/1162)).** `BFS_EDGE_WEIGHTS` pins `POTENTIALLY_STALE` at 0.0 with a comment saying demotion happens in the rerank pass instead — and `git grep edge_rerank -- src/` returned no importer, while its producer (`aelf doctor --detect-stale`, [#387](https://github.com/robotrocketscience/aelfrice/issues/387)) was live and writing those edges. So BFS declined to demote on the grounds that a downstream pass would, and that pass never ran. [#1208](https://github.com/robotrocketscience/aelfrice/issues/1208) made the module correct; it did not make it reachable. `expand_bfs` now calls it on the way out. **Wired unconditionally rather than behind a lane flag** per the operator decision: the producer is already the opt-in, `--detect-stale` defaults off, and a store holding no marker edges makes the pass a mathematical identity — the firing set is empty and the re-sort uses the same `(-score, belief.id)` key `expand_bfs` had already applied. A second flag would double-gate it, nothing would fire for anyone, and the finding would simply be re-filed later. The 0.0 pin stays and did not need to change: it governs traversal *through* a marker edge, whereas the rerank keys off marker edges *incoming to* a surfaced belief — disjoint sets, pinned by a test that a belief reachable only via a marker stays unreachable. **A second instance of the same rot was found while wiring it:** the pass read `edges_to`, which is local, and it predates federation ([#690](https://github.com/robotrocketscience/aelfrice/issues/690)) — so a hop the walk stepped into a peer for would have its marker edges looked up in the wrong database and a stale belief in a peer store would be silently exempt. It reads `edges_to_in_scope` against the hop's own scope now, which delegates to `edges_to` when the scope is None, so the non-federated path is byte-identical. That is the same failure mode as the `ScoredHop` fields #1208 restored, found the same way: unexercised code does not stay correct, it ages against whatever changed around it. Tests assert through `expand_bfs` rather than the helper, since a helper-level test is exactly what this module already had while being inert — and the reorder test gives the stale belief the *stronger* path first, because with both rivals on equal paths the id tie-break alone produces the expected order and the test would pass against an unwired walk. Wiring the pass also broke the bench gate that *grades* it — both arms of `test_edge_rerank_potentially_stale.py` call `expand_bfs`, so the control arm silently acquired the treatment and the treatment arm applied the penalty twice, collapsing the rate difference the gate measures while grading a 0.25x penalty no production path produces. It stayed invisible because the gate is `bench_gated` and skips whenever `AELFRICE_CORPUS_ROOT` is unset, which is every CI run — the same shape as the finding itself, one layer up. `expand_bfs` grew a `rerank` keyword defaulting to on purely as that control seam, with a test that **parses** `src/` rather than grepping it to assert no production caller disables it, since the substring occurs in `expand_bfs`'s own docstring. Finally, the pass short-circuits on a `LIMIT 1` existence probe when no keyed marker edge exists and every hop is local, rather than issuing one `edges_to_in_scope` per hop to rediscover an empty table; the local-only condition is load-bearing, because `has_edge_type` reads the local DB and probing it for a federated hop would reintroduce the scope bug above on exactly the store where the probe looks most justified.

src/aelfrice/value_compare.py

@@ -38,6 +38,7 @@
 """
 from __future__ import annotations
 
+import math
 import re
 from dataclasses import dataclass
 from typing import Final
@@ -212,6 +213,17 @@ def _extract_numerics(text: str) -> tuple[NumericSlot, ...]:
             value = float(m.group("value"))
         except ValueError:
             continue
+        # `float()` does not raise on overflow — it saturates to
+        # +/-inf. The exponent branch of `_NUMERIC_RE` matches
+        # abbreviated git SHAs like `592e701`, which are hex strings
+        # that happen to hold one `e` between digits; parsed as
+        # scientific notation they become `inf`. That is not a
+        # measurement, so admitting it as a slot manufactures a
+        # comparison against a value no belief actually asserts.
+        # Dropping it here also keeps every downstream consumer off
+        # the non-finite path (#1227).
+        if not math.isfinite(value):
+            continue
         pair = (key, value)
         if pair in seen:
             continue
@@ -337,6 +349,13 @@ def _numeric_close(a: float, b: float, rel_tol: float) -> bool:
 
 
 def _format_number(x: float) -> str:
+    # `int()` raises on non-finite input — OverflowError for +/-inf,
+    # ValueError for nan — so the narrowing below cannot be reached
+    # unguarded. `_extract_numerics` already refuses to admit such a
+    # value as a slot (#1227); this is the second line of defence, for
+    # any caller that reaches the comparator by another route.
+    if not math.isfinite(x):
+        return f"{x:g}"
     if x == int(x):
         return str(int(x))
     return f"{x:g}"

tests/test_value_compare_nonfinite_1227.py

@@ -0,0 +1,94 @@
+"""Non-finite numeric slots must never reach the comparator (#1227).
+
+`float()` does not raise on overflow, it saturates to `inf`. The exponent
+branch of `_NUMERIC_RE` matches abbreviated git SHAs — `592e701` is a hex
+string that happens to hold one `e` between digits — so parsing it as
+scientific notation yields `inf`. `_format_number` then narrowed with a bare
+`int(x)` and raised `OverflowError`.
+
+Both halves are pinned here: the extractor must not admit the slot, and the
+formatter must survive a non-finite input regardless of how it got one. Either
+fix alone leaves the other path live, so both are asserted.
+
+The literals below are real values found on a production store, where the
+crash surfaced through `aelf search` under `AELF_SHOW_CONFLICTS=1`.
+"""
+from __future__ import annotations
+
+import math
+
+import pytest
+
+from aelfrice.contradiction import _slot_conflict_preextracted, extract_values
+from aelfrice.models import BELIEF_FACTUAL, LOCK_NONE, LOCK_USER, Belief
+from aelfrice.value_compare import _format_number, find_conflicts
+
+# Overflow to +/-inf when read as scientific notation. The first two are
+# abbreviated commit SHAs.
+OVERFLOWING = ("592e701", "1e124732", "4e999", "-3e400")
+
+
+def _mk(bid: str, content: str, *, locked: bool = False) -> Belief:
+    return Belief(
+        id=bid,
+        content=content,
+        content_hash=f"h_{bid}",
+        alpha=1.0,
+        beta=1.0,
+        type=BELIEF_FACTUAL,
+        lock_level=LOCK_USER if locked else LOCK_NONE,
+        locked_at="2026-07-30T00:00:00Z" if locked else None,
+        created_at="2026-07-30T00:00:00Z",
+        last_retrieved_at=None,
+    )
+
+
+@pytest.mark.parametrize("literal", OVERFLOWING)
+def test_the_literal_really_does_overflow(literal: str) -> None:
+    """Guard the premise. If `float()` ever stopped saturating, every other
+    test in this file would pass vacuously against a finite value."""
+    assert not math.isfinite(float(literal))
+
+
+@pytest.mark.parametrize("literal", OVERFLOWING)
+def test_extractor_drops_the_non_finite_slot(literal: str) -> None:
+    """An overflowing literal is a parse artifact, not a measurement, so it
+    must not become a comparable slot at all."""
+    slots = extract_values(f"commit {literal} landed the fix")
+    assert all(math.isfinite(s.value) for s in slots.numeric)
+
+
+def test_a_finite_neighbour_is_still_extracted() -> None:
+    """Control. Dropping non-finite values must not take ordinary numerics
+    with it — otherwise the fix could pass by extracting nothing."""
+    slots = extract_values("commit 592e701 bumped timeout to 30")
+    assert any(s.value == 30.0 for s in slots.numeric)
+
+
+@pytest.mark.parametrize("value", [math.inf, -math.inf, math.nan])
+def test_format_number_survives_a_non_finite_input(value: float) -> None:
+    """Defence in depth: even handed a non-finite value directly, the
+    formatter must not raise. `int(inf)` raises OverflowError and `int(nan)`
+    raises ValueError, so a bare `int()` narrowing fails both."""
+    assert _format_number(value) in {"inf", "-inf", "nan"}
+
+
+def test_find_conflicts_does_not_raise_on_a_sha_bearing_belief() -> None:
+    """The end-to-end path that actually broke: a belief carrying a
+    SHA-shaped literal, compared against a lock holding a numeric slot."""
+    lock = _mk("lock1", "timeout is 30 seconds", locked=True)
+    sha = _mk("b1", "reverted in 51ebe7a commit 1e124732 timeout is 45")
+    lock_slots = extract_values(lock.content)
+    # Direct comparator call — this raised OverflowError before #1227.
+    find_conflicts(extract_values(sha.content), lock_slots)
+    # And through the wiring `aelf search` uses.
+    _slot_conflict_preextracted(sha, [(lock, lock_slots)])
+
+
+def test_a_real_conflict_is_still_detected_alongside_a_sha() -> None:
+    """Control for the above: the SHA must not suppress detection of the
+    genuine numeric disagreement sharing the same belief."""
+    lock = _mk("lock1", "timeout is 30", locked=True)
+    lock_slots = extract_values(lock.content)
+    sha = _mk("b1", "commit 1e124732 changed timeout to 45")
+    assert _slot_conflict_preextracted(sha, [(lock, lock_slots)]) == "lock1"