Capture-Reliability Anti-Patterns

TL;DR: Twelve concrete failure modes observed in hippo’s capture layer, each with a forbidden behavior, the reason it is forbidden, and the required alternative. Any of these patterns in a PR is a review blocker.

For the system reference these rules guard, see architecture.md. For per-source detail, see sources.md. Historical design records are archived under docs/archive/capture-reliability-overhaul/.

AP-1: Blocking the Shell Hook on Health Writes

Forbidden. Writing to source_health or any SQLite table synchronously inside shell/claude-session-hook.sh, or inside send_event_fire_and_forget in crates/hippo-daemon/src/commands.rs.

Why. The shell hook runs in the user’s interactive prompt critical path. The &! disown pattern exists to keep the hook from adding perceptible latency. send_event_fire_and_forget documents its durability contract at commands.rs:96–103: success means the frame hit the socket, not that SQLite was touched. Health writes belong in flush_events (background Tokio timer).

The right way. source_health is updated inside flush_events (daemon.rs) after the batch write succeeds or fails. Shell hook writes nothing to disk; sends one fire-and-forget frame and returns.

AP-2: Coupling Capture Health to Enrichment Health

Forbidden. Setting source_health.status = 'degraded' because the inference server is unreachable, the enrichment_queue is backed up, or hippo-brain is offline.

Why. Orthogonal failure domains. Capture has operated correctly while enrichment was offline for days. Coupling them means a routine inference-server model swap triggers a false capture alarm. hippo doctor already separates brain health from daemon health; source_health must maintain the same boundary. See architecture.md invariant I-10.

The right way. source_health tracks only capture-layer observables: last_event_ts, consecutive_failures, events_last_1h/24h. Enrichment health lives in the brain /health endpoint. Watchdog fires alarms only on capture metrics; never inspects enrichment_queue depth.

AP-3: Alerting on Absolute Silence Without Upstream Context

Forbidden. Firing capture_alarms or macOS notification whenever source_health.last_event_ts is older than N minutes, unconditionally.

Why. Shell silence overnight is normal. Browser silence is normal when Firefox is closed. Unconditional alarms train users to dismiss them — and they’ll dismiss a real outage for the same reason. Alerting must distinguish “user active, source silent” from “user asleep, source correctly quiet.”

The right way. Watchdog and doctor gate silence alarms on corroborating signals drawn from data hippo already captures, not new OS-level process/idle-time checks: source process confirmed running (pgrep -x firefox for browser), backing session files changed recently (Codex/Cursor idle probes; opencode uses its session.time_updated DB column instead, since the DB file’s own mtime is always recent regardless of session activity), and for shell, the recency of a real (probe_tag IS NULL) shell event (shell_health::shell_real_activity_recent). The synthetic probe alone cannot serve as that signal, since it lands on its own 5-minute cadence regardless of user presence. Thresholds per-source, configurable, conservative defaults.

AP-4: Adding Notification Types Outside the Single Alarm Channel

Forbidden. Calling osascript -e 'display notification' from anywhere other than the watchdog’s notify_macos path. Adding a new capture_alarms.alarm_type without updating the rate-limiter.

Why. Hippo already has multiple notification paths (doctor, version mismatch, brain error). Fragmented paths cannot be rate-limited uniformly. Users will disable all hippo notifications after three spurious alerts during bring-up.

The right way. One notification channel: watchdog reads capture_alarms, applies rate limit (1 macOS notification per invariant per hour via last_notified_at), calls osascript once per qualifying alarm. notify_macos defaults to false. All other code paths write to capture_alarms and let the watchdog decide.

AP-5: Reinventing Metrics Storage Alongside OTel

Forbidden. Adding in-process HashMap<String, u64> or AtomicU64 metrics store for per-source event counts, then exposing via a new HTTP endpoint or a new SQLite table distinct from source_health.

Why. crates/hippo-daemon/src/metrics.rs already defines OTel counters (EVENTS_INGESTED, FLUSH_EVENTS, FALLBACK_WRITES). A parallel store diverges from the OTel pipeline, creates two sources of truth, and is invisible to Grafana/Prometheus. This mistake happened once already — MetricsCollector in the brain was a parallel store noted in CLAUDE.md “for future OTel export” that was never wired.

The right way. Add source KeyValue attribute to existing OTel counters (P0.4). Aggregates for source_health rolling counts computed from SQLite event tables by the recompute job, not in-process. Do not create new HTTP metrics endpoints; Prometheus scrape already exists behind otel feature flag.

AP-6: Letting Probes Appear in User-Facing Queries

Forbidden. Inserting synthetic probe events into events, claude_sessions, or browser_events without filtering them from every user-facing query: hippo ask, hippo events, hippo sessions, MCP search_events, search_knowledge, get_entities.

Why. A probe in RAG retrieval produces nonsense answers. A probe in hippo events output makes the user think they ran a command they didn’t. Probe contamination is hard to detect after the fact — probe rows look structurally identical to real rows.

