Bug fixes

crates/core/Cargo.toml

@@ -36,7 +36,7 @@ tracing-subscriber.workspace = true
 notify.workspace = true
 minijinja = { version = "2.18.0", features = ["builtins", "json"] }
 minijinja-contrib = { version = "2.18.0", features = ["pycompat"] }
-llama-cpp-2 = "=0.1.146"
+llama-cpp-2 = "=0.1.150"
 
 [dev-dependencies]
 tempfile = { workspace = true, features = [] }

crates/core/src/context.rs

@@ -428,9 +428,8 @@ impl Context {
                 .sum()
         };
 
-        self.messages = recent;
-        self.messages
-            .extend(summaries.into_iter().map(|m| (m, None)));
+        self.messages = summaries.into_iter().map(|m| (m, None)).collect();
+        self.messages.extend(recent);
 
         let compacted_count = self.messages.len();
 
@@ -772,6 +771,11 @@ mod compact_tests {
         noop_when_empty = { vec![], true },
         noop_when_under_threshold = { vec![msg_tuple(new_chat_msg(SenderType::User, "Hi"))], true },
         compact_when_over_threshold = { vec![msg_tuple(new_chat_msg(SenderType::User, &"X".repeat(10000)))], false },
+        compact_maintains_order = { vec![
+            msg_tuple(new_chat_msg(SenderType::User, &"X".repeat(11000))),
+            msg_tuple(new_chat_msg(SenderType::User, &"Y".repeat(11000))),
+            msg_tuple(new_chat_msg(SenderType::User, &"Z".repeat(11000))),
+        ], false },
     )]
     fn test_compact(messages: Vec<(ChatMessage, Option<usize>)>, expect_noop: bool) {
         let mut ctx = Context::new(10000);
@@ -786,6 +790,20 @@ mod compact_tests {
             assert_eq!(result.compacted_messages, result.original_messages);
         } else {
             assert!(result.compacted_messages <= result.original_messages);
+            // Verify chronological order is maintained
+            let texts: Vec<&str> = ctx.messages.iter().map(|(m, _)| m.text.as_str()).collect();
+            if texts.len() >= 3 {
+                assert!(
+                    texts[0].starts_with('X'),
+                    "Oldest message should be first after compaction, got: {}",
+                    texts[0].chars().next().unwrap_or('?')
+                );
+                assert!(
+                    texts[texts.len() - 1].starts_with('Z'),
+                    "Newest message should be last after compaction, got: {}",
+                    texts[texts.len() - 1].chars().next().unwrap_or('?')
+                );
+            }
         }
     }
 }

crates/core/src/provider/gguf/checkpoint.rs

