Address code review comments from @Zabuzard and @tj-wazei

  - refactor(XkcdRetriever): rename to XkcdService

  - doc(ChatGptService): clarify the meaning of RAG and vector store

  - XkcdService: improve fetchAllXkcdPosts method

  - XkcdCommand: do not allow more than 40KB of chat messages

Signed-off-by: Chris Sdogkos <work@chris-sdogkos.com>

Author: christolis

application/src/main/java/org/togetherjava/tjbot/features/chatgpt/ChatGptService.java

@@ -167,8 +167,10 @@ public Optional<String> uploadFileIfNotExists(Path filePath, FilePurpose purpose
      * Creates a new vector store with the given file ID if none exists or returns the ID of the
      * existing vector store with that name.
      * <p>
-     * You can use this for RAG purposes, it is an effective way to give ChatGPT extra information
-     * from what it has been trained.
+     * A vector store indexes document content as embeddings for semantic search. You can use this
+     * for RAG (Retrieval-Augmented Generation), where the model retrieves relevant context from
+     * your documents before generating responses, effectively giving it access to information
+     * beyond its training data.
      *
      * @param fileId The ID of the file to include in the new vector store.
      * @return The vector store ID (existing or newly created).

application/src/main/java/org/togetherjava/tjbot/features/xkcd/XkcdCommand.java

@@ -11,6 +11,7 @@
 import net.dv8tion.jda.api.interactions.commands.OptionType;
 import net.dv8tion.jda.api.interactions.commands.build.OptionData;
 import net.dv8tion.jda.api.interactions.commands.build.SubcommandData;
+import org.apache.commons.lang3.IntegerRange;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -37,7 +38,7 @@
  * <li>{@code /xkcd custom <id>} - Posts a specific XKCD comic by ID from local cache.</li>
  * </ul>
  *
- * Relies on {@link XkcdRetriever} for local XKCD data and {@link ChatGptService} for AI-powered
+ * Relies on {@link XkcdService} for local XKCD data and {@link ChatGptService} for AI-powered
  * relevance matching via OpenAI's file search tool and vector stores.
  */
 public final class XkcdCommand extends SlashCommandAdapter {
@@ -50,21 +51,22 @@ public final class XkcdCommand extends SlashCommandAdapter {
     private static final String LAST_MESSAGES_AMOUNT_OPTION_NAME = "amount";
     private static final String XKCD_ID_OPTION_NAME = "id";
     private static final int MAXIMUM_MESSAGE_HISTORY = 100;
+    private static final int MESSAGE_HISTORY_CUTOFF_SIZE_KB = 40_000;
     private static final String VECTOR_STORE_XKCD = "xkcd-comics";
     private static final ChatGptModel CHAT_GPT_MODEL = ChatGptModel.FAST;
     private static final Pattern XKCD_POST_PATTERN = Pattern.compile("^\\D*(\\d+)");
     private static final String CHATGPT_NO_ID_MESSAGE =
             "ChatGPT could not respond with a XKCD post ID.";
 
     private final ChatGptService chatGptService;
-    private final XkcdRetriever xkcdRetriever;
+    private final XkcdService xkcdService;
 
     public XkcdCommand(ChatGptService chatGptService) {
         super(COMMAND_NAME, "Post a relevant XKCD from the chat or your own",
                 CommandVisibility.GLOBAL);
 
         this.chatGptService = chatGptService;
-        this.xkcdRetriever = new XkcdRetriever(chatGptService);
+        this.xkcdService = new XkcdService(chatGptService);
 
         OptionData lastMessagesAmountOption =
                 new OptionData(OptionType.INTEGER, LAST_MESSAGES_AMOUNT_OPTION_NAME,
@@ -81,7 +83,7 @@ public XkcdCommand(ChatGptService chatGptService) {
                 "The XKCD number to post to the chat")
             .setMinValue(0)
             .setRequired(true)
-            .setMaxValue(xkcdRetriever.getXkcdPosts().size());
+            .setMaxValue(xkcdService.getXkcdPosts().size());
 
         SubcommandData customSubcommand = new SubcommandData(SUBCOMMAND_CUSTOM,
                 "Post your own XKCD regardless of the recent chat messages")
@@ -149,17 +151,18 @@ private void handleCustomXkcd(SlashCommandInteractionEvent event) {
 
     private void sendRelevantXkcdEmbedFromMessages(List<Message> messages,
             SlashCommandInteractionEvent event) {
-        String discordChat = formatDiscordChatHistory(messages);
-        String xkcdComicsFileId = xkcdRetriever.getXkcdUploadedFileId();
+        List<Message> discordChatCutoff = cutoffDiscordChatHistory(messages);
+        String discordChatFormatted = formatDiscordChatHistory(discordChatCutoff);
+        String xkcdComicsFileId = xkcdService.getXkcdUploadedFileId();
         String xkcdVectorStore =
                 chatGptService.createOrGetVectorStore(xkcdComicsFileId, VECTOR_STORE_XKCD);
         FileSearchTool fileSearch =
                 FileSearchTool.builder().vectorStoreIds(List.of(xkcdVectorStore)).build();
 
         Tool tool = Tool.ofFileSearch(fileSearch);
 
-        Optional<String> responseOptional = chatGptService
-            .sendPrompt(getChatgptRelevantPrompt(discordChat), CHAT_GPT_MODEL, List.of(tool));
+        Optional<String> responseOptional = chatGptService.sendPrompt(
+                getChatgptRelevantPrompt(discordChatFormatted), CHAT_GPT_MODEL, List.of(tool));
 
         Optional<Integer> responseIdOptional = getXkcdIdFromMessage(responseOptional.orElseThrow());
 
@@ -174,11 +177,15 @@ private void sendRelevantXkcdEmbedFromMessages(List<Message> messages,
         Optional<MessageEmbed> embedOptional =
                 constructEmbed(responseId, "Most relevant XKCD according to ChatGPT.");
 
-        embedOptional.ifPresent(embed -> event.getHook().sendMessageEmbeds(embed).queue());
+        embedOptional.ifPresentOrElse(embed -> event.getHook().sendMessageEmbeds(embed).queue(),
+                () -> event.getHook()
+                    .setEphemeral(true)
+                    .sendMessage("I could not find post with ID " + responseId)
+                    .queue());
     }
 
     private Optional<MessageEmbed> constructEmbed(int xkcdId, String footer) {
-        Optional<XkcdPost> xkcdPostOptional = xkcdRetriever.getXkcdPost(xkcdId);
+        Optional<XkcdPost> xkcdPostOptional = xkcdService.getXkcdPost(xkcdId);
 
         if (xkcdPostOptional.isEmpty()) {
             logger.warn("Could not find XKCD post with ID {} from local map", xkcdId);
@@ -220,6 +227,23 @@ private String formatDiscordChatHistory(List<Message> messages) {
             .toString();
     }
 
+    private List<Message> cutoffDiscordChatHistory(List<Message> messages) {
+        int cutoffMessageIndex = (int) IntegerRange.of(0, messages.size() - 1)
+            .toIntStream()
+            .map(index -> countMessagesLength(messages.subList(0, index)))
+            .filter(length -> length < MESSAGE_HISTORY_CUTOFF_SIZE_KB)
+            .count();
+
+        return messages.subList(0, cutoffMessageIndex);
+    }
+
+    private int countMessagesLength(List<Message> messages) {
+        return messages.stream()
+            .mapToInt(message -> message.getContentRaw().length()
+                    + message.getAuthor().getName().length())
+            .sum();
+    }
+
     private static String getChatgptRelevantPrompt(String discordChat) {
         return """
                 <discord-chat>

…a/tjbot/features/xkcd/XkcdRetriever.java …ava/tjbot/features/xkcd/XkcdService.javaapplication/src/main/java/org/togetherjava/tjbot/features/xkcd/XkcdRetriever.java renamed to application/src/main/java/org/togetherjava/tjbot/features/xkcd/XkcdService.java application/src/main/java/org/togetherjava/tjbot/features/xkcd/XkcdRetriever.java renamed to application/src/main/java/org/togetherjava/tjbot/features/xkcd/XkcdService.java

@@ -20,11 +20,14 @@
 import java.nio.file.Path;
 import java.time.Duration;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
 import java.util.concurrent.Semaphore;
 
 /**
@@ -36,25 +39,23 @@
  * Posts are cached locally in {@value #SAVED_XKCD_PATH} as JSON and uploaded to OpenAI using the
  * provided {@link ChatGptService} if not already present.
  */
-public class XkcdRetriever {
+public class XkcdService {
 
-    private static final Logger logger = LoggerFactory.getLogger(XkcdRetriever.class);
+    private static final Logger logger = LoggerFactory.getLogger(XkcdService.class);
 
     private static final HttpClient CLIENT =
             HttpClient.newBuilder().connectTimeout(Duration.ofSeconds(10)).build();
     private static final String XKCD_GET_URL = "https://xkcd.com/%d/info.0.json";
     private static final String SAVED_XKCD_PATH = "xkcd.generated.json";
     private static final int XKCD_POSTS_AMOUNT = 3201;
-    private static final int FETCH_XCKD_POSTS_POOL_SIZE = 20;
     private static final int FETCH_XKCD_POSTS_SEMAPHORE_SIZE = 10;
-    private static final int FETCH_XKCD_POSTS_THREAD_SLEEP_MS = 50;
     private static final ObjectMapper objectMapper = new ObjectMapper();
 
     private final Map<Integer, XkcdPost> xkcdPosts = new HashMap<>();
     private final ChatGptService chatGptService;
     private String xkcdUploadedFileId;
 
-    public XkcdRetriever(ChatGptService chatGptService) {
+    public XkcdService(ChatGptService chatGptService) {
         this.chatGptService = chatGptService;
 
         Optional<String> xkcdUploadedFileIdOptional =
@@ -97,27 +98,30 @@ public Map<Integer, XkcdPost> getXkcdPosts() {
 
     private void fetchAllXkcdPosts(Path savedXckdsPath) {
         objectMapper.enable(SerializationFeature.INDENT_OUTPUT);
-        Semaphore semaphore = new Semaphore(FETCH_XKCD_POSTS_SEMAPHORE_SIZE);
 
         logger.info("Fetching {} XKCD posts...", XKCD_POSTS_AMOUNT);
-        try (ExecutorService executor = Executors.newFixedThreadPool(FETCH_XCKD_POSTS_POOL_SIZE)) {
-            CompletableFuture.allOf(IntegerRange.of(1, XKCD_POSTS_AMOUNT)
+        try (ExecutorService executor = Executors.newVirtualThreadPerTaskExecutor()) {
+            Semaphore semaphore = new Semaphore(FETCH_XKCD_POSTS_SEMAPHORE_SIZE);
+            List<? extends Future<?>> futures = IntegerRange.of(1, XKCD_POSTS_AMOUNT)
                 .toIntStream()
                 .filter(id -> id != 404) // XKCD has a joke on comic ID 404 so exclude
-                .mapToObj(xkcdId -> CompletableFuture.runAsync(() -> {
+                .mapToObj(xkcdId -> executor.submit(() -> {
                     semaphore.acquireUninterruptibly();
-                    try {
-                        Optional<XkcdPost> postOptional = this.retrieveXkcdPost(xkcdId).join();
-                        postOptional.ifPresent(post -> xkcdPosts.put(xkcdId, post));
-
-                        Thread.sleep(FETCH_XKCD_POSTS_THREAD_SLEEP_MS);
-                    } catch (InterruptedException _) {
-                        Thread.currentThread().interrupt();
-                    } finally {
-                        semaphore.release();
-                    }
-                }, executor))
-                .toArray(CompletableFuture[]::new)).join();
+                    retrieveXkcdPost(xkcdId).join().ifPresent(post -> xkcdPosts.put(xkcdId, post));
+                    semaphore.release();
+                }))
+                .toList();
+
+            try {
+                for (Future<?> future : futures) {
+                    future.get();
+                }
+            } catch (InterruptedException e) {
+                logger.error("Failed to wait for future", e);
+                Thread.currentThread().interrupt();
+            } catch (ExecutionException e) {
+                logger.error("Could not get result from future", e);
+            }
         }
 
         saveToFile(savedXckdsPath, xkcdPosts);