The right way. Every probe row has probe_tag TEXT NOT NULL set to a non-null sentinel. Every query touching event tables adds AND probe_tag IS NULL (or NOT LIKE 'synthetic%'). P2.2 ships a Rust integration test asserting this filter in all query paths. Upstream filter is load-bearing: daemon never enqueues probe events — downstream filters are belt-and-braces.

AP-7: Making hippo doctor Slower Than 2 Seconds

Forbidden. Adding a doctor check that performs HTTP without a timeout, does a full table scan on events (millions of rows), spawns an unbounded subprocess, or blocks on brain returning healthy.

Why. hippo doctor is run interactively, especially during incidents. 10-second doctor is useless when something is on fire. Current inference-server check already enforces 1-second timeout; that constraint extends to every new check.

The right way. All new SQL uses indexed columns only. HTTP checks carry 1-second timeout. Any check that can’t complete in 500 ms on a loaded machine moves to hippo doctor --full. Total wall-clock asserted under 2 s in CI.

AP-8: Leaving the Watchdog Itself Unmonitored

Forbidden. Shipping the watchdog (P1.1) without a mechanism to detect that the watchdog process has stopped.

Why. Watchdog is launchd-managed. LaunchAgents fail to load silently on plist errors or post-update binary moves. A dead watchdog means every downstream invariant stops firing while the user believes they’re being monitored — strictly worse than no watchdog.

The right way. Watchdog writes heartbeat timestamp to source_health WHERE source='watchdog' on every cycle. Doctor check 8 (P0.3 stub, completed in P1.3) reads it and emits [!!] if older than 2 * poll_interval. Watchdog also sets HIPPO_WATCHDOG_VERSION env so hippo doctor --versions confirms binary alignment.

AP-9: Deleting the Fallback JSONL Path

Forbidden. Removing storage::write_fallback_jsonl, removing fallback branches in flush_events, or omitting fallback from any new ingestion path — even in P3 cleanup.

Why. Fallback JSONL is the last-resort durability backstop: catches events when SQLite is locked, DB is corrupt, or daemon mid-restart. Contract documented at commands.rs:96–103. At least one real event-loss incident was fully recovered from fallback files. No substitute.

The right way. Fallback path stays permanently. What improves is observability: P0.3 alarms on fallback > 24 h old (not replayed); P1.3 alarms on fallback count exceeding threshold. Accumulating fallback files are a symptom; response is to diagnose root cause, not remove the backstop.

AP-10: Patching tmux Window Targeting (historical, retired 2026-04-25)

Status: retired. T-5 shipped the FS watcher; T-8 deleted the tmux-spawn path entirely from shell/claude-session-hook.sh and crates/hippo-daemon/src/main.rs. There is no tmux code in hippo to patch. Kept here as a historical record of why the watcher work was prioritized.

Original rule (no longer applicable). During P0–P2, PRs modifying tmux window creation, tmux session targeting, or the TMUX_TARGET_SESSION path were rejected. Tmux-targeting code had been patched at least four times for variations of the same bug class (index conflicts, session-name collisions, non-default base-index, $TMUX_PANE unset). The structural fix was the long-lived FS watcher (P2.1), which eliminated the tmux dependency.

If a session-ingestion regression occurs now, triage against the watcher (watch_claude_sessions.rs). Manual recovery remains hippo ingest claude-session <path>.

AP-11: Silently Swallowing Capture Errors

Forbidden. Using .filter_map(|r| r.ok()), .ok().unwrap_or_default(), or .ok() on a Result inside any capture write path without logging the error and bumping a failure counter.

Why. Silent swallowing turns capture bugs invisible. flush_events in daemon.rs already demonstrates the correct pattern: every Err(e) branch calls warn!, writes to fallback, and calls state.drop_count.fetch_add. Failure is counted, logged, recoverable. Iterator patterns like .filter_map(|r| r.ok()) produce zero log output when half the writes fail.

The right way. Every error in a capture write path either propagates to the caller or, where continuation is required (batch processing), calls warn! with the full error and increments consecutive_failures in source_health. Clippy lint clippy::result_map_unit_fn and custom Semgrep rule flag silent-swallow in CI.

AP-12: INSERT OR IGNORE on a derived bucket key whose content is mutable

Forbidden. Using INSERT OR IGNORE (or INSERT … ON CONFLICT DO NOTHING) when the conflict key identifies a bucket whose row content is derived from an ever-growing source (e.g., (session_id, segment_index) for a JSONL segment that accretes messages over time).

Where seen: crates/hippo-daemon/src/claude_session.rs::insert_segments until 2026-04-27 (see T-A.3 and docs/archive/capture-reliability-overhaul/11-watcher-data-loss-fix.md ↗ Phase 1).

