fix(archivist): stop persisting, embedding and enriching from a heuristic recap

.github/workflows/ci-full.yml

@@ -132,8 +132,8 @@ jobs:
           # and leave a stale digest that only fails after the download. Both values
           # are the ubuntu-22.04-x86_64 row of `src/openhuman/modules/registry.rs`,
           # which is the authoritative pin — copy them from there, never recompute.
-          memory_version="1.15.3"
-          memory_sha256="5f50715abdc1b48745282db4718027159a292b5336d4bbd258cc471bb037b78e"
+          memory_version="1.16.0"
+          memory_sha256="f3ba06867ec89b8374a405f8a8569f5cecf88490609354ae4a6c1faa6e55b425"
           module_root="/opt/openhuman-test-modules/${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}"
           memory_dir="$module_root/tinymemory"
           juice_dir="$module_root/tinyjuice"

.github/workflows/ci-lite.yml

@@ -831,8 +831,8 @@ jobs:
           # and leave a stale digest that only fails after the download. Both values
           # are the ubuntu-22.04-x86_64 row of `src/openhuman/modules/registry.rs`,
           # which is the authoritative pin — copy them from there, never recompute.
-          memory_version="1.15.3"
-          memory_sha256="5f50715abdc1b48745282db4718027159a292b5336d4bbd258cc471bb037b78e"
+          memory_version="1.16.0"
+          memory_sha256="f3ba06867ec89b8374a405f8a8569f5cecf88490609354ae4a6c1faa6e55b425"
           module_root="/opt/openhuman-test-modules/${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}"
           memory_dir="$module_root/tinymemory"
           juice_dir="$module_root/tinyjuice"

.github/workflows/e2e-reusable.yml

@@ -168,8 +168,8 @@ jobs:
           # and leave a stale digest that only fails after the download. Both values
           # are the ubuntu-22.04-x86_64 row of `src/openhuman/modules/registry.rs`,
           # which is the authoritative pin — copy them from there, never recompute.
-          memory_version="1.15.3"
-          memory_sha256="5f50715abdc1b48745282db4718027159a292b5336d4bbd258cc471bb037b78e"
+          memory_version="1.16.0"
+          memory_sha256="f3ba06867ec89b8374a405f8a8569f5cecf88490609354ae4a6c1faa6e55b425"
           module_root="/opt/openhuman-test-modules/${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}"
           memory_dir="$module_root/tinymemory"
           memory_archive="$memory_dir/tinymemory-module-${memory_version}-ubuntu-22.04-x86_64.tar.gz"
@@ -374,8 +374,8 @@ jobs:
           # and leave a stale digest that only fails after the download. Both values
           # are the ubuntu-22.04-x86_64 row of `src/openhuman/modules/registry.rs`,
           # which is the authoritative pin — copy them from there, never recompute.
-          memory_version="1.15.3"
-          memory_sha256="5f50715abdc1b48745282db4718027159a292b5336d4bbd258cc471bb037b78e"
+          memory_version="1.16.0"
+          memory_sha256="f3ba06867ec89b8374a405f8a8569f5cecf88490609354ae4a6c1faa6e55b425"
           module_root="/opt/openhuman-test-modules/${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}"
           memory_dir="$module_root/tinymemory"
           memory_archive="$memory_dir/tinymemory-module-${memory_version}-ubuntu-22.04-x86_64.tar.gz"

scripts/ci/rust-coverage-changed.sh

