diff --git a/chat.el b/chat.el index ec64cba..174c927 100644 --- a/chat.el +++ b/chat.el @@ -12,34 +12,59 @@ 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" is a valid zero. + let parsed: Int = str_to_int(no_dot) + if parsed == 0 && !str_eq(no_dot, "0") { 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 } } @@ -265,6 +290,13 @@ fn engram_nodes_merge(a: String, b: String) -> String { return engram_dedup_nodes("[" + ai + "," + bi + "]") } +// 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 { // Issue 1: decompose multi-topic messages into sub-queries. let topics: String = engram_split_topics(intent) @@ -376,7 +408,8 @@ 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 { "" } // Positive emotion context: check for recent joy/success moments within 72h. @@ -410,7 +443,19 @@ fn engram_compile(intent: String) -> String { let sep_ma: String = if !str_eq(main_part, "") && !str_eq(affective_part, "") { "\n" } else { "" } let ctx: String = main_part + sep_ma + 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 "" + } // Issue 7 fix: safe JSON truncation — find last closing brace before budget cap. // Budget raised from 6000 to 8000 for the larger multi-topic pool. @@ -460,12 +505,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, "") { "" @@ -580,36 +646,65 @@ 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 ']' would overwrite a good node. + if !str_starts_with(hist, "[") { return "" } + if !str_contains(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_contains(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 ']' presence. + if !str_starts_with(content, "[") || !str_contains(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. @@ -617,6 +712,7 @@ fn handle_chat(body: String) -> String { // /api/chat requests without session_id race on this read-append-write. 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) } // Issue 8 fix: use semantic continuation detection instead of brittle 50-char threshold. @@ -833,7 +929,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 { @@ -1847,6 +1949,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 diff --git a/soul.el b/soul.el index d0c212c..56dfb13 100644 --- a/soul.el +++ b/soul.el @@ -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.