@@ -119,10 +119,6 @@ pub struct CheckpointManager {
     is_hybrid: bool,
     flags: LlamaStateSeqFlags,
     first_checkpoint_size: Option<usize>,
-    /// Dynamic periodic checkpoint interval (calculated from n_ctx and max_checkpoints)
-    periodic_interval: usize,
-    /// Last position where a periodic checkpoint was saved
-    last_periodic_position: usize,
 }
 
 impl CheckpointManager {
@@ -131,28 +127,20 @@ impl CheckpointManager {
     /// # Arguments
     /// * `max_checkpoints` - Maximum number of checkpoints to keep (FIFO eviction)
     /// * `is_hybrid` - Whether the model is hybrid/recurrent (uses PARTIAL_ONLY flag)
-    /// * `n_ctx` - Context window size (used to calculate periodic interval)
-    pub fn new(max_checkpoints: usize, is_hybrid: bool, n_ctx: usize) -> Self {
+    /// * `n_ctx` - Context window size
+    pub fn new(max_checkpoints: usize, is_hybrid: bool, _n_ctx: usize) -> Self {
         let flags = if is_hybrid {
             LlamaStateSeqFlags::PARTIAL_ONLY
         } else {
             LlamaStateSeqFlags::empty()
         };
 
-        // Calculate periodic checkpoint interval
-        // Reserve 4 slots for transitions + 4 for request boundaries = 8
-        // Remaining for periodic
-        let periodic_slots = max_checkpoints.saturating_sub(8).max(1);
-        let periodic_interval = n_ctx / periodic_slots;
-
         Self {
             checkpoints: Vec::new(),
             max_checkpoints,
             is_hybrid,
             flags,
             first_checkpoint_size: None,
-            periodic_interval,
-            last_periodic_position: 0,
         }
     }
 
@@ -223,7 +211,10 @@ impl CheckpointManager {
             };
         }
 
-        // Hybrid: use checkpoint exact prefix matching with priority.
+        // Hybrid: use checkpoint exact prefix matching.
+        // Prefer longest prefix match (most tokens skipped = most efficient).
+        // Use priority as tiebreaker when lengths are equal.
+        let mut best_match_len: usize = 0;
         let mut best_match_priority: u8 = 0;
         let mut best_match_transition: Option<TransitionType> = None;
         if !self.checkpoints.is_empty() && !tokens.is_empty() {
@@ -238,21 +229,21 @@ impl CheckpointManager {
                 };
 
                 if is_prefix {
+                    let match_len = cp.tokens.len();
                     let priority = checkpoint_priority(cp.transition);
                     debug!(
                         "Checkpoint check: cp_position={}, cp_tokens_len={}, transition={:?}, priority={}, current_position={}",
-                        cp.position,
-                        cp.tokens.len(),
-                        cp.transition,
-                        priority,
-                        current_position
+                        cp.position, match_len, cp.transition, priority, current_position
                     );
 
-                    // Pick highest priority match
-                    if priority > best_match_priority {
+                    // Pick longest prefix match; break ties by highest priority
+                    if match_len > best_match_len
+                        || (match_len == best_match_len && priority > best_match_priority)
+                    {
+                        best_match_len = match_len;
                         best_match_priority = priority;
                         checkpoint_restored = true;
-                        checkpoint_tokens_skipped = cp.position;
+                        checkpoint_tokens_skipped = match_len;
                         restored_position = Some(cp.position as i32);
                         best_match_transition = Some(cp.transition);
                     }
@@ -307,21 +298,6 @@ impl CheckpointManager {
         }
     }
 
-    /// Checks if we should save a periodic checkpoint based on tokens processed.
-    ///
-    /// # Arguments
-    /// * `current_position` - Current position in the sequence
-    ///
-    /// # Returns
-    /// `true` if a periodic checkpoint should be saved
-    pub fn should_save_periodic(&self, current_position: usize) -> bool {
-        if self.periodic_interval == 0 {
-            return false;
-        }
-        let tokens_since_last = current_position.saturating_sub(self.last_periodic_position);
-        tokens_since_last >= self.periodic_interval
-    }
-
     /// Saves a checkpoint at a specific transition point.
     ///
     /// # Arguments
@@ -742,6 +718,87 @@ mod checkpoint_tests {
         assert_eq!(result.restored_position, Some(3));
     }
 
+    #[test]
+    fn test_cache_status_prefers_longer_prefix_over_higher_priority() {
+        // ToolCall (3 tokens, pri 40) vs TurnEnd (5 tokens, pri 20).
+        // Both are prefixes of the 7-token request.
+        // Longest prefix should win: TurnEnd (5 tokens) beats ToolCall (3 tokens).
+        let checkpoints = vec![
+            Checkpoint::new(
+                vec![token(1), token(2), token(3)],
+                vec![1, 2, 3],
+                3,
+                TransitionType::ToolCall,
+            ),
+            Checkpoint::new(
+                vec![token(1), token(2), token(3), token(4), token(5)],
+                vec![1, 2, 3, 4, 5],
+                5,
+                TransitionType::TurnEnd,
+            ),
+        ];
+        let tokens = vec![
+            token(1),
+            token(2),
+            token(3),
+            token(4),
+            token(5),
+            token(6),
+            token(7),
+        ];
+
+        let mgr = add_checkpoints(CheckpointManager::new(10, true, 4096), checkpoints);
+        let result = mgr.cache_status(true, 0, &tokens, &[]);
+
+        assert!(result.checkpoint_restored);
+        assert_eq!(
+            result.tokens_to_skip, 5,
+            "TurnEnd (5 tokens) should be preferred over ToolCall (3 tokens) due to longer prefix"
+        );
+        assert_eq!(result.restored_position, Some(5));
+        assert_eq!(result.restored_transition, Some(TransitionType::TurnEnd));
+    }
+
+    #[test]
+    fn test_cache_status_same_length_uses_priority_tiebreaker() {
+        // Periodic (3 tokens, pri 10) vs TurnStart (3 tokens, pri 50).
+        // Same prefix length, so higher priority (TurnStart) wins.
+        let checkpoints = vec![
+            Checkpoint::new(
+                vec![token(1), token(2), token(3)],
+                vec![1, 2, 3],
+                3,
+                TransitionType::Periodic,
+            ),
+            Checkpoint::new(
+                vec![token(1), token(2), token(3)],
+                vec![1, 2, 3],
+                3,
+                TransitionType::TurnStart,
+            ),
+        ];
+        let tokens = vec![
+            token(1),
+            token(2),
+            token(3),
+            token(4),
+            token(5),
+            token(6),
+            token(7),
+        ];
+
+        let mgr = add_checkpoints(CheckpointManager::new(10, true, 4096), checkpoints);
+        let result = mgr.cache_status(true, 0, &tokens, &[]);
+
+        assert!(result.checkpoint_restored);
+        assert_eq!(result.tokens_to_skip, 3);
+        assert_eq!(
+            result.restored_transition,
+            Some(TransitionType::TurnStart),
+            "TurnStart (pri 50) should be preferred over Periodic (pri 10) when lengths are equal"
+        );
+    }
+
     // Helper function to add checkpoints to manager for testing
     fn add_checkpoints(
         mut mgr: CheckpointManager,

crates/core/src/provider/gguf/model.rs

@@ -751,6 +751,7 @@ fn handle_request(
         );
         if state.checkpoint_manager.restore(context, cp) {
             state.position = cp.position as i32;
+            state.previous_tokens = cp.tokens.clone();
             debug!(
                 "Checkpoint restored: skipping {} tokens, position now {}",
                 cache_status.tokens_to_skip, state.position
@@ -839,6 +840,36 @@ fn handle_request(
             }
             // Use decode with memory recovery
             if let Err(_e) = decode_with_recovery(context, &mut batch, state) {
+                // If OOM cleared state (position=0), retry all tokens from scratch
+                if state.position == 0 {
+                    let mut retry_ok = true;
+                    let mut retry_pos = 0i32;
+                    for retry_chunk in tokens.chunks(state.n_batch) {
+                        batch.clear();
+                        for (i, &token) in retry_chunk.iter().enumerate() {
+                            if let Err(_e) =
+                                batch.add(token, retry_pos, &[0], i == retry_chunk.len() - 1)
+                            {
+                                retry_ok = false;
+                                break;
+                            }
+                            retry_pos += 1;
+                        }
+                        if !retry_ok {
+                            break;
+                        }
+                        if let Err(e) = context.decode(&mut batch) {
+                            tracing::error!("decode failed during retry after OOM: {}", e);
+                            retry_ok = false;
+                            break;
+                        }
+                    }
+                    if retry_ok {
+                        in_token_count = retry_pos;
+                        tracing::info!("decode succeeded after OOM recovery + full retry");
+                        break;
+                    }
+                }
                 // Memory was trimmed - send completion with trim signal instead of error
                 // This allows generation to continue gracefully
                 let _ = response_tx.blocking_send(Ok(Completion::Response(CompletionResponse {
@@ -871,30 +902,6 @@ fn handle_request(
                 })));
                 return;
             }
-
-            // Save periodic checkpoint if we've crossed the interval
-            if state.is_hybrid
-                && state
-                    .checkpoint_manager
-                    .should_save_periodic(in_token_count as usize)
-            {
-                // tokens contains new tokens for this request
-                // in_token_count = previous_position + tokens_processed_in_this_request
-                // We need to save all tokens up to current position = previous_tokens + processed new tokens
-                let tokens_processed = (in_token_count - state.position) as usize;
-                let tokens_for_checkpoint = state
-                    .previous_tokens
-                    .iter()
-                    .chain(tokens.iter().take(tokens_processed))
-                    .cloned()
-                    .collect::<Vec<_>>();
-                state.checkpoint_manager.save_at_transition(
-                    context,
-                    tokens_for_checkpoint,
-                    in_token_count as usize,
-                    TransitionType::Periodic,
-                );
-            }
         }
     } else {
         debug!(
@@ -906,6 +913,16 @@ fn handle_request(
     state.position = in_token_count;
     state.previous_tokens = tokens.clone();
 
+    // Save periodic checkpoint after all prompt tokens processed
+    if state.is_hybrid && !had_overflow && state.position > 0 {
+        state.checkpoint_manager.save_at_transition(
+            context,
+            tokens.clone(),
+            state.position as usize,
+            TransitionType::Periodic,
+        );
+    }
+
     debug!(
         "After prompt: position={}, batch_n_tokens={}, is_hybrid={}, tokens_to_process.len()={}",
         state.position,