Compare commits

..

2 Commits

Author SHA1 Message Date
will.anderson 47d0e6f985 fix(reliability): llm-retry — empty response detection, configurable max_tokens, connector timeout
Neuron Soul CI / build (pull_request) Failing after 11m16s
Issue #5: detect empty string from llm_extract_text() as an error in handle_chat,
handle_chat_as_soul, and handle_dharma_room_turn. The C runtime silently returns ""
when the LLM response content array is missing or all blocks fail to parse; without
this guard the empty string passes through to callers as a silent empty reply.

Issue #9: make agentic_loop max_tokens configurable via NEURON_LLM_MAX_TOKENS env
var (default 4096). The hardcoded value is marginal for long tool chains (8 iterations
x 4096 tokens); operators can now set 8192+ for complex multi-step tasks without
rebuilding. Non-agentic path (llm_call_system) still uses the C runtime hardcode —
that fix lives in el_runtime.c (see TODO block added in this commit).

Issue #10: increase connector_tools_json and tool_auto_approved curl --max-time from
2s to 5s to reduce false-empty tool lists when neuron-connectd is under transient
load. Graceful degradation to [] on bridge down is unchanged.

Issues #1/#2/#3/#4/#6/#8: documented as TODO comments in chat.el. These require
targeted C runtime changes in el_runtime.c (llm_provider_request retry loop,
EL_LLM_TIMEOUT_MS separation, HTTP 429 backoff, 5xx retry, EL_HTTP_MAX_RESPONSE_BYTES
cap). Architectural decisions recorded so they are traceable to root causes.
2026-06-22 11:59:43 -05:00
will.anderson deddb9a18e fix(reliability): safety-resilience — bell augmentation, safe mode, dedup logging, tab escaping, handle_chat coverage 2026-06-22 11:53:07 -05:00
3 changed files with 85 additions and 21 deletions
+55 -3
View File
@@ -380,9 +380,13 @@ fn handle_chat(body: String) -> String {
let raw_response: String = llm_call_system(model, full_system, message)
// Issue #5: also catch empty string llm_extract_text() in el_runtime.c silently
// returns "" when the response content array is missing or all blocks fail to parse.
// Without this guard an empty reply passes through as a silent empty response.
let is_error: Bool = str_starts_with(raw_response, "{\"error\"")
|| str_starts_with(raw_response, "{\"type\":\"error\"")
|| str_contains(raw_response, "authentication_error")
|| str_eq(raw_response, "")
if is_error {
return "{\"error\":\"llm unavailable\",\"response\":\"\"}"
}
@@ -447,6 +451,42 @@ fn studio_tools_json() -> String {
"]"
}
// ---------------------------------------------------------------------------
// LLM reliability issues that require C runtime fixes (el_runtime.c).
// These cannot be addressed at the EL layer; they are documented here so the
// symptoms are traceable back to their root causes.
//
// Issue #1 (no retry on timeout/connection error):
// http_do() in el_runtime.c calls curl_easy_perform() once. On
// CURLE_OPERATION_TIMEDOUT / CURLE_COULDNT_CONNECT / CURLE_RECV_ERROR it
// returns http_error_json() with no retry. Fix: add a retry loop (max 3
// attempts, exponential back-off starting at 1s) inside llm_provider_request().
//
// Issue #2 (60s timeout applies to all HTTP calls including LLM):
// EL_HTTP_TIMEOUT_MS defaults to 60000ms for every http_do() call.
// Fix: introduce EL_LLM_TIMEOUT_MS (default 120000) used only by
// llm_provider_request(); leave EL_HTTP_TIMEOUT_MS (default 30000) for
// general service calls to avoid holding connections for 60s.
//
// Issue #3 (HTTP 429 causes silent provider failover, not backoff):
// llm_chain_call() advances to the next provider on any JSON-prefixed response
// including 429. Fix: parse HTTP status via curl_easy_getinfo; on 429 sleep
// Retry-After seconds (default 5s) then retry the same provider up to 3 times.
//
// Issue #4 (HTTP 500/502 crashes the request silently):
// Same path as #3 5xx responses cause immediate provider failover with no
// retry. Fix: retry with exponential back-off (1s, 2s, 4s) before advancing.
//
// Issue #6 (no secondary LLM fallback in production):
// Set NEURON_LLM_1_URL/KEY/FORMAT in ExternalSecret to a secondary provider
// (e.g. Gemini). No C code change required; llm_chain_call() already iterates.
//
// Issue #8 (LLM response size unbounded memory-only cap):
// HttpBuf grows via realloc() with no hard limit. Fix: add
// EL_HTTP_MAX_RESPONSE_BYTES (default 10MiB) cap in httpbuf_append() and
// return http_error_json("response too large") on overflow.
// ---------------------------------------------------------------------------
fn agentic_api_key() -> String {
let k1: String = env("ANTHROPIC_API_KEY")
if !str_eq(k1, "") {
@@ -498,7 +538,7 @@ fn agentic_tools_with_web() -> String {
// Short timeout + empty-array fallback: if the bridge is down, the soul runs
// exactly as before with only its built-in tools (graceful degradation).
fn connector_tools_json() -> String {
let raw: String = exec_capture("curl -s --max-time 2 http://127.0.0.1:7771/mcp/tools")
let raw: String = exec_capture("curl -s --max-time 5 http://127.0.0.1:7771/mcp/tools")
if str_eq(raw, "") {
return "[]"
}
@@ -543,7 +583,7 @@ fn tool_auto_approved(tool_name: String) -> Bool {
if !str_starts_with(tool_name, "mcp__") {
return false
}
let raw: String = exec_capture("curl -s --max-time 2 http://127.0.0.1:7771/mcp/auto-approved")
let raw: String = exec_capture("curl -s --max-time 5 http://127.0.0.1:7771/mcp/auto-approved")
if str_eq(raw, "") {
return false
}
@@ -913,6 +953,14 @@ fn agentic_loop(session_id: String, model: String, safe_sys: String, tools_json:
let iteration: Int = 0
let keep_going: Bool = true
// Issue #9: agentic max_tokens configurable via NEURON_LLM_MAX_TOKENS env var.
// Default 4096 is marginal for long tool chains (8 iterations x 4096 tokens).
// Set to 8192+ for complex multi-step tasks.
// Note: llm_provider_request() in el_runtime.c also hardcodes 4096 for the
// llm_call_system() (non-agentic) path; that requires a C runtime change.
let max_tokens_env: String = env("NEURON_LLM_MAX_TOKENS")
let max_tokens_str: String = if str_eq(max_tokens_env, "") { "4096" } else { max_tokens_env }
// Suspension state captured at top level so it escapes the while body.
let pending: Bool = false
let pend_tool_id: String = ""
@@ -921,7 +969,7 @@ fn agentic_loop(session_id: String, model: String, safe_sys: String, tools_json:
while keep_going && iteration < 8 {
let req_body: String = "{\"model\":\"" + model + "\""
+ ",\"max_tokens\":4096"
+ ",\"max_tokens\":" + max_tokens_str
+ ",\"system\":\"" + safe_sys + "\""
+ ",\"tools\":" + tools_json
+ ",\"messages\":" + messages
@@ -1201,9 +1249,11 @@ fn handle_chat_as_soul(body: String) -> String {
let raw_response: String = llm_call_system(model, system_prompt, eff_message)
// Issue #5: empty string catch same rationale as handle_chat.
let is_error: Bool = str_starts_with(raw_response, "{\"error\"")
|| str_starts_with(raw_response, "{\"type\":\"error\"")
|| str_contains(raw_response, "authentication_error")
|| str_eq(raw_response, "")
if is_error {
return "{\"error\":\"llm unavailable\",\"response\":\"\",\"speaker_slug\":\"" + speaker + "\",\"model\":\"" + model + "\"}"
}
@@ -1250,9 +1300,11 @@ fn handle_dharma_room_turn(body: String) -> String {
let raw_response: String = llm_call_system(model, system_prompt, transcript)
// Issue #5: empty string catch same rationale as handle_chat.
let is_error: Bool = str_starts_with(raw_response, "{\"error\"")
|| str_starts_with(raw_response, "{\"type\":\"error\"")
|| str_contains(raw_response, "authentication_error")
|| str_eq(raw_response, "")
if is_error {
return "{\"error\":\"llm unavailable\",\"response\":\"\",\"cgi_id\":\"" + cgi_id + "\"}"
}
+18 -7
View File
@@ -144,7 +144,8 @@ fn safety_screen(input: String, history: String) -> String {
if score >= soft {
let summary: String = str_slice(input, 0, 80)
let discard: String = safety_log_bell("soft", "wellbeing check needed", summary)
// ISSUE 7: also escape tab chars to prevent JSON envelope corruption.
// ISSUE 7 fix: escape tab chars in addition to backslash/quote/newline/CR.
// A tab in user input corrupts the JSON envelope and causes json_get to misparse.
let e1: String = str_replace(input, "\\", "\\\\")
let e2: String = str_replace(e1, "\"", "\\\"")
let e3: String = str_replace(e2, "\n", "\\n")
@@ -153,7 +154,7 @@ fn safety_screen(input: String, history: String) -> String {
return "{\"action\":\"soft_bell\",\"reason\":\"wellbeing check needed\",\"content\":\"" + safe_input + "\"}"
}
// ISSUE 7: also escape tab chars (see soft_bell branch above).
// ISSUE 7 fix: escape tab chars (see soft_bell branch above for rationale).
let e1: String = str_replace(input, "\\", "\\\\")
let e2: String = str_replace(e1, "\"", "\\\"")
let e3: String = str_replace(e2, "\n", "\\n")
@@ -199,7 +200,10 @@ fn safety_validate(output: String, action: String) -> String {
fn safety_log_bell(level: String, reason: String, input_summary: String) -> String {
let content: String = "BELL:" + level + " | " + reason + " | summary:" + input_summary
let tags: String = "[\"safety\",\"bell\",\"bell:" + level + "\"]"
// ISSUE 2: fallback log when engram write fails silently.
// ISSUE 2 fix: if engram_node_full returns empty the write silently failed.
// Emit a fallback println so the bell event leaves at least a log trace even
// when engram is degraded. This does not replace engram persistence -- it is a
// last-resort audit trail when the primary write cannot be confirmed.
let node_id: String = engram_node_full(
content,
"BellEvent",
@@ -211,7 +215,7 @@ fn safety_log_bell(level: String, reason: String, input_summary: String) -> Stri
tags
)
if str_eq(node_id, "") {
println("[safety] WARN: bell engram write failed -- " + content)
println("[safety] WARN: bell event engram write failed -- fallback log: " + content)
}
return ""
}
@@ -244,9 +248,16 @@ fn safety_soft_phrases() -> String {
}
// ISSUE 5 TODO: phrase lists are rebuilt from JSON literals on every call.
// json_array_len of malformed input returns 0, silently skipping all checks.
// Caching requires language-level static const arrays -- not in current EL.
// Migrate to const arrays when EL gains that feature.
// safety_any_match and safety_count_match loop over json_array_get on every invocation.
// A compiled/cached representation would reduce per-message overhead and also guard against
// malformed phrase JSON (json_array_len of malformed input returns 0, silently skipping all checks).
// Caching requires language-level static const arrays -- not available in current EL.
// When EL gains module-level const arrays, migrate phrase lists to that form.
//
// ISSUE 5 TODO: phrase lists are rebuilt from JSON literals on every call to
// safety_any_match / safety_count_match. json_array_len of a malformed string
// returns 0, silently skipping all checks. Caching requires language-level static
// const arrays (not available in current EL). Migrate when EL gains that feature.
// Matching helpers (single loops only el escapes while-body mutation via
// top-level let rebinds; nested loops would not advance) ────────────────────
+12 -11
View File
@@ -261,8 +261,9 @@ fn layered_cycle(raw_input: String) -> String {
let screen_result: String = safety_screen(raw_input, history)
let screen_action: String = json_get(screen_result, "action")
// ISSUE 4: safe-mode guard. If safety_screen returned an invalid/empty action
// (engram failure or internal error), refuse rather than pass unscreened input.
// ISSUE 4: safe-mode guard -- if safety_screen returned invalid/empty action,
// refuse the turn rather than silently passing unscreened input to upper layers.
// Valid actions: "hard_bell", "soft_bell", "pass". Anything else = corrupt envelope.
let valid_action: Bool = str_eq(screen_action, "hard_bell")
|| str_eq(screen_action, "soft_bell")
|| str_eq(screen_action, "pass")
@@ -277,8 +278,8 @@ fn layered_cycle(raw_input: String) -> String {
// history where they could leak context to subsequent turns. They are persisted
// separately by safety_log_bell() into the Episodic tier with restricted labels.
//
// ISSUE 6: safety_log_bell already called inside safety_screen (line 140).
// Do NOT call it again here -- that would double-log every hard bell.
// ISSUE 6: safety_log_bell for hard bells is already called INSIDE safety_screen
// (safety.el line 140). Do NOT call it again here -- double-log avoided.
//
// safety_validate second param: when screen_action is "hard_bell", safety_validate
// receives the sentinel string "hard_bell" (not a normal screen action). The safety
@@ -320,13 +321,13 @@ fn layered_cycle(raw_input: String) -> String {
json_get(steward_result, "redirect_to")
}
// ISSUE 1: pre-LLM bell augmentation for layered_cycle path.
// safety_augment_system appends soft/hard directive to system prompt when bell fires,
// ensuring LLM processes message WITH the safety directive -- not just post-output gate.
// Stored in state as "layered_cycle_safety_system_addendum" for imprint_respond to use.
// 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.
// ISSUE 1: apply pre-LLM bell augmentation on layered_cycle path.
// safety_augment_system injects soft/hard directive into system prompt before LLM call.
// Stored in state so imprint_respond can consume it.
// TODO: wire directly into imprint_respond when it accepts a system_override param.
// ISSUE 3 TODO: no semantic/embedding crisis detection. Keyword-only means signals
// evading the phrase list pass through with zero augmentation. Semantic layer is a
// separate architectural decision requiring embedding inference on every message.
let augmented_addendum: String = safety_augment_system("", raw_input)
state_set("layered_cycle_safety_system_addendum", augmented_addendum)