Why it’s wrong. (session_id, segment_index) is a bucket key — it identifies a slice of a JSONL session file, but the slice’s content (message_count, tool_calls_json, assistant_texts, etc.) grows over time as the file grows. The watcher re-runs extract_segments on every FSEvents notification. With INSERT OR IGNORE, the first partial extraction wins forever; subsequent reparses with more content are silently rejected. Result: every Claude session segment between 2026-04-25 and the fix was a tiny prefix of the actual content (median 2–4 messages out of hundreds; 59/63 recent segments had empty tool_calls_json — 6% non-empty vs ≈50%+ historical baseline).

Detection. A query like the following will show implausibly low tool-extraction rates after a regression:

SELECT
  ROUND(100.0 * COUNT(CASE WHEN json_array_length(tool_calls_json) > 0 THEN 1 END)
        / COUNT(*), 1) AS pct_with_tools
FROM claude_sessions
WHERE end_time > strftime('%s', 'now', '-7 days') * 1000;

Healthy baseline: ≥ 40%. During the bug: 6%.

Use instead. INSERT … ON CONFLICT(key) DO UPDATE SET (mutable cols). If you also need to detect insert-vs-update for downstream side effects (e.g., re-enqueueing for enrichment), compare a content hash stored on the row rather than keying off the conflict outcome alone.

INSERT INTO claude_sessions (session_id, segment_index, content_hash, tool_calls_json, …)
VALUES (?, ?, ?, ?, …)
ON CONFLICT(session_id, segment_index) DO UPDATE SET
    content_hash    = excluded.content_hash,
    tool_calls_json = excluded.tool_calls_json,
    …;

Rule of thumb. INSERT OR IGNORE is correct only when the conflict key uniquely identifies the entire immutable row (e.g., envelope_id for a fire-and-forget event, tool_use_id for a per-tool event). It is wrong for bucket keys whose row content is derived and mutable.

Cross-reference: docs/archive/capture-reliability-overhaul/11-watcher-data-loss-fix.md ↗ Phase 1.

AP-13: Minting a knowledge node without a per-segment replacement gate

Forbidden. An enrichment writer that does a blind INSERT INTO knowledge_nodes for a source segment without first removing the prior node(s) for that same segment. Enrichment is re-run whenever a segment is re-enqueued (file growth, a reparse, a stale-lock re-pend), so append-only writes accumulate near-duplicate nodes — each with its own vector and FTS row — that pollute retrieval and bloat the DB.

Where seen: every agentic/workflow/browser writer until 2026-05-29. The claude-session watcher re-enqueued growing segments and enrichment appended a fresh node each time; one 13cc2867 segment accreted 14 byte-identical nodes over 121 minutes, and ~2,486 redundant nodes (≈18% of the corpus) accumulated in total. Root cause: no upsert/replacement keyed on the source segment, and (for opencode) no daemon-side content-hash re-enqueue gate at all.

Scope of the fix. All node-minting enrichers are now guarded, by one of two mechanisms suited to their duplication shape:

Because every writer is now guarded, I-16 covers all node types (no longer scoped to 'observation').

Why it’s wrong. “ON CONFLICT makes re-ingest idempotent” is true for the session row but NOT for enrichment: the enrichment pipeline mints a brand-new UUID knowledge_nodes row per run. Re-enqueuing a finished segment is therefore not idempotent at the pipeline level — it multiplies nodes (and vectors, and FTS rows).

Detection. I-16 watches all node types continuously. Ad-hoc (matches I-16):

SELECT COUNT(*) FROM (
  SELECT 1 FROM knowledge_node_agentic_sessions kas
  JOIN knowledge_nodes kn ON kn.id = kas.knowledge_node_id
  GROUP BY kas.agentic_session_id, kn.content, kn.embed_text, kn.node_type
  HAVING COUNT(*) > 1);

Healthy: 0.

Use instead. Defenses by writer class:

  1. Daemon re-enqueue gate (agentic) — only re-enqueue a segment when its content_hash diverges from last_enriched_content_hash (decide_enqueue, mirrored across all four agentic pollers).
  2. Write-time replacement (agentic) — replace_prior_agentic_nodes(conn, segment_ids) deletes prior observation nodes linked solely to the segment set (all knowledge_node_* link rows + the vec0 vector, in the same transaction) before the new INSERT. Scoped to 'observation' so a co-linked workflow change_outcome node is never collateral.
  3. Skip-unchanged claim guard (agentic) — drop a claimed segment whose content_hash already equals last_enriched_content_hash before the LLM call.
  4. Write-time content dedup (workflow, browser) — find_identical_node(conn, content, embed_text, node_type) links a new run/visit onto an existing identical node instead of minting a copy.

Rule of thumb. A node delete must also delete the vec0 knowledge_vectors row in the same transaction — vec0 has no FK cascade, so a node-only delete silently orphans the vector into similarity search (the FTS row auto-drops via the knowledge_nodes_fts_ad trigger, but the vector does not).

Cross-reference: I-16 in architecture.md; remediation tool brain/scripts/dedup-knowledge-nodes.py; recipe in operator-runbook.md.