Skip to content

feat(memory): schedule STMO jobs after conversation persistence (NoOp writer — follow-up for Oracle wiring) #1346

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
spalimpaaces-star wants to merge 5 commits into from
Closed

feat(memory): schedule STMO jobs after conversation persistence (NoOp writer — follow-up for Oracle wiring) #1346

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
eef0602
eliminate redundant header parse and thread STM config into MemoryExe…
Apr 23, 2026
690350e
feat(memory): thread conversation memory writer and execution context…
Apr 23, 2026
20ed351
feat(memory): add STMO scheduling logic into post-persist conversatio…
Apr 23, 2026
76bdb07
fix(memory): compute STMO turn count from assembled conversation history
Apr 23, 2026
1c910fe
fix(memory): gate stm_enabled by runtime memory switch
Apr 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions model_gateway/src/memory/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ pub struct MemoryExecutionContext {
pub subject_id: Option<String>,
pub embedding_model: Option<String>,
pub extraction_model: Option<String>,
/// Whether STM condensation was requested by the caller.
pub stm_enabled: bool,
/// Model to use for STM condensation, if specified by the caller.
pub stm_condenser_model_id: Option<String>,
}

impl MemoryExecutionContext {
Expand Down Expand Up @@ -82,6 +86,8 @@ impl MemoryExecutionContext {
subject_id: headers.subject_id.clone(),
embedding_model: headers.embedding_model.clone(),
extraction_model: headers.extraction_model.clone(),
stm_enabled: headers.stm_enabled && runtime.enabled,
stm_condenser_model_id: headers.stm_condenser_model_id.clone(),
}
Comment on lines +89 to 91

@coderabbitai coderabbitai Bot Apr 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Minor inconsistency: stm_condenser_model_id is not gated by runtime.enabled.

When headers.stm_enabled=true but runtime.enabled=false, the resulting MemoryExecutionContext has stm_enabled=false yet stm_condenser_model_id=Some(...). This is fine as long as every downstream consumer gates on stm_enabled (not on stm_condenser_model_id.is_some()). To remove the footgun entirely and keep the two STM fields consistent, consider clearing the model id when the gate trips:

♻️ Proposed diff 
-            stm_enabled: headers.stm_enabled && runtime.enabled,
-            stm_condenser_model_id: headers.stm_condenser_model_id.clone(),
+            stm_enabled: headers.stm_enabled && runtime.enabled,
+            stm_condenser_model_id: if headers.stm_enabled && runtime.enabled {
+                headers.stm_condenser_model_id.clone()
+            } else {
+                None
+            },

Adding a test asserting ctx.stm_condenser_model_id.is_none() when runtime is disabled but the header set a condenser would lock this in alongside the existing stm_enabled_gated_off_when_runtime_disabled test.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
stm_enabled: headers.stm_enabled && runtime.enabled,
stm_condenser_model_id: headers.stm_condenser_model_id.clone(),
}
stm_enabled: headers.stm_enabled && runtime.enabled,
stm_condenser_model_id: if headers.stm_enabled && runtime.enabled {
headers.stm_condenser_model_id.clone()
} else {
None
},
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@model_gateway/src/memory/context.rs` around lines 89 - 91, The
MemoryExecutionContext currently sets stm_condenser_model_id unconditionally
which can leave a model id present when stm_enabled is false; update the
constructor/initializer that builds MemoryExecutionContext so
stm_condenser_model_id is only assigned when both
headers.stm_condenser_model_id.is_some() and runtime.enabled are true (otherwise
set it to None) to keep it consistent with stm_enabled, and add a unit test
similar to stm_enabled_gated_off_when_runtime_disabled that asserts
ctx.stm_condenser_model_id.is_none() when runtime.enabled is false but the
header provided a condenser id; refer to MemoryExecutionContext, stm_enabled,
stm_condenser_model_id, headers and runtime to locate affected code.

}
}
Expand Down Expand Up @@ -183,4 +189,16 @@ mod tests {
);
assert_eq!(ctx.extraction_model.as_deref(), Some("gpt-4.1-mini"));
}

#[test]
fn stm_enabled_gated_off_when_runtime_disabled() {
let headers = MemoryHeaderView {
stm_enabled: true,
..MemoryHeaderView::default()
};

let ctx = MemoryExecutionContext::from_headers(&headers, &runtime(false));

assert!(!ctx.stm_enabled);
}
}
10 changes: 10 additions & 0 deletions model_gateway/src/routers/common/header_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ pub struct MemoryHeaderView {
pub subject_id: Option<String>,
pub embedding_model: Option<String>,
pub extraction_model: Option<String>,
/// Whether STM condensation was requested by the caller.
pub stm_enabled: bool,
/// Model to use for STM condensation, if specified by the caller.
pub stm_condenser_model_id: Option<String>,
}

impl MemoryHeaderView {
Expand Down Expand Up @@ -52,6 +56,12 @@ impl MemoryHeaderView {
extraction_model: ltm_enabled
.then_some(config.long_term_memory.extraction_model_id)
.flatten(),
stm_enabled: config.short_term_memory.enabled,
stm_condenser_model_id: config
.short_term_memory
.enabled
.then_some(config.short_term_memory.condenser_model_id)
.flatten(),
}
}
}
Expand Down
Loading
Loading