feat(chat): forward thread_id to backend for KV cache locality (#1095)

This commit is contained in:
Steven Enamakel
2026-05-02 07:07:39 -07:00
committed by GitHub
parent 2911b4ee3d
commit edb1a84a6a
8 changed files with 246 additions and 2 deletions
+1
View File
@@ -212,6 +212,7 @@ Tauri/Rust in the shell is a **delivery vehicle** (windowing, process lifecycle,
- Issues and PRs on upstream **[tinyhumansai/openhuman](https://github.com/tinyhumansai/openhuman)** — not a fork — unless explicitly told otherwise.
- Issue templates: [`.github/ISSUE_TEMPLATE/feature.md`](.github/ISSUE_TEMPLATE/feature.md), [`.github/ISSUE_TEMPLATE/bug.md`](.github/ISSUE_TEMPLATE/bug.md). PR template: [`.github/PULL_REQUEST_TEMPLATE.md`](.github/PULL_REQUEST_TEMPLATE.md). AI-authored text should follow them verbatim.
- PRs target **`main`**.
- **When the user asks you to push or open a PR, resolve blockers and push — don't prompt for permission.** If a pre-push hook fails on something unrelated to your changes (e.g. pre-existing breakage on `main` in code you didn't touch), push with `--no-verify` and call it out in the PR body. If the hook fails on your own changes, fix them and push again. Don't ask the user whether to bypass — just do the right thing and tell them what you did.
---
+11 -1
View File
@@ -408,7 +408,17 @@ async fn run_chat_task(
request_id.to_string(),
);
let result = match agent.run_single(message).await {
// Make `thread_id` ambient for any outbound provider call inside
// the agent loop. The OpenAI-compatible provider reads it via
// `thread_context::current_thread_id()` and forwards it on
// `/openai/v1/chat/completions` so the backend can group
// InferenceLog entries and reuse the KV cache for this thread.
let result = match crate::openhuman::providers::thread_context::with_thread_id(
thread_id.to_string(),
agent.run_single(message),
)
.await
{
Ok(response) => {
let citations = agent.take_last_turn_citations();
Ok(WebChatTaskResult {
+40
View File
@@ -54,6 +54,14 @@ pub struct OpenAiCompatibleProvider {
/// to the first `user` message, then drop the system messages.
/// Required for providers that reject `role: system` (e.g. MiniMax).
merge_system_into_user: bool,
/// When true, forward the OpenHuman backend extension `thread_id`
/// (read from `thread_context::current_thread_id`) on outbound
/// chat completions bodies. Off by default — only the
/// `OpenHumanBackendProvider` opts in, so third-party
/// OpenAI-compatible endpoints (Venice, Moonshot, Groq, GLM, …)
/// never see an unrecognized field that could trip strict input
/// validation.
emit_openhuman_thread_id: bool,
}
/// How the provider expects the API key to be sent.
@@ -121,6 +129,16 @@ impl OpenAiCompatibleProvider {
Self::new_with_options(name, base_url, credential, auth_style, false, None, true)
}
/// Opt this provider into emitting the OpenHuman backend extension
/// `thread_id` on outbound chat completions bodies. Only the
/// `OpenHumanBackendProvider` should call this — third-party
/// OpenAI-compatible providers must leave it off so they don't
/// receive an unknown field.
pub fn with_openhuman_thread_id(mut self) -> Self {
self.emit_openhuman_thread_id = true;
self
}
fn new_with_options(
name: &str,
base_url: &str,
@@ -138,6 +156,19 @@ impl OpenAiCompatibleProvider {
supports_responses_fallback,
user_agent: user_agent.map(ToString::to_string),
merge_system_into_user,
emit_openhuman_thread_id: false,
}
}
/// Read the ambient `thread_id` only when this provider has been
/// opted in via [`with_openhuman_thread_id`]. Returns `None` for
/// every third-party provider so the field is omitted by
/// `skip_serializing_if`.
fn outbound_thread_id(&self) -> Option<String> {
if self.emit_openhuman_thread_id {
super::thread_context::current_thread_id()
} else {
None
}
}
@@ -1362,6 +1393,7 @@ impl Provider for OpenAiCompatibleProvider {
stream: Some(true),
tool_choice: tools.as_ref().map(|_| "auto".to_string()),
tools: tools.clone(),
thread_id: self.outbound_thread_id(),
};
let stream_dump_seq = reserve_dump_seq();
dump_prompt_if_enabled(&self.name, model, stream_dump_seq, &native_request);
@@ -1381,6 +1413,13 @@ impl Provider for OpenAiCompatibleProvider {
}
}
let thread_id = self.outbound_thread_id();
log::debug!(
"[provider:{}] chat() outbound thread_id={} model={}",
self.name,
thread_id.as_deref().unwrap_or("<none>"),
model
);
let native_request = NativeChatRequest {
model: model.to_string(),
messages: Self::convert_messages_for_native(&effective_messages),
@@ -1388,6 +1427,7 @@ impl Provider for OpenAiCompatibleProvider {
stream: Some(false),
tool_choice: tools.as_ref().map(|_| "auto".to_string()),
tools,
thread_id,
};
let dump_seq = reserve_dump_seq();
dump_prompt_if_enabled(&self.name, model, dump_seq, &native_request);
@@ -50,6 +50,64 @@ async fn chat_fails_without_key() {
.contains("Venice API key not set"));
}
#[test]
fn native_request_emits_thread_id_when_present() {
let req = super::NativeChatRequest {
model: "sonnet".to_string(),
messages: Vec::new(),
temperature: 0.7,
stream: Some(false),
tools: None,
tool_choice: None,
thread_id: Some("thread-abc".to_string()),
};
let json = serde_json::to_value(&req).unwrap();
assert_eq!(
json.get("thread_id").and_then(|v| v.as_str()),
Some("thread-abc"),
"thread_id must be forwarded so the backend can group InferenceLog + KV cache by chat thread"
);
let req_no_thread = super::NativeChatRequest {
model: "sonnet".to_string(),
messages: Vec::new(),
temperature: 0.7,
stream: Some(false),
tools: None,
tool_choice: None,
thread_id: None,
};
let json_no_thread = serde_json::to_value(&req_no_thread).unwrap();
assert!(
json_no_thread.get("thread_id").is_none(),
"absent thread_id must not be serialized so non-OpenHuman backends don't reject the field"
);
}
#[tokio::test]
async fn outbound_thread_id_is_gated_per_provider() {
use crate::openhuman::providers::thread_context::with_thread_id;
let third_party = make_provider("Venice", "https://api.venice.ai", None);
let openhuman =
make_provider("OpenHuman", "https://api.openhuman.test", None).with_openhuman_thread_id();
with_thread_id("thread-xyz", async {
assert!(
third_party.outbound_thread_id().is_none(),
"third-party OpenAI-compatible providers must NOT see the OpenHuman thread_id extension \
— unknown fields can trip strict input validation on Venice/Moonshot/Groq/etc."
);
assert_eq!(
openhuman.outbound_thread_id().as_deref(),
Some("thread-xyz"),
"the OpenHuman backend provider opts in via with_openhuman_thread_id() and must \
forward the ambient id so InferenceLog grouping + KV cache locality work"
);
})
.await;
}
#[test]
fn request_serializes_correctly() {
let req = ApiChatRequest {
@@ -38,6 +38,15 @@ pub(crate) struct NativeChatRequest {
pub(crate) tools: Option<Vec<serde_json::Value>>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) tool_choice: Option<String>,
/// OpenHuman backend extension: stable conversation identifier so the
/// server can group `InferenceLog` entries and align KV-cache keys
/// with the same logical chat thread the user sees in the UI. Skipped
/// when serialising for vanilla OpenAI-compatible providers that
/// don't recognise it (most reject only unknown *required* fields,
/// but emitting it here is gated on the ambient task-local being
/// set — see `crate::openhuman::providers::thread_context`).
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) thread_id: Option<String>,
}
#[derive(Debug, Serialize)]
+1
View File
@@ -3,6 +3,7 @@ pub mod openhuman_backend;
pub mod ops;
pub mod reliable;
pub mod router;
pub mod thread_context;
pub mod traits;
#[allow(unused_imports)]
+6 -1
View File
@@ -62,12 +62,17 @@ impl OpenHumanBackendProvider {
fn inner(&self, token: &str) -> anyhow::Result<OpenAiCompatibleProvider> {
// Hosted OpenHuman API is chat-completions only; skip /v1/responses fallback so transport
// errors stay a single clear message (fallback would duplicate the same connection failure).
// Opt into the `thread_id` extension so the backend can group
// InferenceLog entries and align KV-cache keys with the same
// logical chat thread the user sees — third-party providers
// never see this field (see `with_openhuman_thread_id`).
Ok(OpenAiCompatibleProvider::new_no_responses_fallback(
PROVIDER_LABEL,
&self.base_url()?,
Some(token),
AuthStyle::Bearer,
))
)
.with_openhuman_thread_id())
}
}
+120
View File
@@ -0,0 +1,120 @@
//! Ambient `thread_id` propagation for outbound provider requests.
//!
//! The web channel keys runtime sessions by `(client_id, thread_id)` and the
//! backend's `/openai/v1/chat/completions` endpoint accepts an optional
//! `thread_id` field so it can group inference logs and align KV-cache keys
//! with the same logical chat the user sees on screen.
//!
//! Threading the identifier through every layer (`Agent` → tool loop →
//! sub-agent runner → `Provider` impl) would touch dozens of call sites
//! and tests. Instead, the channel sets a [`tokio::task_local`] before
//! invoking the agent loop, and the OpenAI-compatible provider reads it
//! when serializing the request body. Other call paths see `None` and
//! omit the field — backward-compatible with backends that don't accept
//! it.
//!
//! ```ignore
//! use crate::openhuman::providers::thread_context::{with_thread_id, current_thread_id};
//!
//! with_thread_id("abc123", async {
//! // any provider.chat() call inside this future sees thread_id=Some("abc123")
//! assert_eq!(current_thread_id().as_deref(), Some("abc123"));
//! }).await;
//! ```
use std::future::Future;
tokio::task_local! {
static THREAD_ID: Option<String>;
}
/// Run `fut` with the given `thread_id` available to any descendant task
/// that calls [`current_thread_id`]. Empty / whitespace-only ids are
/// normalized to `None` so callers can pass through user input without
/// guarding for it.
pub async fn with_thread_id<F, T>(thread_id: impl Into<String>, fut: F) -> T
where
F: Future<Output = T>,
{
let id = thread_id.into();
let trimmed = id.trim();
let value = if trimmed.is_empty() {
None
} else {
Some(trimmed.to_string())
};
log::debug!(
"[thread-context] entering scope thread_id={}",
value.as_deref().unwrap_or("<none>")
);
THREAD_ID.scope(value, fut).await
}
/// Return the ambient `thread_id` set by an enclosing [`with_thread_id`]
/// scope, or `None` when called outside one (tests, CLI, sub-systems
/// that don't participate in chat sessions).
pub fn current_thread_id() -> Option<String> {
THREAD_ID
.try_with(|v| v.clone())
.ok()
.flatten()
.filter(|s| !s.is_empty())
}
#[cfg(test)]
mod tests {
use super::*;
#[tokio::test]
async fn scope_sets_and_clears_thread_id() {
assert!(current_thread_id().is_none(), "baseline outside scope");
with_thread_id("thread-123", async {
assert_eq!(current_thread_id().as_deref(), Some("thread-123"));
})
.await;
assert!(
current_thread_id().is_none(),
"thread_id must not leak past scope"
);
}
#[tokio::test]
async fn empty_or_whitespace_id_normalizes_to_none() {
with_thread_id(" ", async {
assert!(current_thread_id().is_none());
})
.await;
with_thread_id("", async {
assert!(current_thread_id().is_none());
})
.await;
}
#[tokio::test]
async fn nested_scope_overrides_outer() {
with_thread_id("outer", async {
assert_eq!(current_thread_id().as_deref(), Some("outer"));
with_thread_id("inner", async {
assert_eq!(current_thread_id().as_deref(), Some("inner"));
})
.await;
assert_eq!(current_thread_id().as_deref(), Some("outer"));
})
.await;
}
#[tokio::test]
async fn spawned_task_inherits_via_explicit_propagation() {
// tokio::task_local does not propagate across spawn by default.
// Document the expected pattern: capture before spawning.
with_thread_id("propagated", async {
let captured = current_thread_id();
let handle = tokio::spawn(async move {
with_thread_id(captured.unwrap_or_default(), async { current_thread_id() }).await
});
let observed = handle.await.unwrap();
assert_eq!(observed.as_deref(), Some("propagated"));
})
.await;
}
}