fix(recall): address all five code-review issues in context-dedup
Issue 1 — cache read-before-write: move engram_compile_multi call to
before the affective_prefix block in handle_chat. engram_compile writes
"engram_compile_bell_node" to state; the previous ordering meant the
first-turn affective prefix always read an empty cache even when a recent
bell node existed.
Issue 2 — double-write clobber: engram_compile_multi now saves the
primary-seed activation ("engram_compile_primary_activation_json") after
the first engram_compile call, before the secondary call can overwrite
the shared "engram_compile_activation_json" key. strengthen_chat_nodes
now prefers the primary key, falling back only when absent.
Issue 3 — mid-object truncation in engram_compile_multi: replace the
dumb str_slice(merged, 0, 6000) with the same safe JSON boundary-scan
(last closing brace before cap) already used in engram_compile, so
ctx1+ctx2+ctx3 over 6000 chars never produces a torn JSON object.
Issue 4 — heuristic regression in is_genuine_continuation: add explicit
question-word prefix detection (what/how/why/when/where/who/which/is/
can/could/does/do/explain/describe/define) that fires before the 50-char
length gate. A message starting with a question word is always a new
topic, regardless of length, so "what is rust?" (14 chars, all-lowercase,
no mid-capitals) correctly returns false instead of true.
Issue 5 — unreliable dedup via str_contains: remove the substring
duplicate checks in engram_compile_multi. str_contains across multi-KB
JSON strings is not a reliable deduplication mechanism — coincidental
field-value matches suppress valid context, and truncated ctx1 misses
genuine duplicates. We now concatenate ctx1+ctx2+ctx3 unconditionally
and accept minor node redundancy in exchange for correctness.
This commit is contained in:
@@ -139,14 +139,40 @@ fn is_followup_phrase(msg: String) -> Bool {
|
||||
}
|
||||
|
||||
// is_genuine_continuation — returns true when a short message is a contextual
|
||||
// follow-up rather than a new topic. Fixes Issue 2: the old threshold (str_len < 50)
|
||||
// conflated new-topic short messages like "explain quantum tunneling" (49 chars)
|
||||
// with genuine follow-ups like "ok", "yes", or "what do you think?".
|
||||
// follow-up rather than a new topic.
|
||||
// Issue 4 fix: the prior heuristic only checked for mid-string capitals, which
|
||||
// fails for all-lowercase new-topic queries like "what is rust?" (14 chars) or
|
||||
// "explain quantum computing" (26 chars). Added question-word prefix detection
|
||||
// that fires BEFORE the length check: any message starting with a question word
|
||||
// (what/how/why/when/where/who/which/is/can/could/does/do) introduces a new
|
||||
// topic and is never a continuation, regardless of length.
|
||||
fn is_genuine_continuation(msg: String, hist_len: Int) -> Bool {
|
||||
if hist_len == 0 { return false }
|
||||
if str_len(msg) == 0 { return false }
|
||||
if is_followup_phrase(msg) { return true }
|
||||
// Question-word prefix: messages starting with these introduce new topics.
|
||||
// Check before the length heuristic so short new-topic questions escape.
|
||||
let is_question_start: Bool = str_starts_with(msg, "what ")
|
||||
|| str_starts_with(msg, "What ")
|
||||
|| str_starts_with(msg, "how ") || str_starts_with(msg, "How ")
|
||||
|| str_starts_with(msg, "why ") || str_starts_with(msg, "Why ")
|
||||
|| str_starts_with(msg, "when ") || str_starts_with(msg, "When ")
|
||||
|| str_starts_with(msg, "where ") || str_starts_with(msg, "Where ")
|
||||
|| str_starts_with(msg, "who ") || str_starts_with(msg, "Who ")
|
||||
|| str_starts_with(msg, "which ") || str_starts_with(msg, "Which ")
|
||||
|| str_starts_with(msg, "is ") || str_starts_with(msg, "Is ")
|
||||
|| str_starts_with(msg, "can ") || str_starts_with(msg, "Can ")
|
||||
|| str_starts_with(msg, "could ") || str_starts_with(msg, "Could ")
|
||||
|| str_starts_with(msg, "does ") || str_starts_with(msg, "Does ")
|
||||
|| str_starts_with(msg, "do ") || str_starts_with(msg, "Do ")
|
||||
|| str_starts_with(msg, "explain ") || str_starts_with(msg, "Explain ")
|
||||
|| str_starts_with(msg, "describe ") || str_starts_with(msg, "Describe ")
|
||||
|| str_starts_with(msg, "define ") || str_starts_with(msg, "Define ")
|
||||
if is_question_start { return false }
|
||||
// Long messages (50+ chars) typically introduce new topics.
|
||||
if str_len(msg) >= 50 { return false }
|
||||
// Short messages with a mid-string capital are likely named-concept queries
|
||||
// (e.g. "tell me about Rust", "what about AWS") — treat as new topic.
|
||||
let rest: String = str_slice(msg, 1, str_len(msg))
|
||||
let has_mid_capital: Bool = false
|
||||
let has_mid_capital = has_mid_capital || str_contains(rest, " A")
|
||||
@@ -319,9 +345,34 @@ fn build_activation_seed(message: String, hist: String, hist_len: Int) -> String
|
||||
|
||||
// engram_compile_multi — fan-out activation across multiple query seeds. Fixes Issue 4:
|
||||
// only a single seed was tried per turn, with no entity/emotion/topic diversification.
|
||||
//
|
||||
// Issue 2 fix: save the primary-seed activation to a dedicated state key BEFORE calling
|
||||
// engram_compile(message). Each engram_compile call overwrites "engram_compile_activation_json"
|
||||
// with its own activation result. Without this save, the secondary compile (bare message,
|
||||
// lower signal) clobbers the primary (enriched seed, higher signal), and strengthen_chat_nodes
|
||||
// later reads the lower-signal result for node strengthening.
|
||||
//
|
||||
// Issue 3 fix: replace the dumb str_slice(merged, 0, 6000) truncation with the same
|
||||
// safe JSON boundary-scan used in engram_compile. The old truncation could cut mid-object
|
||||
// when ctx1+ctx2+ctx3 together exceeded 6000 chars, producing malformed JSON context.
|
||||
//
|
||||
// Issue 5 fix: remove str_contains(ctx1, ctx2) / str_contains(merged, ctx3) substring
|
||||
// duplicate checks. These compared multi-KB JSON strings and were unreliable in both
|
||||
// directions: a coincidental substring match inside a JSON field value could falsely suppress
|
||||
// ctx2 entirely; a genuinely duplicate ctx2 was missed when ctx1 was already truncated.
|
||||
// We now concatenate unconditionally and let engram_compile's own dedup (node-ID based)
|
||||
// handle within-result duplicates. Slight redundancy across ctx1/ctx2 is acceptable; false
|
||||
// suppression of valid context is not.
|
||||
fn engram_compile_multi(primary_seed: String, message: String) -> String {
|
||||
let ctx1: String = engram_compile(primary_seed)
|
||||
|
||||
// Issue 2 fix: save the primary-seed activation before any secondary compile can
|
||||
// overwrite the shared "engram_compile_activation_json" state key.
|
||||
let primary_act: String = state_get("engram_compile_activation_json")
|
||||
if !str_eq(primary_act, "") && !str_eq(primary_act, "[]") {
|
||||
state_set("engram_compile_primary_activation_json", primary_act)
|
||||
}
|
||||
|
||||
let entity_seed_differs: Bool = !str_eq(primary_seed, message)
|
||||
let ctx2: String = if entity_seed_differs {
|
||||
let raw_ctx: String = engram_compile(message)
|
||||
@@ -335,19 +386,29 @@ fn engram_compile_multi(primary_seed: String, message: String) -> String {
|
||||
if emo_ok { engram_compile_ranked(emo_results, 3) } else { "" }
|
||||
} else { "" }
|
||||
|
||||
let merged: String = ctx1
|
||||
let sep2: String = if !str_eq(merged, "") && !str_eq(ctx2, "") { "\n" } else { "" }
|
||||
let merged = if !str_eq(ctx2, "") && !str_contains(ctx1, ctx2) {
|
||||
merged + sep2 + ctx2
|
||||
} else { merged }
|
||||
// Issue 5 fix: concatenate unconditionally — no str_contains substring dedup.
|
||||
let sep2: String = if !str_eq(ctx1, "") && !str_eq(ctx2, "") { "\n" } else { "" }
|
||||
let merged: String = ctx1 + sep2 + ctx2
|
||||
let sep3: String = if !str_eq(merged, "") && !str_eq(ctx3, "") { "\n" } else { "" }
|
||||
let merged = if !str_eq(ctx3, "") && !str_contains(merged, ctx3) {
|
||||
merged + sep3 + ctx3
|
||||
} else { merged }
|
||||
let merged = if !str_eq(ctx3, "") { merged + sep3 + ctx3 } else { merged }
|
||||
|
||||
if str_eq(merged, "") { return "" }
|
||||
if str_len(merged) > 6000 { return str_slice(merged, 0, 6000) }
|
||||
return merged
|
||||
|
||||
// Issue 3 fix: safe JSON boundary-scan truncation — find the last closing brace
|
||||
// before the 6000-char cap rather than slicing mid-object.
|
||||
let cap_len: Int = 6000
|
||||
if str_len(merged) <= cap_len { return merged }
|
||||
let cap_search: Int = cap_len - 1
|
||||
let cap_min: Int = if cap_len > 500 { cap_len - 500 } else { 0 }
|
||||
let cap_pos: Int = -1
|
||||
let cap_si: Int = cap_search
|
||||
while cap_si >= cap_min && cap_pos < 0 {
|
||||
let cap_ch: String = str_slice(merged, cap_si, cap_si + 1)
|
||||
let cap_pos = if str_eq(cap_ch, "}") { cap_si } else { cap_pos }
|
||||
let cap_si = if cap_pos < 0 { cap_si - 1 } else { cap_si }
|
||||
}
|
||||
if cap_pos > 0 { return str_slice(merged, 0, cap_pos + 1) }
|
||||
return str_slice(merged, 0, cap_len)
|
||||
}
|
||||
|
||||
fn engram_compile(intent: String) -> String {
|
||||
@@ -636,7 +697,15 @@ fn handle_chat(body: String) -> String {
|
||||
// and tail-biased snipping from long assistant replies.
|
||||
let activation_seed: String = build_activation_seed(message, stored_hist, hist_len)
|
||||
|
||||
// Issue 1 fix: call engram_compile_multi BEFORE reading the bell-node cache.
|
||||
// engram_compile (called inside engram_compile_multi) writes "engram_compile_bell_node"
|
||||
// at line 426. Reading the cache before the compile call means the first session turn
|
||||
// always sees an empty cache — the very turn where safety continuity matters most.
|
||||
// Moving compile first ensures the cache is populated before affective_prefix reads it.
|
||||
let ctx: String = engram_compile_multi(activation_seed, message)
|
||||
|
||||
// Fix Issue 2: reuse cached bell result from engram_compile — no second engram query.
|
||||
// Now runs AFTER engram_compile_multi so the cache is guaranteed to be warm.
|
||||
let affective_prefix: String = if hist_len == 0 {
|
||||
let cached_bell: String = state_get("engram_compile_bell_node")
|
||||
if !str_eq(cached_bell, "") {
|
||||
@@ -644,8 +713,6 @@ fn handle_chat(body: String) -> String {
|
||||
} else { "" }
|
||||
} else { "" }
|
||||
|
||||
// Issue 4 fix: engram_compile_multi adds entity + emotion fan-out seeds
|
||||
let ctx: String = engram_compile_multi(activation_seed, message)
|
||||
let system: String = affective_prefix + build_system_prompt(ctx)
|
||||
|
||||
// First message of the session: proactively load user profile and active work context.
|
||||
@@ -755,7 +822,16 @@ fn handle_chat(body: String) -> String {
|
||||
conv_history_persist(final_hist)
|
||||
|
||||
// Fix Issue 7: reuse activation JSON from engram_compile — no third activate query.
|
||||
let cached_act: String = state_get("engram_compile_activation_json")
|
||||
// Issue 2 fix: prefer the primary-seed activation (enriched seed, depth 5) saved
|
||||
// before the secondary compile could overwrite the shared state key. Fall back to
|
||||
// the final compile activation only when the primary key is absent (e.g. first boot
|
||||
// before any compile has run or when primary_seed == message and ctx2 was skipped).
|
||||
let primary_cached: String = state_get("engram_compile_primary_activation_json")
|
||||
let cached_act: String = if !str_eq(primary_cached, "") && !str_eq(primary_cached, "[]") {
|
||||
primary_cached
|
||||
} else {
|
||||
state_get("engram_compile_activation_json")
|
||||
}
|
||||
let act_out: String = if !str_eq(cached_act, "") && !str_eq(cached_act, "[]") {
|
||||
cached_act
|
||||
} else { "[]" }
|
||||
|
||||
Reference in New Issue
Block a user