Skip to content

Commit ff47ae8

Browse files
author
scott
committed
fix: UTF-8 char boundary panic in repetition detector (floor_char_boundary)
The repetition detector splits the output buffer into two halves using byte arithmetic (len/2, len-600) which panics when the offset lands inside a multi-byte UTF-8 character (math symbols like ᵢ, ×, σ). This poisons the Mutex<LlamaContext> and every subsequent request returns 502 forever. Also included: sliding-window KV eviction to allow generation beyond the context window, char-trigram cosine repetition detection, model caching, and penalty forwarding (from prior work on this branch). Adds floor_char_boundary() — O(1) walk-back to nearest valid char boundary. Applied at the two slice sites in the repetition detection window splitting. Includes 7 unit tests. cargo fmt applied. Fixes #183 (residual from the from_utf8_lossy fix which addressed token-level UTF-8 but not the repetition detector string slicing). Signed-off-by: Scott Johnson <rsjohnnson@gmail.com> Signed-off-by: scott <you@example.com>
1 parent b98391e commit ff47ae8

3 files changed

Lines changed: 661 additions & 21 deletions

File tree

src/engine/adapter.rs

Lines changed: 82 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,21 @@
11
use anyhow::Result;
22
use async_trait::async_trait;
3+
use std::collections::HashMap;
4+
use std::path::PathBuf;
5+
use std::sync::Arc;
6+
use tokio::sync::RwLock;
37

48
use super::{GenOptions, InferenceEngine, LoadedModel, ModelSpec};
59

610
#[cfg(feature = "huggingface")]
711
use super::{UniversalEngine, UniversalModel, UniversalModelSpec};
812

9-
/// Universal adapter that bridges legacy and new engine interfaces
13+
/// Universal adapter that bridges legacy and new engine interfaces.
14+
///
15+
/// Caches loaded models by file path so that repeated requests for the same
16+
/// model reuse the existing llama.cpp context instead of reloading from disk.
17+
/// The KV cache is cleared at the start of each generate() call (in the
18+
/// llama engine) so cached contexts produce correct results.
1019
pub struct InferenceEngineAdapter {
1120
#[cfg(feature = "huggingface")]
1221
huggingface_engine: super::huggingface::HuggingFaceEngine,
@@ -15,7 +24,11 @@ pub struct InferenceEngineAdapter {
1524
#[cfg(feature = "mlx")]
1625
mlx_engine: super::mlx::MLXEngine,
1726
safetensors_engine: super::safetensors_native::SafeTensorsEngine,
18-
// Note: loaded_models removed as caching is not currently implemented
27+
/// Model cache: base_path → shared loaded model.
28+
/// Each cached model holds its native GGUF chat template (read once at
29+
/// load time) and a Mutex-guarded llama.cpp context for sequential
30+
/// inference.
31+
loaded_cache: Arc<RwLock<HashMap<PathBuf, Arc<dyn LoadedModel>>>>,
1932
}
2033

2134
impl Default for InferenceEngineAdapter {
@@ -34,6 +47,7 @@ impl InferenceEngineAdapter {
3447
#[cfg(feature = "mlx")]
3548
mlx_engine: super::mlx::MLXEngine::new(),
3649
safetensors_engine: super::safetensors_native::SafeTensorsEngine::new(),
50+
loaded_cache: Arc::new(RwLock::new(HashMap::new())),
3751
}
3852
}
3953

@@ -47,6 +61,7 @@ impl InferenceEngineAdapter {
4761
#[cfg(feature = "mlx")]
4862
mlx_engine: super::mlx::MLXEngine::new(),
4963
safetensors_engine: super::safetensors_native::SafeTensorsEngine::new(),
64+
loaded_cache: Arc::new(RwLock::new(HashMap::new())),
5065
}
5166
}
5267

@@ -193,26 +208,67 @@ enum BackendChoice {
193208
SafeTensors,
194209
}
195210

