Compare commits

...

2 Commits

Author SHA1 Message Date
will.anderson 309c388b5b fix(recall): address four remaining recall-reliability review issues
1. engram_numeric_valid: "0.0"/"0.00"/"00.0" now return true — after
   removing dots, strip all '0' chars; non-empty remainder means
   non-numeric (letters), empty means genuinely all-zero, which is valid.
   Prevents zero-salience/importance nodes being scored at the 70 default
   and ranked above the threshold they deserve to sit below.

2. Partial-write guard: replace str_contains(hist, "]") with
   str_ends_with(hist, "]") in conv_history_persist, conv_history_load
   (label path) and conv_history_load (vector fallback path). A truncated
   array whose *content* contains "]" (e.g. "item 1] item 2") no longer
   passes the guard.

3. Cold-start Persona fallback: replace engram_search_json("soul:persona
   Persona identity", 5) with engram_get_node_by_label("soul:persona").
   The label lookup is index-independent, so the fallback now actually
   works when the vector index is cold — the exact condition that
   triggers this branch.

4. handle_chat_agentic hard_bell block: add the missing closing } after
   the return statement. The unclosed if caused undefined control flow
   from line 1101 onward for any non-hard-bell path.
2026-06-22 13:34:05 -05:00
will.anderson a39998a502 feat(recall): recall-reliability improvements
Neuron Soul CI / build (pull_request) Failing after 12m52s
- Q1: engram_numeric_valid() guard against non-numeric timestamps in bell scoring
- Q2: soul-agnostic cold-start fallback in engram_compile (drops genesis-specific hardcoded node IDs)
- Q3: partial-write guard and failure logging in conv_history_persist/load
- Q4: document circuit-breaker limitation requiring C runtime support
- Q5: println warnings on empty activation/search paths
- Q6: load_identity_context warns when all identity fetches return empty
- Q7: recall_status state tracking (ok/empty/unavailable) surfaced to LLM via MEMORY STATUS block
- Q8: document shared-state race conditions in engram_recall_status and safety_system_addendum
- CRITICAL BUG: conv_node_id empty check moved outside is_bell block so silent Conversation node loss is always logged
2026-06-22 12:52:31 -05:00
2 changed files with 202 additions and 34 deletions
+179 -34
View File
@@ -12,34 +12,66 @@ fn chat_default_model() -> String {
return "claude-sonnet-4-5"
}
// engram_numeric_valid guard for str_to_int: returns true only when s is a valid
// decimal number (integer or single-decimal-point float, optional leading minus).
// Q1 fix: rejects "", "null", "N/A", multi-dot strings ("1.2.3"), pure-letter strings.
// Prevents engram_score_node from passing malformed JSON field values to str_to_int
// which has undefined behaviour on non-numeric input and can corrupt score arithmetic.
fn engram_numeric_valid(s: String) -> Bool {
if str_eq(s, "") { return false }
if str_eq(s, "null") { return false }
if str_eq(s, "N/A") { return false }
if str_eq(s, "-") { return false }
let body: String = if str_starts_with(s, "-") { str_slice(s, 1, str_len(s)) } else { s }
if str_eq(body, "") { return false }
// Count dots: remove all, compare lengths. Allow at most one dot (float).
let no_dot: String = str_replace(body, ".", "")
let dot_count: Int = str_len(body) - str_len(no_dot)
if dot_count > 1 { return false }
if str_eq(no_dot, "") { return false }
// str_to_int on a letter-containing string returns 0; "0" and "00..." (e.g. from "0.0")
// are valid zeros. We accept any all-zero no_dot string; reject only when it contains
// non-digit characters (str_to_int returns 0 for those too).
let parsed: Int = str_to_int(no_dot)
if parsed == 0 {
// Verify no_dot is truly all-digit-zeros, not a letter-contaminated string.
// Strip all '0' characters; if anything remains the string is non-numeric.
let stripped_zeros: String = str_replace(no_dot, "0", "")
if !str_eq(stripped_zeros, "") { return false }
}
return true
}
// engram_score_node compute a recency x relevance score for a single engram
// node JSON object. Higher is better. Score = salience * importance * recency_factor.
// recency_factor decays linearly over 30 days: nodes updated today score 1.0,
// nodes 30+ days old score 0.1 (floor). Nodes with no created_at score 0.5.
// This keeps fresh, high-salience nodes at the top and pushes stale low-signal
// nodes to the bottom so they get trimmed when we cap context size.
// Q1 fix: all three numeric fields validated with engram_numeric_valid before str_to_int.
fn engram_score_node(node_json: String) -> Int {
let salience_str: String = json_get(node_json, "salience")
let importance_str: String = json_get(node_json, "importance")
let created_str: String = json_get(node_json, "created_at")
// Parse as floats via * 100 integer arithmetic (el has no float math)
let salience_100: Int = if str_eq(salience_str, "") { 70 } else {
// Q1 fix: validate before str_to_int. Non-numeric values fall back to safe defaults.
// Parse as floats via * 100 integer arithmetic (el has no float math).
let salience_100: Int = if !engram_numeric_valid(salience_str) { 70 } else {
let s: Int = str_to_int(str_replace(salience_str, ".", ""))
// Clamp to 0-100 range (value was e.g. "0.85" -> parsed "085" = 85)
if s > 100 { 100 } else { if s < 0 { 0 } else { s } }
}
let importance_100: Int = if str_eq(importance_str, "") { 70 } else {
let importance_100: Int = if !engram_numeric_valid(importance_str) { 70 } else {
let v: Int = str_to_int(str_replace(importance_str, ".", ""))
if v > 100 { 100 } else { if v < 0 { 0 } else { v } }
}
// Recency: decay from 100 (today) to 10 (30+ days). created_at is Unix seconds.
let now_ts: Int = time_now()
let recency_100: Int = if str_eq(created_str, "") { 50 } else {
let recency_100: Int = if !engram_numeric_valid(created_str) { 50 } else {
let created_ts: Int = str_to_int(created_str)
let age_secs: Int = now_ts - created_ts
let age_days: Int = age_secs / 86400
// Q1 fix: guard against clock skew / future timestamps treat as fresh.
let age_days: Int = if age_secs < 0 { 0 } else { age_secs / 86400 }
let decay: Int = if age_days >= 30 { 10 } else { 100 - (age_days * 3) }
if decay < 10 { 10 } else { decay }
}
@@ -116,13 +148,23 @@ fn engram_compile_ranked(nodes_json: String, max_nodes: Int) -> String {
return c9
}
// Q4 note: engram_compile has no cache or circuit-breaker at the EL layer.
// Every handle_chat call invokes engram_activate_json + engram_search_json unconditionally.
// If the engram backend is repeatedly unreachable (e.g., during startup or after a crash),
// every turn pays two failed RPC round-trips before reaching the cold-start fallback.
// A proper cache/circuit-breaker requires C runtime support (e.g., a shared "engram_healthy"
// flag set by the runtime, or a time-bucketed result cache in el_runtime.c). At the EL
// layer we can only detect failure after the fact (empty string return) and log it.
fn engram_compile(intent: String) -> String {
let activate_json: String = engram_activate_json(intent, 5)
// Fetch more search results than we'll use so ranking has a real pool to pick from.
let search_json: String = engram_search_json(intent, 20)
let act_ok: Bool = !str_eq(activate_json, "") && !str_eq(activate_json, "[]")
let srch_ok: Bool = !str_eq(search_json, "") && !str_eq(search_json, "[]")
// Q6/Q7 fix: track raw "" (engram down) vs "[]" (empty graph) to surface different warnings.
let act_failed: Bool = str_eq(activate_json, "")
let srch_failed: Bool = str_eq(search_json, "")
let act_ok: Bool = !act_failed && !str_eq(activate_json, "[]")
let srch_ok: Bool = !srch_failed && !str_eq(search_json, "[]")
// Activation nodes (spreading activation) are already high-signal keep all 5.
let act_part: String = if act_ok { activate_json } else { "" }
@@ -132,23 +174,37 @@ fn engram_compile(intent: String) -> String {
let srch_ranked: String = if srch_ok { engram_compile_ranked(search_json, 8) } else { "" }
let srch_part: String = srch_ranked
// Fallback: when vector search returns nothing (no embeddings), fetch pinned
// high-salience nodes by their known IDs. These are the canonical identity
// and biography nodes that should always be in context.
// engram_get_node_json(id) returns a single node as JSON or "" if missing.
// Q2 fix: soul-agnostic cold-start fallback. The previous code used two genesis-specific
// hardcoded node IDs ("knw-35940684..." and "knw-729fc901..."). Cultivated souls with a
// cold or empty vector index received zero episodic context with no error and no log.
// New fallback: search for Persona/Identity nodes seeded by seed_persona_from_env()
// which works for any soul regardless of which specific node IDs were created at seeding.
// Q6 fix: log a warning so the empty-recall path is visible in operator logs.
let scan_part: String = if !act_ok && !srch_ok {
let family_node: String = engram_get_node_json("knw-35940684-abc4-42f0-b942-818f66b1f69a")
let origin_node: String = engram_get_node_json("knw-729fc901-8335-44c4-9f3a-b150b4aa0915")
let fam_ok: Bool = !str_eq(family_node, "") && !str_eq(family_node, "null")
let orig_ok: Bool = !str_eq(origin_node, "") && !str_eq(origin_node, "null")
let fam_str: String = if fam_ok { family_node } else { "" }
let orig_str: String = if orig_ok { origin_node } else { "" }
let sep: String = if fam_ok && orig_ok { "\n" } else { "" }
let combined: String = fam_str + sep + orig_str
if str_eq(combined, "") { "" } else { combined }
let engram_down: Bool = act_failed && srch_failed
if engram_down {
println("[chat] engram_compile: WARN engram_down — all calls returned empty string for intent=" + str_slice(intent, 0, 60))
} else {
println("[chat] engram_compile: WARN cold-index — activation and search returned no results for intent=" + str_slice(intent, 0, 60))
}
// Soul-agnostic fallback: fetch the Persona node by label immune to cold vector index.
// seed_persona_from_env() always writes this node with label "soul:persona", so
// engram_get_node_by_label works even when the vector index has not yet been built.
// Using engram_search_json here would fail for the same reason as the primary path
// (vector index cold), defeating the purpose of this fallback branch entirely.
let persona_node: String = engram_get_node_by_label("soul:persona")
let pf_node_ok: Bool = !str_eq(persona_node, "") && !str_eq(persona_node, "null")
let persona_arr: String = if pf_node_ok { "[" + persona_node + "]" } else { "" }
let pf_ok: Bool = pf_node_ok
let combined: String = if pf_ok { engram_compile_ranked(persona_arr, 1) } else { "" }
if str_eq(combined, "") {
println("[chat] engram_compile: WARN cold-start fallback also empty — LLM has no episodic context")
}
combined
} else {
""
}
let scan_ok: Bool = !str_eq(scan_part, "")
// Affective context: always include the most recent high-emotion memory if one
// exists within 72 hours. This ensures continuity of care across turns when
@@ -178,17 +234,31 @@ fn engram_compile(intent: String) -> String {
let ca: String = json_get(bn0, "created_at")
if str_eq(ca, "") { json_get(bn0, "updated_at") } else { ca }
}
let bn_ts: Int = if str_eq(bn_ts_raw, "") { 0 } else { str_to_int(bn_ts_raw) }
// Q1 fix: validate bell timestamp before str_to_int.
let bn_ts: Int = if !engram_numeric_valid(bn_ts_raw) { 0 } else { str_to_int(bn_ts_raw) }
if bn_ts > cutoff_ts { bn0 } else { "" }
} else { "" }
let affective_part: String = if !str_eq(recent_bell, "") { recent_bell } else { "" }
let affective_ok: Bool = !str_eq(affective_part, "")
let sep1: String = if !str_eq(act_part, "") && !str_eq(srch_part, "") { "\n" } else { "" }
let sep2: String = if (!str_eq(act_part, "") || !str_eq(srch_part, "")) && !str_eq(scan_part, "") { "\n" } else { "" }
let sep3: String = if (!str_eq(act_part, "") || !str_eq(srch_part, "") || !str_eq(scan_part, "")) && !str_eq(affective_part, "") { "\n" } else { "" }
let ctx: String = act_part + sep1 + srch_part + sep2 + scan_part + sep3 + affective_part
if str_eq(ctx, "") { return "" }
// Q7 fix: store recall status so build_system_prompt can include a hint to the LLM
// distinguishing "no memories yet" (cold start) from "memory system unreachable".
// Values: "ok" | "empty" | "unavailable"
let any_ok: Bool = act_ok || srch_ok || scan_ok || affective_ok
let all_failed: Bool = act_failed && srch_failed
let recall_status: String = if any_ok { "ok" } else { if all_failed { "unavailable" } else { "empty" } }
state_set("engram_recall_status", recall_status)
if str_eq(ctx, "") {
// Q6 fix: log when ctx is empty after all recall paths so cold-start is visible.
println("[chat] engram_compile: all paths empty — recall_status=" + recall_status + " intent=" + str_slice(intent, 0, 60))
return ""
}
// Raise the cap slightly to match the ranked (higher-signal) output.
if str_len(ctx) > 6000 {
@@ -227,12 +297,33 @@ fn build_system_prompt(ctx: String) -> String {
"\n\n[IDENTITY GRAPH — who you are, loaded from your engram]\n" + id_ctx
}
// Q7 fix: if recall produced no results, include a hint so the LLM can respond
// authentically ("I seem to be starting fresh" vs "memory system may be down")
// rather than silently acting as if it has context it doesn't have.
// Q8 note: "engram_recall_status" is a shared state key under http_serve_async.
// Concurrent requests can overwrite each other's status. This is best-effort:
// a full fix requires per-request scoping (not feasible at EL layer without C support).
let recall_status: String = state_get("engram_recall_status")
let engram_block: String = if str_eq(ctx, "") {
""
let status_hint: String = if str_eq(recall_status, "unavailable") {
"\n\n[MEMORY STATUS]\nYour episodic memory system appears to be temporarily unreachable. You may not have access to memories from previous sessions. If asked about past conversations, acknowledge this honestly rather than confabulating."
} else if str_eq(recall_status, "empty") {
"\n\n[MEMORY STATUS]\nNo episodic memories were found for this topic. This may be a new soul or a new area of conversation. Respond naturally from your identity without fabricating memories."
} else {
""
}
status_hint
} else {
"\n\n[ENGRAM CONTEXT — compiled from your graph]\n" + ctx
}
// Q8 note: layered_cycle_safety_system_addendum is a shared mutable state key.
// Two concurrent requests can both read it (state_get), both see the same value,
// and one clears it (state_set("", "")) while the other uses the value or both
// clear it and one request gets "" while expecting real content. The race is benign
// in practice (the addendum is only written by layered_cycle and read here once
// per turn; concurrent chat turns are rare in the current deployment), but a full
// fix requires per-session or per-request key scoping at the C runtime level.
let safety_addendum: String = state_get("layered_cycle_safety_system_addendum")
let safety_block: String = if str_eq(safety_addendum, "") {
""
@@ -347,41 +438,82 @@ fn clean_llm_response(s: String) -> String {
}
// conv_history_persist save conversation history to engram for cross-restart continuity.
// Stores as a Conversation node. Overwrites by using consistent label "conv:history".
// Stores as a Conversation node with consistent label "conv:history" (upsert by label).
// Q3/Q6 fix: added partial-write guard and failure logging.
fn conv_history_persist(hist: String) -> Void {
if str_eq(hist, "") { return "" }
if str_eq(hist, "[]") { return "" }
let ts: Int = time_now()
// Partial-write guard: refuse to persist a blob that is not a complete JSON array.
// A truncated write starting with '[' but missing the closing ']' must be rejected.
// str_ends_with is used (not str_contains) so that embedded ']' characters in content
// (e.g. "item 1] item 2") do not fool the guard when the array tail is actually missing.
if !str_starts_with(hist, "[") { return "" }
if !str_ends_with(hist, "]") { return "" }
let tags: String = "[\"conv-history\",\"persistent\"]"
let discard: String = engram_node_full(
let node_id: String = engram_node_full(
hist, "Conversation", "conv:history",
el_from_float(0.7), el_from_float(0.8), el_from_float(0.9),
"Episodic", tags
)
// Q6 fix: log write failure silent history loss is now visible.
if str_eq(node_id, "") {
println("[chat] conv_history_persist: engram_node_full returned empty — history node may be lost")
}
}
// conv_history_load restore conversation history from engram on first access.
// Returns the most recent "conv:history" node content, or "" if none found.
// Q3/Q6 fix: added partial-write guard, log on invalid content, and state flag for
// callers to distinguish genuine first-turn from a load failure.
fn conv_history_load() -> String {
// Primary: label-based fetch symmetric with persist, immune to vector index drift.
let label_node: String = engram_get_node_by_label("conv:history")
let label_ok: Bool = !str_eq(label_node, "") && !str_eq(label_node, "null")
if label_ok {
let label_content: String = json_get(label_node, "content")
let label_valid: Bool = str_starts_with(label_content, "[") && str_ends_with(label_content, "]")
if label_valid {
return label_content
}
println("[chat] conv_history_load: label node found but content invalid — falling back to vector search")
}
// Fallback: vector search.
let results: String = engram_search_json("conv:history", 3)
if str_eq(results, "") { return "" }
if str_eq(results, "") {
// Q3 fix: set a state flag so callers can distinguish load failure from first turn.
state_set("conv_history_load_failed", "1")
return ""
}
if str_eq(results, "[]") { return "" }
let node: String = json_array_get(results, 0)
let content: String = json_get(node, "content")
// Validate it looks like a JSON array
if !str_starts_with(content, "[") { return "" }
// Partial-write guard: require both '[' prefix AND closing ']' at the tail.
// str_ends_with guards against embedded ']' in content fooling the check.
if !str_starts_with(content, "[") || !str_ends_with(content, "]") {
println("[chat] conv_history_load: vector search result content invalid — treating as first turn")
state_set("conv_history_load_failed", "1")
return ""
}
return content
}
fn handle_chat(body: String) -> String {
let message: String = json_get(body, "message")
if str_eq(message, "") {
return "{\"error\":\"message is required\",\"response\":\"\"}"
return "{\"__status__\":400,\"error\":\"message is required\",\"response\":\"\"}"
}
// Load history BEFORE compiling context so we can anchor activation to the thread.
// Q3 fix: clear the load-failure flag before loading so it accurately reflects this call.
state_set("conv_history_load_failed", "")
// Q8 note: "conv_history" is a process-global state key. Concurrent /api/chat requests
// all read the same key, append their exchange, and write it back. Because _state_mu
// serializes individual state_get/state_set calls but NOT the read-append-write sequence,
// two concurrent requests can read the same base history and the last writer wins one
// turn is silently dropped. A full fix requires per-session history keys (session_hist_<id>)
// and deprecating the global "conv_history" path. Callers using session_id are not affected.
let state_hist: String = state_get("conv_history")
let stored_hist: String = if str_eq(state_hist, "") { conv_history_load() } else { state_hist }
let hist_load_failed: Bool = str_eq(state_get("conv_history_load_failed"), "1")
let hist_len: Int = if str_eq(stored_hist, "") { 0 } else { json_array_len(stored_hist) }
// Thread-aware activation: short/ambiguous messages (continuations like "go on",
@@ -531,7 +663,13 @@ fn handle_chat(body: String) -> String {
let act_out: String = if act_ok { activation_nodes } else { "[]" }
strengthen_chat_nodes(act_out)
return "{\"response\":\"" + safe_response + "\",\"model\":\"" + model + "\",\"activation_nodes\":" + act_out + "}"
// Q3 fix: surface history load failure in the response envelope so callers can
// show a "starting fresh — could not load previous conversation" indicator.
let hist_warning: String = if hist_load_failed {
",\"history_load_failed\":true"
} else { "" }
return "{\"response\":\"" + safe_response + "\",\"model\":\"" + model + "\",\"activation_nodes\":" + act_out + hist_warning + "}"
}
fn handle_see(body: String) -> String {
@@ -974,7 +1112,7 @@ fn handle_chat_agentic(body: String) -> String {
if str_eq(screen_action, "hard_bell") {
safety_log_bell("hard", json_get(screen_result, "reason"), str_slice(message, 0, 80))
return "{\"reply\":\"" + json_safe(safety_validate("", "hard_bell")) + "\",\"model\":\"\",\"agentic\":true,\"tools_used\":[]}"
}
let req_model: String = json_get(body, "model")
let model: String = if str_eq(req_model, "") { chat_default_model() } else { req_model }
@@ -1536,6 +1674,13 @@ fn auto_persist(req: String, resp: String) -> Void {
"Episodic",
tags
)
// CRITICAL BUG fix: log conv_node_id failure OUTSIDE the is_bell block.
// The original code had this check inside the is_bell block (or missing entirely),
// making the log unreachable on every non-bell turn (the common case). This meant
// silent failure of the Conversation node write went unlogged on most turns.
if str_eq(conv_node_id, "") {
println("[chat] auto_persist: engram_node_full returned empty — conversation node lost (ts=" + ts_str + ")")
}
// When a bell fires, write a dedicated BellEvent node in addition to the
// Conversation node. This makes distress moments directly findable by label
+23
View File
@@ -148,6 +148,14 @@ fn load_identity_context() -> Void {
println("[soul] identity context loaded (" + int_to_str(str_len(ctx)) + " chars, " + int_to_str(parts_count) + " nodes)")
}
// Q6 fix: warn when all three identity node fetches return empty. For genesis this
// indicates a corrupted or missing graph. For cultivated souls it is expected on first
// boot (nodes are seeded by seed_persona_from_env, not these genesis-specific IDs).
// The log makes the silent-empty case visible instead of indistinguishable from success.
if parts_count == 0 {
println("[soul] load_identity_context: WARN all three identity node fetches returned empty — no graph-derived identity context loaded")
}
// Scan for a Persona node the explicit identity declaration seeded into cultivated souls.
// Stored at seeding time with label "soul:persona" and node_type "Persona".
// genesis derives identity from the graph directly; cultivated souls have this node seeded.
@@ -162,6 +170,12 @@ fn load_identity_context() -> Void {
println("[soul] persona node loaded (" + int_to_str(str_len(p_content)) + " chars)")
}
}
// Q6 fix: if neither identity nodes nor persona node were loaded, log explicitly.
let soul_id_ctx: String = state_get("soul_identity_context")
let soul_persona_ctx: String = state_get("soul_persona")
if str_eq(soul_id_ctx, "") && str_eq(soul_persona_ctx, "") {
println("[soul] load_identity_context: WARN no identity context available from graph — soul will have identity_block empty in system prompts")
}
}
// seed_persona_from_env one-time migration: SOUL_IDENTITY env var Persona graph node.
@@ -327,6 +341,15 @@ fn layered_cycle(raw_input: String) -> String {
// TODO: wire directly when imprint_respond gains system_override param (imprint.el change).
// ISSUE 3 TODO: no semantic crisis detection. Keyword-only means signals that evade
// the phrase list pass with zero augmentation. Semantic layer = separate decision.
//
// Q8 race documentation: "layered_cycle_safety_system_addendum" is a shared process-global
// state key. Two concurrent requests to layered_cycle() both write this key; whichever
// writes last wins. The concurrent build_system_prompt() read in chat.el:236 may then
// consume the wrong request's addendum, or find an empty string after the other request's
// build_system_prompt consumed and cleared it. Mitigation: under http_serve_async, the
// layered_cycle path and the /api/chat path are different endpoints (typically); true
// concurrent layered_cycle calls are uncommon. A robust fix requires per-request state
// scoping which needs C runtime support (e.g. a request-id-keyed addendum map).
let augmented_addendum: String = safety_augment_system("", raw_input)
state_set("layered_cycle_safety_system_addendum", augmented_addendum)