fix insert document

Author: dfalbel

R/ragnar-chat.R

@@ -253,9 +253,10 @@ RagnarChat <- R6::R6Class(
               # when it's a tool result we drop the assistant turn that (usually a tool call)
               c(ti, ti - 1)
             } else if (S7::S7_inherits(content, ContentRagnarDocuments)) {
-              # when it's content ragnar documents, we drop the next turn - because we proactively
-              # inserted the documents and the next turn is just the LLM acknowledging them.
-              c(ti, ti + 1)
+              # when it's content ragnar documents, we drop only the current turn.
+              # Note: tha's unlikely to ever drop a turn because the turn also contains
+              # the user question, so we always have at least one content.
+              ti
             }
             next
           }
@@ -327,9 +328,10 @@ RagnarChat <- R6::R6Class(
               # when it's a tool result we drop the assistant turn that (usually a tool call)
               c(ti, ti - 1)
             } else if (S7::S7_inherits(content, ContentRagnarDocuments)) {
-              # when it's content ragnar documents, we drop the next turn - because we proactively
-              # inserted the documents and the next turn is just the LLM acknowledging them.
-              c(ti, ti + 1)
+              # when it's content ragnar documents, we drop only the current turn.
+              # Note: tha's unlikely to ever drop a turn because the turn also contains
+              # the user question, so we always have at least one content.
+              ti
             }
             next
           }
@@ -399,39 +401,19 @@ RagnarChat <- R6::R6Class(
     #' @description Some models do not support tool calling, so instead of adding a tool call
     #' request (faking that the model asked for some search results) we actually
     #' proactively insert context into the chat user - as if the user did it had done it.
-    #' - User: {documents}
-    #' - LLM: {llm_answer}.
-    #' - {...} The contents of the user turn that generated the tool call.
+    #' - User: {question} {documents}
+    #' - LLM: answer
     #' @param ... The contents of the user turn that generated the tool call.
-    #' @param llm_answer The answer that the LLM should give after the documents are inserted.
-    #' @param after The index in the turns list to insert the user and assistant turns.
     turns_insert_documents = function(
       ...,
-      query,
-      llm_answer = "Thanks for the information. I'll use the documents to answer your question. Please ask a question.",
-      after = 0L
+      query
     ) {
       documents <- self$ragnar_tool(query)
 
-      user_turn <- ellmer::Turn(
-        role = "user",
-        contents = list(
-          ContentRagnarDocuments(text = documents)
-        )
-      )
-
-      assistant_turn <- ellmer::Turn(
-        role = "assistant",
-        contents = list(
-          ellmer::ContentText(text = llm_answer)
-        )
+      list(
+        ...,
+        ContentRagnarDocuments(text = documents)
       )
-
-      turns <- self$get_turns()
-      turns <- append(turns, list(user_turn, assistant_turn), after = after)
-      self$set_turns(turns)
-
-      list(...)
     }
   ),
 

tests/testthat/test-ragnar-chat.R

@@ -1,6 +1,6 @@
 test_that("RagnarChat", {
   store <- test_store()
-  chat <- chat_ragnar(ellmer::chat_openai, .store = store)
+  chat <- chat_ragnar(ellmer::chat_openai, model = "gpt-4.1-nano", .store = store)
 
   out <- chat$chat("advanced R")
 
@@ -12,12 +12,18 @@ test_that("RagnarChat", {
   expect_equal(length(chat$get_turns()), 2)
 
   out <- chat$chat("advanced R")
-  expect_equal(length(chat$get_turns()), 2 + 2)
+  # 2 turns that were already there + 2 turns of forced tool calls
+  # + user + LLM
+  expect_equal(length(chat$get_turns()), 2 + 2 + 2)
 
   # the default pruning will clear the previous tool calls.
-  # so we end up with 6 turns
+  # so we end up with 6 turns + 1 pair
   out <- chat$chat("more chatting")
-  expect_equal(length(chat$get_turns()), 2 + 2 + 2)
+  expect_equal(length(chat$get_turns()), 6 + 2)
+
+  # prune tool calls will clear two turns
+  chat$turns_prune_tool_calls()
+  expect_equal(length(chat$get_turns()), 6)
 })
 
 test_that("Implementing query rewriting", {
@@ -33,6 +39,7 @@ test_that("Implementing query rewriting", {
   store <- test_store()
   chat <- chat_ragnar(
     ellmer::chat_openai, 
+    model = "gpt-4.1-nano",
     .store = store,
     .on_user_turn = function(self, ...) {
 
@@ -55,6 +62,7 @@ test_that("remove chunks by id works", {
   store <- test_store()
   chat <- chat_ragnar(
     ellmer::chat_openai, 
+    model = "gpt-4.1-nano",
     .store = store,
     .on_user_turn = function(self, ...) {
        self$turns_insert_tool_call_request(
@@ -84,6 +92,7 @@ test_that("duplicated chunks are not returned", {
   store <- test_store()
   chat <- chat_ragnar(
     ellmer::chat_openai, 
+    model = "gpt-4.1-nano",
     .store = store,
     .on_user_turn = function(self, ...) {
       self$turns_insert_tool_call_request(
@@ -102,3 +111,27 @@ test_that("duplicated chunks are not returned", {
 
   expect_equal(length(unique(new_chunk_ids)), length(new_chunk_ids))
 })
+
+test_that("Can insert chunks premptively in the user chat", {
+  store <- test_store()
+  chat <- chat_ragnar(
+    ellmer::chat_openai, 
+    model = "gpt-4.1-nano",
+    .store = store,
+    .on_user_turn = function(self, ...) {
+      self$turns_insert_documents(
+        ...,
+        query = paste(..., collapse = " ")
+      )
+    }
+  )
+
+  out <- chat$chat("advanced R")
+  expect_equal(length(chat$get_turns()), 2)
+  # it adds the documents into the context
+  expect_equal(length(chat$get_turns()[[1]]@contents), 2)
+
+  chat$turns_prune_chunks()
+  expect_equal(length(chat$get_turns()), 2)
+  expect_equal(length(chat$get_turns()[[1]]@contents), 1)
+})