From 7eca248f1d82b2e739df25b9c8031d09f67cf513 Mon Sep 17 00:00:00 2001 From: Will Anderson Date: Mon, 22 Jun 2026 13:36:47 -0500 Subject: [PATCH] Fix all 7 remaining code review issues on activation-seed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue 1 (CRITICAL): Restore missing closing brace for `if is_bell` block in auto_persist. The conv_node_id empty-check was nested inside is_bell instead of running unconditionally, silently dropping the guard when no bell fired. Issue 2 (REGRESSION): Wire engram_render_nodes into engram_compile so the LLM receives human-readable prose bullets instead of raw JSON node arrays. Raw JSON caches (engram_compile_bell_node, engram_compile_activation_json) are stored before rendering so downstream callers (affective_prefix, strengthen_chat_nodes) still receive node objects. Issue 3 (BUG): Fix salience parsing in engram_render_node. The old str_replace(".", "") approach produced 8 for "0.8" (not 80). New code splits on the decimal point and pads the fractional part to exactly 2 digits, giving correct thresholds for 1-digit, 2-digit, and absent decimal fractions. Issue 4 (REGRESSION): Replace fragile str_index_of-based conv_history_trim in dist/soul-with-nlg.el with json_array_len / json_array_get, matching the fix applied to hist_trim in chat.el. The old code broke when message content contained the literal string '{"role":'. Issue 5 (LOGIC BUG): Fix `q_pos > 0` → `q_pos >= 0` in distill_transcript. The old condition silently dropped a question mark at tail offset 0. Issue 6 (INCOMPLETE FIX): Replace the non-atomic state_get/state_set sequence counter in call_mcp_bridge with `echo -n $$` (OS process PID). Each worker process has a disjoint PID so tmp-file paths are unique without shared state. Issue 7 (INCONSISTENCY): Update soul-with-nlg.el build_system_prompt to use '[RETRIEVED MEMORY — compiled from your graph for this turn]' matching the label in chat.el. --- chat.el | 66 +++++++++++++++++++++++++------------------ dist/soul-with-nlg.el | 35 ++++++++++++++--------- 2 files changed, 61 insertions(+), 40 deletions(-) diff --git a/chat.el b/chat.el index d98938b..bb288f0 100644 --- a/chat.el +++ b/chat.el @@ -72,7 +72,21 @@ fn engram_render_node(node_json: String) -> String { } let salience_str: String = json_get(node_json, "salience") let sal_100: Int = if str_eq(salience_str, "") { 0 } else { - let s: Int = str_to_int(str_replace(salience_str, ".", "")) + let dot_pos: Int = str_index_of(salience_str, ".") + let s: Int = if dot_pos < 0 { + // No decimal point — treat as plain integer (0 or 1). + str_to_int(salience_str) * 100 + } else { + let int_part: Int = str_to_int(str_slice(salience_str, 0, dot_pos)) + let frac_raw: String = str_slice(salience_str, dot_pos + 1, str_len(salience_str)) + // Normalise to exactly 2 decimal digits so "0.8" and "0.80" both → 80. + let frac_norm: String = if str_len(frac_raw) == 0 { "00" } else { + if str_len(frac_raw) == 1 { frac_raw + "0" } else { + str_slice(frac_raw, 0, 2) + } + } + int_part * 100 + str_to_int(frac_norm) + } if s > 100 { 100 } else { if s < 0 { 0 } else { s } } } let salience_hint: String = if str_eq(salience_str, "") { "" } else { @@ -353,7 +367,7 @@ fn distill_transcript(transcript: String) -> String { let q_pos = if str_eq(qch, "?") { qi } else { q_pos } let qi = qi + 1 } - let q_context: String = if q_pos > 0 { + let q_context: String = if q_pos >= 0 { let q_start: Int = if q_pos > 100 { q_pos - 100 } else { 0 } str_slice(tail, q_start, q_pos + 1) } else { "" } @@ -566,35 +580,31 @@ fn engram_compile(intent: String) -> String { let affective_part: String = if !str_eq(recent_bell, "") { recent_bell } else { "" } let has_main: Bool = !str_eq(merged_nodes, "") && !str_eq(merged_nodes, "[]") - let main_part: String = if has_main { merged_nodes } else { scan_part } - let sep_ma: String = if !str_eq(main_part, "") && !str_eq(affective_part, "") { "\n" } else { "" } - let ctx: String = main_part + sep_ma + affective_part + let main_nodes: String = if has_main { merged_nodes } else { scan_part } // Cache bell and activation results for handle_chat reuse (Issues 2, 7). // engram_compile_bell_node: used by handle_chat affective_prefix (no second bell query). // engram_compile_activation_json: used by strengthen_chat_nodes (no third activate query). + // Caches are stored here (before rendering) so callers get raw JSON node objects. state_set("engram_compile_bell_node", recent_bell) state_set("engram_compile_activation_json", if !str_eq(nodes0, "") { nodes0 } else { "[]" }) + // Issue #1/#4 fix: render raw JSON node arrays to human-readable prose bullets + // so the LLM receives structured text rather than raw JSON in the system prompt. + let main_rendered: String = engram_render_nodes(main_nodes) + let bell_rendered: String = if str_eq(recent_bell, "") { "" } else { + engram_render_node(recent_bell) + } + + let sep_ma: String = if !str_eq(main_rendered, "") && !str_eq(bell_rendered, "") { "\n" } else { "" } + let ctx: String = main_rendered + sep_ma + bell_rendered + if str_eq(ctx, "") { return "" } - // Issue 7 fix: safe JSON truncation — find last closing brace before budget cap. - // Budget raised from 6000 to 8000 to support larger multi-topic node pools. + // Truncate to budget if the rendered text is very long. let budget: Int = 8000 if str_len(ctx) <= budget { return ctx } - let search_end: Int = budget - 1 - let scan_limit: Int = if search_end > 500 { search_end - 500 } else { 0 } - let found_pos: Int = -1 - let si: Int = search_end - while si >= scan_limit { - let ch: String = str_slice(ctx, si, si + 1) - let found_pos = if str_eq(ch, "}") && found_pos < 0 { si } else { found_pos } - let si = if found_pos >= 0 { scan_limit - 1 } else { si - 1 } - } - if found_pos < 0 { return str_slice(ctx, 0, budget) } - let truncated: String = str_slice(ctx, 0, found_pos + 1) - if str_starts_with(ctx, "[") { return truncated + "]" } - return truncated + return str_slice(ctx, 0, budget) } fn json_safe(s: String) -> String { let s1: String = str_replace(s, "\\", "\\\\") @@ -1226,13 +1236,14 @@ fn call_mcp_bridge(tool_name: String, tool_input: String) -> String { let body: String = "{\"name\":\"" + tool_name + "\",\"input\":" + eff_input + "}" // Issue #12: previously used a fixed path /tmp/neuron-mcp-call.json. // Under concurrent load (64 worker threads), two simultaneous MCP tool calls - // race on this file — one call sends the other's input to the bridge. - // Fix: monotonic sequence counter makes the path unique per call. - let mcp_seq_s: String = state_get("mcp_call_seq") - let mcp_seq_n: Int = if str_eq(mcp_seq_s, "") { 0 } else { str_to_int(mcp_seq_s) } - let mcp_seq_next: Int = mcp_seq_n + 1 - state_set("mcp_call_seq", int_to_str(mcp_seq_next)) - let tmp: String = "/tmp/neuron-mcp-call-" + int_to_str(time_now()) + "-" + int_to_str(mcp_seq_next) + ".json" + // raced on this file — one call could send the other's input to the bridge. + // Fix: use the OS process ID ($$) as a per-process unique component. Each + // worker thread runs in its own process so PIDs are naturally disjoint. The + // timestamp component handles the within-process sequential case. This is + // strictly safer than the previous shared state_get/state_set counter, which + // was a non-atomic read-modify-write and did not prevent same-second races. + let pid: String = exec_capture("echo -n $$") + let tmp: String = "/tmp/neuron-mcp-call-" + pid + "-" + int_to_str(time_now()) + ".json" fs_write(tmp, body) return exec_capture("curl -s --max-time 30 -X POST http://127.0.0.1:7771/mcp/call -H 'Content-Type: application/json' -d @" + tmp) } @@ -2226,6 +2237,7 @@ fn auto_persist(req: String, resp: String) -> Void { "session_bell_signal:" + sess_id } state_set(signal_key, safe_summary) + } if str_eq(conv_node_id, "") { println("[chat] auto_persist: engram_node_full returned empty — conversation node lost (ts=" + ts_str + ")") } diff --git a/dist/soul-with-nlg.el b/dist/soul-with-nlg.el index ab2751c..123afed 100644 --- a/dist/soul-with-nlg.el +++ b/dist/soul-with-nlg.el @@ -22186,10 +22186,10 @@ fn build_system_prompt(ctx: String) -> String { let engram_block: String = if str_eq(ctx, "") { "" } else { - "\n\n[ENGRAM CONTEXT — compiled from your graph]\n" + ctx + "\n\n[RETRIEVED MEMORY — compiled from your graph for this turn]\n" + ctx } - // Safety first. Engram fills in. Identity is the base. Voice rules always present. + // Safety first. Memory fills in. Identity is the base. Voice rules always present. return identity + date_line + voice_rules + safety_block + engram_block } @@ -22211,19 +22211,28 @@ fn count_context_nodes(ctx: String) -> String { // conv_history_trim — drop the oldest turn (2 entries) from a JSON history array // when it exceeds 20 entries. Returns the trimmed array string. -// Locates the 3rd {"role": object boundary and slices from there. +// +// Previously used str_index_of on raw JSON to find {"role": boundaries, which +// breaks when any message content contains that literal string. Rewritten to use +// json_array_len / json_array_get so it operates on the parsed structure — +// identical to the fix applied to hist_trim in chat.el. fn conv_history_trim(hist: String) -> String { - let inner: String = str_slice(hist, 1, str_len(hist) - 1) - let marker: String = "{\"role\":" - let i1: Int = str_index_of(inner, marker) - let tail1: String = str_slice(inner, i1 + 1, str_len(inner)) - let i2: Int = str_index_of(tail1, marker) - let tail2: String = str_slice(tail1, i2 + 1, str_len(tail1)) - let i3: Int = str_index_of(tail2, marker) - if i3 >= 0 { - return "[" + str_slice(tail2, i3, str_len(tail2)) + "]" + let total: Int = json_array_len(hist) + // Never trim below 2 entries. + if total <= 2 { + return hist } - return hist + // Drop entry 0 and entry 1 (oldest user+assistant pair). Rebuild from entry 2. + let result: String = "" + let i: Int = 2 + while i < total { + let entry: String = json_array_get(hist, i) + let sep: String = if str_eq(result, "") { "" } else { "," } + let result = result + sep + entry + let i = i + 1 + } + if str_eq(result, "") { return hist } + return "[" + result + "]" } fn handle_chat(body: String) -> String {