211+
/// Thin handle returned by `load()`. Each caller gets its own `Box` but
212+
/// all handles for the same model share a single `Arc<dyn LoadedModel>`,
213+
/// which holds the llama.cpp model, context (behind a Mutex), and the
214+
/// native GGUF chat template.
215+
struct CachedModelHandle {
216+
inner: Arc<dyn LoadedModel>,
217+
}
218+
219+
#[async_trait]
220+
impl LoadedModel for CachedModelHandle {
221+
async fn generate(
222+
&self,
223+
prompt: &str,
224+
opts: GenOptions,
225+
on_token: Option<Box<dyn FnMut(String) + Send>>,
226+
) -> Result<String> {
227+
self.inner.generate(prompt, opts, on_token).await
228+
}
229+
230+
async fn generate_vision(
231+
&self,
232+
image_data: &[u8],
233+
prompt: &str,
234+
opts: GenOptions,
235+
on_token: Option<Box<dyn FnMut(String) + Send>>,
236+
) -> Result<String> {
237+
self.inner
238+
.generate_vision(image_data, prompt, opts, on_token)
239+
.await
240+
}
241+
242+
fn format_prompt(&self, messages: &[(String, String)]) -> Option<String> {
243+
self.inner.format_prompt(messages)
244+
}
245+
}
246+
196247
#[async_trait]
197248
impl InferenceEngine for InferenceEngineAdapter {
198249
async fn load(&self, spec: &ModelSpec) -> Result<Box<dyn LoadedModel>> {
199-
// Select backend and load model directly (no caching for now to avoid complexity)
200-
let backend = self.select_backend(spec);
201-
match backend {
202-
BackendChoice::SafeTensors => {
203-
// Use native SafeTensors engine - NO Python dependency!
204-
self.safetensors_engine.load(spec).await
250+
// Fast path: return cached model if already loaded for this path.
251+
{
252+
let cache = self.loaded_cache.read().await;
253+
if let Some(cached) = cache.get(&spec.base_path) {
254+
tracing::debug!("model cache hit: {}", spec.base_path.display());
255+
return Ok(Box::new(CachedModelHandle {
256+
inner: Arc::clone(cached),
257+
}));
205258
}
259+
}
260+
261+
// Cache miss — load model from the appropriate backend.
262+
tracing::info!("model cache miss, loading: {}", spec.base_path.display());
263+
let backend = self.select_backend(spec);
264+
let loaded: Box<dyn LoadedModel> = match backend {
265+
BackendChoice::SafeTensors => self.safetensors_engine.load(spec).await?,
206266
#[cfg(feature = "mlx")]
207-
BackendChoice::MLX => {
208-
// Use MLX engine for Apple Silicon Metal GPU acceleration
209-
self.mlx_engine.load(spec).await
210-
}
267+
BackendChoice::MLX => self.mlx_engine.load(spec).await?,
211268
#[cfg(feature = "llama")]
212-
BackendChoice::Llama => self.llama_engine.load(spec).await,
269+
BackendChoice::Llama => self.llama_engine.load(spec).await?,
213270
#[cfg(feature = "huggingface")]
214271
BackendChoice::HuggingFace => {
215-
// Convert to UniversalModelSpec for huggingface backend (for HF model IDs)
216272
let universal_spec = UniversalModelSpec {
217273
name: spec.name.clone(),
218274
backend: super::ModelBackend::HuggingFace {
@@ -226,11 +282,21 @@ impl InferenceEngine for InferenceEngineAdapter {
226282
n_threads: spec.n_threads,
227283
};
228284
let universal_model = self.huggingface_engine.load(&universal_spec).await?;
229-
Ok(Box::new(UniversalModelWrapper {
285+
Box::new(UniversalModelWrapper {
230286
model: universal_model,
231-
}))
287+
})
232288
}
289+
};
290+
291+
// Convert Box<dyn LoadedModel> → Arc<dyn LoadedModel> and cache.
292+
// SAFETY: LoadedModel: Send + Sync is enforced by the trait definition.
293+
let arc: Arc<dyn LoadedModel> = Arc::from(loaded);
294+
{
295+
let mut cache = self.loaded_cache.write().await;
296+
cache.insert(spec.base_path.clone(), Arc::clone(&arc));
233297
}
298+
299+
Ok(Box::new(CachedModelHandle { inner: arc }))
234300
}
235301
}
236302

@@ -253,9 +319,6 @@ impl LoadedModel for UniversalModelWrapper {
253319
}
254320
}
255321

256-
// Note: Cached model references removed as they were unused placeholder code.
257-
// Future implementation should use Arc<dyn LoadedModel> for proper model sharing.
258-
259322
#[cfg(test)]
260323
mod tests {
261324
use super::*;

0 commit comments

Comments
 (0)