fix: two ABSORB bugs found by adversarial cross-review

[jagoff]

An ultracode workflow (9 agents: 3 independent reviewers × 3 interaction
surfaces, each finding adversarially re-verified) reviewed the three
near-duplicate-handling changes landing this session — ABSORB default-on
(#262, merged), chunk-parent rollup (#260), average-link consolidation
(#261) — for interaction risks. It found two real, independently-verified
bugs in ABSORB, which is already live on master.

Bug 1 — silent race with nightly consolidation (HIGH)

_absorb_into_existing's LLM merge call (up to MEMO_CONSOLIDATE_TIMEOUT,
180s default) runs unlocked. If the nightly consolidation pass
archives+deletes the same target record while the call is in flight,
update()'s resolve_id() cleanly returns None — no exception — and the
only log line in that function was if updated is not None: _log.info(...),
with no else branch. The caller falls through to creating a brand-new
near-duplicate record from the raw content, undoing the consolidation that
just ran, with zero operator-visible signal.

Fix: log a _log.warning naming the likely cause when update() returns
None, so the outcome is observable instead of a silent data-integrity
mystery. Doesn't (and can't cheaply) prevent the race — the LLM call can't
run under the write lock without blocking every other write for up to
180s — but makes it diagnosable.

Bug 2 — no type-match check (MEDIUM)

The save-time dedup/absorb candidate search passes no type_ filter, and
nothing in the trigger loop compared the candidate's type to the incoming
save's type. A note scoring >=0.88 against an existing fact/decision
would still trigger absorb, rewriting the existing record's body via LLM
merge while keeping its original type label — silently blending cross-type
content before average-link consolidation's type-partitioned clustering
(_cluster_within_scope) ever gets a chance to see it.

Fix: absorb now requires _dh.get("type") == type_; a type mismatch falls
through to the ordinary warn-and-create path.

Also

Fixed a stale # C3: absorb-on-recurrence (flag-gated, default off).
comment at the call site — the flag flipped to default-on in #262 and
nothing else touches this line, so it stayed wrong.

Verification

Two new regression tests reproduce both bugs directly, not just assert the
fix's mechanism:

• test_absorb_skips_cross_type_near_duplicate — same content, different
  types, asserts no absorb and the target's body/type stay untouched.
• test_absorb_warns_when_target_vanishes_mid_flight — the mocked LLM chat
  call deletes the absorb target as a side effect, genuinely
  reproducing the race window (not simulated via a mocked return value),
  then asserts the fallback happened AND the warning was logged.

tests/test_save_absorb.py: 7 passed (was 5; +2 new)
targeted suite (save|absorb|dedup|duplicate|synthesize|trust_states|consolidate): 443 passed, 1 skipped
ruff format --check / ruff check / mypy: clean
scripts/quality_gate.py: passed (178 complexity budgets, 190 exception budgets)
memo eval recall --gate (pre-push profile): PASS, prec@5 0.610 / noise@5 0.000

Not fixed here — real but architectural, needs its own scoped pass

The same review surfaced three more findings, all independently verified,
none acted on here to avoid bolting unrelated architecture changes onto a
bugfix:

• Chunk resync after absorb has no auto-heal. maybe_emit_chunks()
  re-checks MEMO_CHUNK_INGEST live (ignoring whether stale chunk rows
  already exist) and wraps the re-chunk step in a blanket
  except Exception that only warns — the code's own "next reindex heals
  them" comment has no automated trigger, since nightly maintain never
  calls Memory.reindex(). Pre-existing behavior for any update(), not
  unique to absorb, but absorb makes it reachable more often.
• Three uncoordinated near-duplicate thresholds: MEMO_SAVE_DEDUP_THRESHOLD
  (0.88, flag-configurable, gates absorb) vs. consolidate()'s hardcoded
  0.85 default (what average-link's PR fix: replace greedy consolidation clustering with average-link (UPGMA) #261 measured its purity/chaining
  stats against) vs. the actual unattended nightly pass
  (_run_consolidate_dups, auto_apply=True) hardcoded to 0.9 with no flag
  at all. Tuning the save-time threshold has zero effect on nightly merge
  aggressiveness.
• Average-link's measurement predates ABSORB's corpus-shrinking effect.
  PR fix: replace greedy consolidation clustering with average-link (UPGMA) #261's stats were measured against a backlog accumulated under the old
  (pre-ABSORB) save behavior; going forward, most "obvious duplicate" pairs
  strong enough to reach nightly clustering (threshold 0.9) will already
  have been intercepted at save time (absorb threshold 0.88 < 0.9), so the
  population average-link actually processes will structurally differ from
  what it was tuned on. Never re-measured.

🤖 Generated with Claude Code