@@ -107,9 +107,14 @@ integration_test_targets() {
 # tested, and wrong for domains whose contract lives in an integration target:
 # such a gate never runs on a PR that touches only the domain's `src/`.
 #
-#   src/openhuman/memory/** → the golden-workspace schema gates. They stand
-#   between a memory-store schema change and a corrupted user workspace, and
-#   they are `tests/` targets, so `--lib` scoping alone skips them entirely.
+#   `src/openhuman/memory/**` used to sit here, naming the golden-workspace
+#   schema gates. Both of those targets — `memory_golden_fixture_e2e` and
+#   `memory_golden_parity_e2e` — were deleted in cc99ba9c6, which cut the
+#   engine out of the test build. A mapping that names a target Cargo no longer
+#   has is not a weaker gate: it is a hard `error: no test target named …` on
+#   every PR that touches the domain, so the entry is gone rather than pointed
+#   at a substitute. The domain scopes to its `--lib` filter alone until there
+#   is a live gate to name again.
 #
 #   src/openhuman/agent/harness/session/** and src/openhuman/threads/goals/**
 #   → `agent_turn_overrides_e2e`. Per-turn `TurnOverrides` (`session/types.rs`)
@@ -124,9 +129,6 @@ integration_test_targets() {
 # empty result.
 domain_integration_targets() {
   case "$1" in
-    src/openhuman/memory/*)
-      printf '%s\n' memory_golden_fixture_e2e memory_golden_parity_e2e
-      ;;
     src/openhuman/agent/harness/session/* | src/openhuman/threads/goals/*)
       printf '%s\n' agent_turn_overrides_e2e
       ;;
@@ -338,14 +340,6 @@ for f in "${files[@]}"; do
         log "${f} → integration gate '--test ${extra_target}'"
       done < <(domain_integration_targets "${f}")
       ;;
-    tests/fixtures/memory_golden/*)
-      # The golden memory-workspace fixture (committed .db blobs + the derived
-      # manifest). A change here IS the schema-gate re-baseline, so run the
-      # gates rather than falling through to the `*)` full-suite arm.
-      test_targets_raw="${test_targets_raw}memory_golden_fixture_e2e
-"
-      log "${f} → integration gate '--test memory_golden_fixture_e2e'"
-      ;;
     tests/raw_coverage/*.rs)
       # The ~76 *_raw_coverage_e2e.rs suites are aggregated into the single
       # `raw_coverage_all` target (see tests/raw_coverage_all.rs + build.rs), so

src/openhuman/agent/harness/archivist/hook_impl.rs

@@ -158,7 +158,28 @@ impl PostTurnHook for ArchivistHook {
         // moves the tree write to segment granularity inside on_segment_closed.
         if let Some(ref segment) = closed_segment {
             let now = Self::now_timestamp();
-            self.on_segment_closed(segment, session_id, now).await;
+            let recap_succeeded = self.on_segment_closed(segment, session_id, now).await;
+            // Recover segments an earlier failed recap left unsummarised
+            // (#6186). Driven from here rather than from a timer because a
+            // recap that just succeeded is first-hand evidence that the
+            // summariser is answering *now* — a scheduler would have to guess.
+            //
+            // The gate is load-bearing, not a nicety. `resummarise_pending`
+            // spends a per-segment attempt budget, and running it while the
+            // provider is still down would burn that budget on calls that
+            // cannot succeed: two segment closes during one outage would
+            // exhaust every pending segment's retries and skip them for the
+            // rest of the process — including after the provider came back.
+            // The outage would consume the recovery it is supposed to trigger.
+            //
+            // Deliberately not called from `flush_open_segment`, the other
+            // caller of `on_segment_closed`: that one is awaited unbounded at
+            // session wind-down, and opportunistic recovery must never be
+            // charged to how long the app takes to close. This path is a
+            // detached post-turn hook, so the time is invisible.
+            if recap_succeeded {
+                self.resummarise_pending(now).await;
+            }
         }
 
         tracing::debug!("[archivist] turn indexed successfully: session={session_id}");

src/openhuman/agent/harness/archivist/lifecycle.rs

@@ -21,6 +21,23 @@ use std::time::{SystemTime, UNIX_EPOCH};
 /// question of the same role.
 const RECAP_INFERENCE_ROLE: &str = "summarization";
 
+/// Whether a finalize-time recap may be handed to a consumer that will treat
+/// it as real conversation content.
+///
+/// `from_llm == false` means [`ArchivistHook::summarize_entries`] returned the
+/// `boundary::fallback_summary` bookend — the segment's first and last 200
+/// characters joined by a pipe. That string is not a summary of anything; it
+/// is a placeholder that reads like one.
+///
+/// The emptiness term is a belt: a successful LLM recap is already non-empty
+/// by construction, because `summarize_entries` only reports `true` from the
+/// `Ok(output) if !output.content.is_empty()` arm. It fires on a shape that
+/// cannot occur today, and exists so a future arm cannot quietly reintroduce
+/// one.
+pub(super) fn recap_is_usable(from_llm: bool, summary: &str) -> bool {
+    from_llm && !summary.trim().is_empty()
+}
+
 impl ArchivistHook {
     /// Create an Archivist hook over the workspace's bound memory driver.
     ///
@@ -301,18 +318,33 @@ impl ArchivistHook {
 
     /// Called when a segment is closed.
     ///
-    /// Produces a segment recap (LLM if a chat provider is configured,
-    /// otherwise the heuristic fallback), embeds the recap, extracts
-    /// heuristic events, and updates the user profile.
+    /// Produces a segment recap, extracts heuristic events, updates the user
+    /// profile, and pipes the segment's raw turns into the memory tree.
+    ///
+    /// What happens to the recap depends on where it came from (#6156). An LLM
+    /// recap is persisted with `set_segment_summary`, embedded, and handed to
+    /// goals enrichment. The heuristic `boundary::fallback_summary` bookend is
+    /// none of those things — it is logged and dropped, leaving the segment
+    /// unsummarised on purpose. Everything downstream of the recap (events,
+    /// profile facets, tree ingest) runs either way, because none of it reads
+    /// the summary.
     ///
     /// Soft-fallback contract (mirrors `LlmSummariser`): this function
     /// never returns `Err`; all failures are logged and ignored.
+    /// Returns whether a usable LLM recap was produced for this segment.
+    ///
+    /// The caller uses it as a liveness signal, not as a success code: a `true`
+    /// means the summariser answered *just now*, which is the only first-hand
+    /// evidence the app gets that it is reachable. `false` covers every reason
+    /// a recap did not happen — no driver, no entries, no summariser, or a
+    /// summariser that failed — because none of them are a moment to spend
+    /// budget re-trying older segments against the same provider (#6186).
     pub(super) async fn on_segment_closed(
         &self,
         segment: &ConversationSegment,
         session_id: &str,
         now: f64,
-    ) {
+    ) -> bool {
         // Gather the conversation text for this segment. Prefer the
         // md-backed memory_archivist read when config is available; fall
         // back to the driver's episodic family otherwise.
@@ -332,7 +364,7 @@ impl ArchivistHook {
                 "[archivist] segment={} has no entries — skipping recap",
                 segment.segment_id
             );
-            return;
+            return false;
         }
 
         // Build segment text from user messages (for event extraction).
@@ -344,33 +376,74 @@ impl ArchivistHook {
             .join(". ");
 
         // ── Segment recap (LLM or heuristic fallback) ────────────────────
-        let (summary, _from_llm) = self
+        let (summary, from_llm) = self
             .summarize_entries(&segment_entries, &segment.segment_id, segment.turn_count)
             .await;
 
-        // Persist the recap.
-        let set_summary = match self.episodic() {
-            Some(episodic) => {
-                episodic
-                    .set_segment_summary(&segment.segment_id, &summary, now)
-                    .await
-            }
-            None => return,
+        // Hoisted above the two recap arms because what is missing here is the
+        // driver, not the summary: with no episodic family there is nothing to
+        // write on either arm, and this is the exit that path has always taken.
+        let Some(episodic) = self.episodic() else {
+            return false;
         };
-        if let Err(e) = set_summary {
-            tracing::warn!("[archivist] failed to set segment summary: {e}");
+
+        if recap_is_usable(from_llm, &summary) {
+            // Persist the recap.
+            let set_summary = episodic
+                .set_segment_summary(&segment.segment_id, &summary, now)
+                .await;
+            if let Err(e) = set_summary {
+                tracing::warn!("[archivist] failed to set segment summary: {e}");
+            } else {
+                tracing::debug!(
+                    "[archivist] recap persisted segment={} summary_chars={}",
+                    segment.segment_id,
+                    summary.len()
+                );
+            }
+
+            // ── Finalize-time embedding ───────────────────────────────────
+            self.embed_segment_recap(&segment.segment_id, &summary, now)
+                .await;
         } else {
-            tracing::debug!(
-                "[archivist] recap persisted segment={} summary_chars={}",
-                segment.segment_id,
-                summary.len()
-            );
+            // #6156. The bookend is not written, not embedded, and not handed
+            // to goals enrichment.
+            //
+            // Persisting it would be worse than storing nothing, because
+            // `segment_set_summary` also flips the row to `status='summarised'`
+            // — and that flip is the one thing that removes the segment from
+            // the `segments_pending_summary` query a later re-summarisation
+            // pass selects on. Leaving the row `'closed'` with a NULL summary
+            // IS the provenance marker, at no schema cost.
+            //
+            // Nothing is lost by dropping the bookend either: it is derived
+            // from the segment's first and last turn, both of which stay in the
+            // episodic store, so it can be recomputed at any time.
+            //
+            // WARN only when a summariser was actually expected. With none
+            // configured for the workspace at all this is the steady state
+            // rather than a degradation, and warning once per segment close
+            // would bury the case #6156 is about — a summariser that exists
+            // and did not answer — in noise from the case that is working as
+            // intended.
+            if self.summariser_available {
+                tracing::warn!(
+                    "[archivist] no LLM recap for segment={} ({} turns) — summary NOT persisted, \
+                     NOT embedded, NOT sent to goals enrichment; the segment stays unsummarised \
+                     so a later pass can recap it once a summariser answers",
+                    segment.segment_id,
+                    segment.turn_count,
+                );
+            } else {
+                tracing::debug!(
+                    "[archivist] no summariser for this workspace — segment={} ({} turns) left \
+                     unsummarised",
+                    segment.segment_id,
+                    segment.turn_count,
+                );
+            }
         }
 
-        // ── Finalize-time embedding ───────────────────────────────────────
-        self.embed_segment_recap(&segment.segment_id, &summary, now)
-            .await;
-
         // ── Heuristic event extraction ────────────────────────────────────
         if !segment_text.is_empty() {
             let extracted = extract_events_heuristic(&segment_text);
@@ -469,7 +542,13 @@ impl ArchivistHook {
         // the user's durable goals list stays fresh. Feed it the fresh recap
         // as context. Detached + non-fatal: never blocks segment close.
         if let Some(ref cfg) = self.config {
-            if cfg.learning.goals_enrichment_enabled && !summary.trim().is_empty() {
+            // #6156: the recap term leads deliberately. Handing the bookend to
+            // the goals agent means an LLM call whose entire context is two
+            // truncated utterances, and it will invent durable goals out of
+            // that noise. Ordering it ahead of the config flag also keeps it
+            // evaluated whenever a config is attached, rather than being
+            // short-circuited away by an unrelated toggle.
+            if recap_is_usable(from_llm, &summary) && cfg.learning.goals_enrichment_enabled {
                 tracing::debug!(
                     "[memory_goals] segment closed — spawning goals enrichment \
                      session={session_id} segment={}",
@@ -486,6 +565,8 @@ impl ArchivistHook {
                 );
             }
         }
+
+        recap_is_usable(from_llm, &summary)
     }
 
     /// Embed `summary` for `segment_id` and write the per-model embedding row.
@@ -499,6 +580,11 @@ impl ArchivistHook {
     /// zero entries) and an empty embed input is guaranteed to 400 from
     /// the upstream embedding API (#13021). The segment is sealed without
     /// an embedding row; subsequent recap edits can re-embed.
+    ///
+    /// Since #6156 the finalize caller also declines to call this at all for a
+    /// heuristic recap, so this guard is no longer the only thing standing
+    /// between a bookend stub and the embedder. It now covers direct callers
+    /// and any future one that has not made that decision for itself.
     pub(super) async fn embed_segment_recap(&self, segment_id: &str, summary: &str, now: f64) {
         if summary.trim().is_empty() {
             tracing::warn!(
@@ -573,3 +659,7 @@ impl ArchivistHook {
         }
     }
 }
+
+#[cfg(test)]
+#[path = "lifecycle_tests.rs"]
+mod tests;