diff --git a/.buildnumber b/.buildnumber index edaf5bd..d69faa4 100644 --- a/.buildnumber +++ b/.buildnumber @@ -1,2 +1,2 @@ month=26.8 -count=49 +count=50 diff --git a/pom.xml b/pom.xml index 0f70d74..ecae19a 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ com.github.drafael.chat4j chat4j - 26.8.49 + 26.8.50 jar Chat4J diff --git a/src/main/java/com/github/drafael/chat4j/chat/AssistantSourceFormatter.java b/src/main/java/com/github/drafael/chat4j/chat/AssistantSourceFormatter.java new file mode 100644 index 0000000..4f2fd6d --- /dev/null +++ b/src/main/java/com/github/drafael/chat4j/chat/AssistantSourceFormatter.java @@ -0,0 +1,192 @@ +package com.github.drafael.chat4j.chat; + +import com.github.drafael.chat4j.chat.content.ExternalLinkSupport; +import com.github.drafael.chat4j.provider.api.content.CitationKind; +import com.github.drafael.chat4j.provider.api.content.CitationRef; +import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.Strings; + +import java.net.URI; +import java.util.Arrays; +import java.util.List; +import java.util.TreeMap; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import static java.util.stream.Collectors.joining; +import static java.util.stream.Collectors.toMap; + +final class AssistantSourceFormatter { + + private static final Pattern SOURCE_URL_PATTERN = Pattern.compile("(?:\\[[^]]+])?\\(<(https?://[^>\\s]+)>\\)|<(https?://[^>\\s]+)>|(?:\\[[^]]+])?\\((https?://(?:[^\\s()<>]|\\([^\\s()<>]*\\))+)\\)|(https?://(?:[^\\s()<>]|\\([^\\s()<>]*\\))+)"); + private static final Pattern SOURCE_REFERENCE_LINE_PATTERN = Pattern.compile("(?m)^\\s*(?:[-*]\\s*)?\\[\\d+]\\s*(?:\\([^)]*https?://[^)]*\\)|.*https?://\\S+)"); + + private AssistantSourceFormatter() { + } + + static String mergeWebSearchActivityWithAnswerSources( + boolean webSearchEnabled, + String assistantText, + String existingActivity, + List citations + ) { + if (!webSearchEnabled) { + return existingActivity; + } + + String sourceActivity = citationSourceLines(citations); + if (StringUtils.isBlank(sourceActivity)) { + sourceActivity = extractWebSearchSourcesFromAssistantText(assistantText); + } + if (StringUtils.isBlank(sourceActivity)) { + return existingActivity; + } + + return "%s\n\n**Sources**\n%s".formatted( + StringUtils.defaultString(existingActivity).trim(), + sourceActivity + ).trim(); + } + + static String appendCitationSourcesIfNeeded(String assistantText, List citations) { + String text = StringUtils.defaultString(assistantText); + if (citations == null || citations.isEmpty() + || SOURCE_REFERENCE_LINE_PATTERN.matcher(text).find() + || hasSourceSectionWithUrls(text)) { + return text; + } + + String sources = citationSourceLines(citations); + if (StringUtils.isBlank(sources)) { + return text; + } + + String answer = text.stripTrailing(); + return StringUtils.isBlank(answer) + ? "Sources:\n%s".formatted(sources) + : "%s\n\nSources:\n%s".formatted(answer, sources); + } + + static String escapeMarkdownLinkLabel(String value) { + return StringUtils.defaultString(value).replace("[", "\\[").replace("]", "\\]"); + } + + private static boolean hasSourceSectionWithUrls(String text) { + boolean inSources = false; + for (String line : text.split("\\R")) { + String normalizedLine = normalizeHeadingLine(line); + if (Strings.CI.equals(normalizedLine, "sources")) { + inSources = true; + continue; + } + if (inSources && isMarkdownHeadingLine(line)) { + inSources = false; + } + if (inSources && SOURCE_URL_PATTERN.matcher(line).find()) { + return true; + } + } + return false; + } + + private static boolean isMarkdownHeadingLine(String line) { + String trimmed = StringUtils.trimToEmpty(line); + return trimmed.startsWith("#") || trimmed.matches("\\*\\*.+\\*\\*:?"); + } + + private static String normalizeHeadingLine(String line) { + String heading = StringUtils.trimToEmpty(line).replaceFirst("^#+\\s*", ""); + heading = Strings.CS.removeEnd(heading, ":").trim(); + heading = Strings.CS.removeEnd(Strings.CS.removeStart(heading, "**"), "**").trim(); + heading = Strings.CS.removeEnd(heading, ":").trim(); + return heading; + } + + private static String citationSourceLines(List citations) { + if (citations == null || citations.isEmpty()) { + return ""; + } + + return citations.stream() + .filter(citation -> citation != null && citation.number() > 0) + .filter(citation -> citation.kind() == CitationKind.WEB) + .filter(citation -> ExternalLinkSupport.isAllowedHttpLink(citation.url())) + .collect(toMap( + CitationRef::number, + AssistantSourceFormatter::citationSourceLine, + (existing, replacement) -> existing, + TreeMap::new + )) + .values() + .stream() + .collect(joining("\n")); + } + + private static String citationSourceLine(CitationRef citation) { + return "[%d] [%s](%s)".formatted( + citation.number(), + escapeMarkdownLinkLabel(citationSourceLabel(citation)), + markdownLinkDestination(citation.url()) + ); + } + + private static String markdownLinkDestination(String url) { + return "<%s>".formatted(StringUtils.defaultString(url).replace(">", "%3E")); + } + + private static String citationSourceLabel(CitationRef citation) { + String title = StringUtils.trimToEmpty(citation.displayTitle()).replaceAll("\\s+", " "); + if (StringUtils.isNotBlank(title) && !Strings.CS.equals(title, citation.url())) { + return title; + } + return sourceDomain(citation.url()); + } + + private static String sourceDomain(String url) { + try { + String host = URI.create(url).getHost(); + return StringUtils.defaultIfBlank(Strings.CS.removeStart(host, "www."), url); + } catch (Exception e) { + return url; + } + } + + private static String extractWebSearchSourcesFromAssistantText(String assistantText) { + if (StringUtils.isBlank(assistantText)) { + return ""; + } + + List sourceItems = Arrays.stream(assistantText.split("\\R")) + .map(SOURCE_URL_PATTERN::matcher) + .filter(Matcher::find) + .map(AssistantSourceFormatter::matchedSourceItem) + .filter(StringUtils::isNotBlank) + .distinct() + .limit(10) + .toList(); + if (sourceItems.isEmpty()) { + return ""; + } + + StringBuilder sources = new StringBuilder(); + sourceItems.forEach(item -> sources.append("- ").append(item).append("\n")); + return sources.toString().trim(); + } + + private static String matchedSourceItem(Matcher matcher) { + String match = StringUtils.trimToEmpty(matcher.group()); + return Strings.CI.startsWith(match, "http://") || Strings.CI.startsWith(match, "https://") + ? markdownLinkDestination(matchedSourceUrl(matcher)) + : match; + } + + private static String matchedSourceUrl(Matcher matcher) { + for (int i = 1; i <= matcher.groupCount(); i++) { + String value = matcher.group(i); + if (StringUtils.isNotBlank(value)) { + return value; + } + } + return ""; + } +} diff --git a/src/main/java/com/github/drafael/chat4j/chat/ChatPanel.java b/src/main/java/com/github/drafael/chat4j/chat/ChatPanel.java index 3a71578..321df43 100644 --- a/src/main/java/com/github/drafael/chat4j/chat/ChatPanel.java +++ b/src/main/java/com/github/drafael/chat4j/chat/ChatPanel.java @@ -15,7 +15,6 @@ import com.github.drafael.chat4j.chat.composer.FileAttachmentChip; import com.github.drafael.chat4j.chat.composer.ImageAttachmentPreview; import com.github.drafael.chat4j.chat.composer.InputBar; -import com.github.drafael.chat4j.chat.content.ExternalLinkSupport; import com.github.drafael.chat4j.chat.conversation.ConversationAttachment; import com.github.drafael.chat4j.chat.conversation.ConversationEntry; import com.github.drafael.chat4j.chat.conversation.webview.jcef.JcefBrowserView; @@ -55,7 +54,6 @@ import com.github.drafael.chat4j.provider.api.WebSearchRequestOptions; import com.github.drafael.chat4j.provider.api.content.AgentToolActivityMeta; import com.github.drafael.chat4j.provider.api.content.AttachmentRef; -import com.github.drafael.chat4j.provider.api.content.CitationKind; import com.github.drafael.chat4j.provider.api.content.CitationRef; import com.github.drafael.chat4j.provider.api.content.ContentPart; import com.github.drafael.chat4j.provider.api.content.FilePart; @@ -92,7 +90,6 @@ import java.awt.event.MouseAdapter; import java.awt.event.MouseEvent; import java.io.IOException; -import java.net.URI; import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; @@ -110,7 +107,6 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; -import java.util.TreeMap; import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; @@ -123,7 +119,6 @@ import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Supplier; -import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.IntStream; import java.util.stream.Stream; @@ -180,8 +175,6 @@ public class ChatPanel extends JPanel { private static final Pattern ANSI_ESCAPE_PATTERN = Pattern.compile("\u001B\\[[;\\d]*[ -/]*[@-~]"); private static final Pattern NON_PRINTABLE_PATTERN = Pattern.compile("[\\p{Cntrl}&&[^\\r\\n\\t]]"); private static final Pattern UNICODE_FORMAT_PATTERN = Pattern.compile("\\p{Cf}"); - private static final Pattern SOURCE_URL_PATTERN = Pattern.compile("(?:\\[[^]]+])?\\(<(https?://[^>\\s]+)>\\)|<(https?://[^>\\s]+)>|(?:\\[[^]]+])?\\((https?://(?:[^\\s()<>]|\\([^\\s()<>]*\\))+)\\)|(https?://(?:[^\\s()<>]|\\([^\\s()<>]*\\))+)"); - private static final Pattern SOURCE_REFERENCE_LINE_PATTERN = Pattern.compile("(?m)^\\s*(?:[-*]\\s*)?\\[\\d+]\\s*(?:\\([^)]*https?://[^)]*\\)|.*https?://\\S+)"); private static final Map CHAT_MENU_ICON_CACHE = new ConcurrentHashMap<>(); private final JPanel messagesPanel; @@ -1945,7 +1938,7 @@ private String renderConsultedSourceActivity(StreamingSession session, boolean c } else { session.webSearchSources.values().forEach(source -> activity .append("- [") - .append(escapeMarkdownLinkLabel(source.title())) + .append(AssistantSourceFormatter.escapeMarkdownLinkLabel(source.title())) .append("](<") .append(source.url()) .append(">)\n")); @@ -1961,10 +1954,6 @@ private void replaceWebSearchActivity(StreamingSession session, String activity) } } - private String escapeMarkdownLinkLabel(String value) { - return StringUtils.defaultString(value).replace("[", "\\[").replace("]", "\\]"); - } - private void showWebSearchActivity(StreamingSession session, String webSearchActivity) { showWebSearchActivity(session, webSearchActivity, false); } @@ -6496,9 +6485,9 @@ private PreparedAssistantResponse prepareAssistantResponse(StreamingSession sess consultedSourceMode = session.consultedSourceMode; } if (!consultedSourceMode) { - assistantText = appendCitationSourcesIfNeeded(assistantText, citations); - assistantWebSearch = mergeAssistantWebSearchWithAnswerSources( - sendJob, + assistantText = AssistantSourceFormatter.appendCitationSourcesIfNeeded(assistantText, citations); + assistantWebSearch = AssistantSourceFormatter.mergeWebSearchActivityWithAnswerSources( + sendJob != null && sendJob.webSearchEnabled, assistantText, assistantWebSearch, citations @@ -6577,172 +6566,10 @@ private void applyPreparedAssistantResponse( nextMessageOrdinal = preparedResponse.entry().ordinal() + 1; } - private String mergeAssistantWebSearchWithAnswerSources( - SendJob sendJob, - String assistantText, - String existingActivity, - List citations - ) { - if (sendJob == null || !sendJob.webSearchEnabled) { - return existingActivity; - } - - String sourceActivity = citationSourceLines(citations); - if (StringUtils.isBlank(sourceActivity)) { - sourceActivity = extractWebSearchSourcesFromAssistantText(assistantText); - } - if (StringUtils.isBlank(sourceActivity)) { - return existingActivity; - } - - return "%s\n\n**Sources**\n%s".formatted( - StringUtils.defaultString(existingActivity).trim(), - sourceActivity - ).trim(); - } - - private String appendCitationSourcesIfNeeded(String assistantText, List citations) { - String text = StringUtils.defaultString(assistantText); - if (citations == null || citations.isEmpty() - || SOURCE_REFERENCE_LINE_PATTERN.matcher(text).find() - || hasSourceSectionWithUrls(text)) { - return text; - } - - String sources = citationSourceLines(citations); - if (StringUtils.isBlank(sources)) { - return text; - } - - String answer = text.stripTrailing(); - return StringUtils.isBlank(answer) - ? "Sources:\n%s".formatted(sources) - : "%s\n\nSources:\n%s".formatted(answer, sources); - } - - private boolean hasSourceSectionWithUrls(String text) { - boolean inSources = false; - for (String line : text.split("\\R")) { - String normalizedLine = normalizeHeadingLine(line); - if (Strings.CI.equals(normalizedLine, "sources")) { - inSources = true; - continue; - } - if (inSources && isMarkdownHeadingLine(line)) { - inSources = false; - } - if (inSources && SOURCE_URL_PATTERN.matcher(line).find()) { - return true; - } - } - return false; - } - - private boolean isMarkdownHeadingLine(String line) { - String trimmed = StringUtils.trimToEmpty(line); - return trimmed.startsWith("#") || trimmed.matches("\\*\\*.+\\*\\*:?"); - } - - private String normalizeHeadingLine(String line) { - String heading = StringUtils.trimToEmpty(line).replaceFirst("^#+\\s*", ""); - heading = Strings.CS.removeEnd(heading, ":").trim(); - heading = Strings.CS.removeEnd(Strings.CS.removeStart(heading, "**"), "**").trim(); - heading = Strings.CS.removeEnd(heading, ":").trim(); - return heading; - } - - private String citationSourceLines(List citations) { - if (citations == null || citations.isEmpty()) { - return ""; - } - - return citations.stream() - .filter(citation -> citation != null && citation.number() > 0) - .filter(citation -> citation.kind() == CitationKind.WEB) - .filter(citation -> ExternalLinkSupport.isAllowedHttpLink(citation.url())) - .collect(toMap( - CitationRef::number, - this::citationSourceLine, - (existing, replacement) -> existing, - TreeMap::new - )) - .values() - .stream() - .collect(joining("\n")); - } - - private String citationSourceLine(CitationRef citation) { - return "[%d] [%s](%s)".formatted( - citation.number(), - escapeMarkdownLinkLabel(citationSourceLabel(citation)), - markdownLinkDestination(citation.url()) - ); - } - - private String markdownLinkDestination(String url) { - return "<%s>".formatted(StringUtils.defaultString(url).replace(">", "%3E")); - } - - private String citationSourceLabel(CitationRef citation) { - String title = StringUtils.trimToEmpty(citation.displayTitle()).replaceAll("\\s+", " "); - if (StringUtils.isNotBlank(title) && !Strings.CS.equals(title, citation.url())) { - return title; - } - return sourceDomain(citation.url()); - } - - private String sourceDomain(String url) { - try { - String host = URI.create(url).getHost(); - return StringUtils.defaultIfBlank(Strings.CS.removeStart(host, "www."), url); - } catch (Exception e) { - return url; - } - } - private String normalizeWebSearchActivity(String activity) { return WebSearchActivityNormalizer.normalize(activity); } - private String extractWebSearchSourcesFromAssistantText(String assistantText) { - if (StringUtils.isBlank(assistantText)) { - return ""; - } - - List sourceItems = Arrays.stream(assistantText.split("\\R")) - .map(SOURCE_URL_PATTERN::matcher) - .filter(Matcher::find) - .map(this::matchedSourceItem) - .filter(StringUtils::isNotBlank) - .distinct() - .limit(10) - .toList(); - if (sourceItems.isEmpty()) { - return ""; - } - - StringBuilder sources = new StringBuilder(); - sourceItems.forEach(item -> sources.append("- ").append(item).append("\n")); - return sources.toString().trim(); - } - - private String matchedSourceItem(Matcher matcher) { - String match = StringUtils.trimToEmpty(matcher.group()); - return Strings.CI.startsWith(match, "http://") || Strings.CI.startsWith(match, "https://") - ? markdownLinkDestination(matchedSourceUrl(matcher)) - : match; - } - - private String matchedSourceUrl(Matcher matcher) { - for (int i = 1; i <= matcher.groupCount(); i++) { - String value = matcher.group(i); - if (StringUtils.isNotBlank(value)) { - return value; - } - } - return ""; - } - private void removeCurrentWebSearchBubbleIfBlank() { if (currentAssistantWebSearchBubble == null || StringUtils.isNotBlank(currentAssistantWebSearchBubble.getFullText())) { return; diff --git a/src/test/java/com/github/drafael/chat4j/chat/AssistantSourceFormatterTest.java b/src/test/java/com/github/drafael/chat4j/chat/AssistantSourceFormatterTest.java new file mode 100644 index 0000000..961f462 --- /dev/null +++ b/src/test/java/com/github/drafael/chat4j/chat/AssistantSourceFormatterTest.java @@ -0,0 +1,258 @@ +package com.github.drafael.chat4j.chat; + +import com.github.drafael.chat4j.provider.api.content.CitationKind; +import com.github.drafael.chat4j.provider.api.content.CitationRef; +import org.apache.commons.lang3.StringUtils; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +class AssistantSourceFormatterTest { + + @Test + @DisplayName("Native search activity adds one Sources section when citations arrive") + void mergeWebSearchActivityWithAnswerSources_whenActivityHasSearchQuery_addsOneSourcesHeading() { + List citations = List.of(webCitation( + 1, + "OpenAI docs", + "https://platform.openai.com/docs" + )); + + String merged = AssistantSourceFormatter.mergeWebSearchActivityWithAnswerSources( + true, + "Answer [1]", + "**Searched**\n- current OpenAI docs", + citations + ); + + assertThat(merged).contains("**Searched**", "**Sources**", "https://platform.openai.com/docs"); + assertThat(StringUtils.countMatches(merged, "**Sources**")).isEqualTo(1); + assertThat(merged).doesNotContain("Source URLs will appear here"); + } + + @Test + @DisplayName("Disabled Web Search leaves existing activity unchanged") + void mergeWebSearchActivityWithAnswerSources_whenWebSearchIsDisabled_keepsExistingActivity() { + String existing = "**Searched**\n- current OpenAI docs"; + + String merged = AssistantSourceFormatter.mergeWebSearchActivityWithAnswerSources( + false, + "Answer with https://platform.openai.com/docs", + existing, + List.of(webCitation(1, "OpenAI docs", "https://platform.openai.com/docs")) + ); + + assertThat(merged).isEqualTo(existing); + } + + @Test + @DisplayName("Answer URLs supply Web Search activity when structured citations are absent") + void mergeWebSearchActivityWithAnswerSources_whenCitationsAreAbsent_extractsAnswerUrls() { + String merged = AssistantSourceFormatter.mergeWebSearchActivityWithAnswerSources( + true, + "Documentation: https://example.com/docs", + "**Searched**\n- example documentation", + List.of() + ); + + assertThat(merged) + .contains("**Sources**") + .contains("- "); + } + + @Test + @DisplayName("Markdown source labels escape square brackets") + void escapeMarkdownLinkLabel_whenLabelContainsBrackets_escapesBrackets() { + assertThat(AssistantSourceFormatter.escapeMarkdownLinkLabel("Docs [current]")) + .isEqualTo("Docs \\[current\\]"); + } + + @Test + @DisplayName("Structured Web citations append a Sources section when the answer has no source references") + void appendCitationSourcesIfNeeded_whenAnswerLacksSourceReferences_appendsSources() { + List citations = List.of(webCitation( + 1, + "JNA GitHub", + "https://github.com/java-native-access/jna" + )); + + String text = AssistantSourceFormatter.appendCitationSourcesIfNeeded( + "JNA docs are at https://github.com/java-native-access/jna.", + citations + ); + + assertThat(text).contains("Sources:\n[1] [JNA GitHub]()"); + } + + @Test + @DisplayName("Structured Web citation sources use the URL domain when the title is missing") + void appendCitationSourcesIfNeeded_whenCitationTitleIsMissing_usesDomainLabel() { + List citations = List.of(webCitation( + 1, + null, + "https://www.example.com/articles/jna" + )); + + String text = AssistantSourceFormatter.appendCitationSourcesIfNeeded("Answer", citations); + + assertThat(text).contains("Sources:\n[1] [example.com]()"); + } + + @Test + @DisplayName("Structured Web citation sources are ordered by citation number") + void appendCitationSourcesIfNeeded_whenCitationsArriveOutOfOrder_ordersSourcesByNumber() { + List citations = List.of( + webCitation(2, "Second", "https://example.com/two"), + webCitation(1, "First", "https://example.com/one") + ); + + String text = AssistantSourceFormatter.appendCitationSourcesIfNeeded("Answer", citations); + + assertThat(text).containsSubsequence( + "[1] [First]()", + "[2] [Second]()" + ); + } + + @Test + @DisplayName("Structured Web citations do not duplicate labeled Markdown source references") + void appendCitationSourcesIfNeeded_whenAnswerAlreadyHasLabeledMarkdownSourceReference_keepsAnswer() { + List citations = List.of(webCitation( + 1, + "JNA GitHub", + "https://github.com/java-native-access/jna" + )); + String existing = "Answer [1]\n\n[1] [JNA GitHub]()"; + + String text = AssistantSourceFormatter.appendCitationSourcesIfNeeded(existing, citations); + + assertThat(text).isEqualTo(existing); + } + + @Test + @DisplayName("Structured Web citations with unsafe URLs are not appended") + void appendCitationSourcesIfNeeded_whenWebCitationUrlIsNotHttp_doesNotAppendSource() { + List citations = List.of(webCitation(1, "Unsafe", "javascript:alert(1)")); + + String text = AssistantSourceFormatter.appendCitationSourcesIfNeeded("Answer", citations); + + assertThat(text).isEqualTo("Answer"); + } + + @Test + @DisplayName("Structured non-Web citations are not appended as Web source URLs") + void appendCitationSourcesIfNeeded_whenCitationIsNotWeb_doesNotAppendSource() { + List citations = List.of(CitationRef.builder() + .number(1) + .kind(CitationKind.DOCUMENT_PAGE) + .title("doc.pdf") + .startPage(1L) + .build()); + + String text = AssistantSourceFormatter.appendCitationSourcesIfNeeded("Answer", citations); + + assertThat(text).isEqualTo("Answer"); + } + + @Test + @DisplayName("Structured Web citations do not duplicate existing source references") + void appendCitationSourcesIfNeeded_whenAnswerAlreadyHasSourceReferences_keepsAnswer() { + List citations = List.of(webCitation( + 1, + "JNA GitHub", + "https://github.com/java-native-access/jna" + )); + String existing = "Answer [1]\n\nSources:\n[1] https://github.com/java-native-access/jna"; + + String text = AssistantSourceFormatter.appendCitationSourcesIfNeeded(existing, citations); + + assertThat(text).isEqualTo(existing); + } + + @Test + @DisplayName("Sources are appended when an empty Sources heading ends before another section") + void appendCitationSourcesIfNeeded_whenEmptySourcesHeadingEndsBeforeUrl_appendsSources() { + List citations = List.of(webCitation( + 1, + "JNA GitHub", + "https://github.com/java-native-access/jna" + )); + String answer = "Answer\n\nSources:\n\n## More reading\nhttps://example.com/unrelated"; + + String text = AssistantSourceFormatter.appendCitationSourcesIfNeeded(answer, citations); + + assertThat(text).endsWith("Sources:\n[1] [JNA GitHub]()"); + } + + @Test + @DisplayName("Structured Web citations do not duplicate an existing provider Sources section") + void appendCitationSourcesIfNeeded_whenAnswerAlreadyHasSourcesSection_keepsAnswer() { + List citations = List.of(webCitation( + 1, + "JNA GitHub", + "https://github.com/java-native-access/jna" + )); + String existing = "Answer [1]\n\nSources:\n1. [JNA GitHub]()"; + + String text = AssistantSourceFormatter.appendCitationSourcesIfNeeded(existing, citations); + + assertThat(text).isEqualTo(existing); + } + + @Test + @DisplayName("Structured Web citations do not duplicate bold Sources headings with trailing colons") + void appendCitationSourcesIfNeeded_whenAnswerHasBoldSourcesHeadingWithColon_keepsAnswer() { + List citations = List.of(webCitation( + 1, + "JNA GitHub", + "https://github.com/java-native-access/jna" + )); + String existing = "Answer [1]\n\n**Sources**:\n- "; + + String text = AssistantSourceFormatter.appendCitationSourcesIfNeeded(existing, citations); + + assertThat(text).isEqualTo(existing); + } + + @Test + @DisplayName("Sources are appended when a bold heading ends an empty Sources section") + void appendCitationSourcesIfNeeded_whenEmptySourcesHeadingEndsBeforeBoldHeadingWithColon_appendsSources() { + List citations = List.of(webCitation( + 1, + "JNA GitHub", + "https://github.com/java-native-access/jna" + )); + String answer = "Answer\n\n**Sources**:\n\n**More reading**:\nhttps://example.com/unrelated"; + + String text = AssistantSourceFormatter.appendCitationSourcesIfNeeded(answer, citations); + + assertThat(text).endsWith("Sources:\n[1] [JNA GitHub]()"); + } + + @Test + @DisplayName("Structured Web citations do not duplicate existing Markdown source links") + void appendCitationSourcesIfNeeded_whenAnswerAlreadyHasMarkdownSourceLinks_keepsAnswer() { + List citations = List.of(webCitation( + 1, + "JNA GitHub", + "https://github.com/java-native-access/jna" + )); + String existing = "Answer [1]\n\nSources:\n[1](https://github.com/java-native-access/jna)"; + + String text = AssistantSourceFormatter.appendCitationSourcesIfNeeded(existing, citations); + + assertThat(text).isEqualTo(existing); + } + + private CitationRef webCitation(int number, String title, String url) { + return CitationRef.builder() + .number(number) + .kind(CitationKind.WEB) + .title(title) + .url(url) + .build(); + } +} diff --git a/src/test/java/com/github/drafael/chat4j/chat/ChatPanelTest.java b/src/test/java/com/github/drafael/chat4j/chat/ChatPanelTest.java index dca8bcd..dd1149e 100644 --- a/src/test/java/com/github/drafael/chat4j/chat/ChatPanelTest.java +++ b/src/test/java/com/github/drafael/chat4j/chat/ChatPanelTest.java @@ -835,242 +835,6 @@ void prepareAssistantResponse_whenConsultedSourceMode_doesNotScrapeAnswerLinks() } - @Test - @DisplayName("Native search activity adds one Sources section when citations arrive") - void mergeAssistantWebSearchWithAnswerSources_whenActivityHasSearchQuery_doesNotDuplicateSourcesHeading() throws Exception { - UUID conversationId = UUID.randomUUID(); - SendJob sendJob = webSearchSendJob( - 9L, - conversationId, - "OpenAI", - "gpt-5", - "https://api.openai.com/v1" - ); - List citations = List.of(CitationRef.builder() - .number(1) - .kind(CitationKind.WEB) - .title("OpenAI docs") - .url("https://platform.openai.com/docs") - .build()); - - String merged = invokeMergeAssistantWebSearchWithAnswerSources( - subject, - sendJob, - "Answer [1]", - "**Searched**\n- current OpenAI docs", - citations - ); - - assertThat(merged).contains("**Searched**", "**Sources**", "https://platform.openai.com/docs"); - assertThat(StringUtils.countMatches(merged, "**Sources**")).isEqualTo(1); - assertThat(merged).doesNotContain("Source URLs will appear here"); - } - - @Test - @DisplayName("Structured web citations append a Sources section when answer has no source references") - void appendCitationSourcesIfNeeded_whenAnswerLacksSourceReferences_appendsSources() throws Exception { - List citations = List.of(CitationRef.builder() - .number(1) - .kind(CitationKind.WEB) - .title("JNA GitHub") - .url("https://github.com/java-native-access/jna") - .build()); - - String text = invokeAppendCitationSourcesIfNeeded( - subject, - "JNA docs are at https://github.com/java-native-access/jna.", - citations - ); - - assertThat(text).contains("Sources:\n[1] [JNA GitHub]()"); - } - - @Test - @DisplayName("Structured web citation sources use the URL domain when title is missing") - void appendCitationSourcesIfNeeded_whenCitationTitleIsMissing_usesDomainLabel() throws Exception { - List citations = List.of(CitationRef.builder() - .number(1) - .kind(CitationKind.WEB) - .url("https://www.example.com/articles/jna") - .build()); - - String text = invokeAppendCitationSourcesIfNeeded(subject, "Answer", citations); - - assertThat(text).contains("Sources:\n[1] [example.com]()"); - } - - - - - - @Test - @DisplayName("Structured web citation sources are ordered by citation number") - void appendCitationSourcesIfNeeded_whenCitationsArriveOutOfOrder_ordersSourcesByNumber() throws Exception { - List citations = List.of( - CitationRef.builder() - .number(2) - .kind(CitationKind.WEB) - .title("Second") - .url("https://example.com/two") - .build(), - CitationRef.builder() - .number(1) - .kind(CitationKind.WEB) - .title("First") - .url("https://example.com/one") - .build() - ); - - String text = invokeAppendCitationSourcesIfNeeded(subject, "Answer", citations); - - assertThat(text).containsSubsequence( - "[1] [First]()", - "[2] [Second]()" - ); - } - - @Test - @DisplayName("Structured web citations do not duplicate existing labeled markdown source references") - void appendCitationSourcesIfNeeded_whenAnswerAlreadyHasLabeledMarkdownSourceReference_keepsAnswer() throws Exception { - List citations = List.of(CitationRef.builder() - .number(1) - .kind(CitationKind.WEB) - .title("JNA GitHub") - .url("https://github.com/java-native-access/jna") - .build()); - String existing = "Answer [1]\n\n[1] [JNA GitHub]()"; - - String text = invokeAppendCitationSourcesIfNeeded(subject, existing, citations); - - assertThat(text).isEqualTo(existing); - } - - @Test - @DisplayName("Structured web citations with unsafe URLs are not appended as source links") - void appendCitationSourcesIfNeeded_whenWebCitationUrlIsNotHttp_doesNotAppendSource() throws Exception { - List citations = List.of(CitationRef.builder() - .number(1) - .kind(CitationKind.WEB) - .title("Unsafe") - .url("javascript:alert(1)") - .build()); - - String text = invokeAppendCitationSourcesIfNeeded(subject, "Answer", citations); - - assertThat(text).isEqualTo("Answer"); - } - - @Test - @DisplayName("Structured non-web citations are not appended as web source URLs") - void appendCitationSourcesIfNeeded_whenCitationIsNotWeb_doesNotAppendSource() throws Exception { - List citations = List.of(CitationRef.builder() - .number(1) - .kind(CitationKind.DOCUMENT_PAGE) - .title("doc.pdf") - .startPage(1L) - .build()); - - String text = invokeAppendCitationSourcesIfNeeded(subject, "Answer", citations); - - assertThat(text).isEqualTo("Answer"); - } - - @Test - @DisplayName("Structured web citations do not duplicate existing source references") - void appendCitationSourcesIfNeeded_whenAnswerAlreadyHasSourceReferences_keepsAnswer() throws Exception { - List citations = List.of(CitationRef.builder() - .number(1) - .kind(CitationKind.WEB) - .title("JNA GitHub") - .url("https://github.com/java-native-access/jna") - .build()); - String existing = "Answer [1]\n\nSources:\n[1] https://github.com/java-native-access/jna"; - - String text = invokeAppendCitationSourcesIfNeeded(subject, existing, citations); - - assertThat(text).isEqualTo(existing); - } - - @Test - @DisplayName("Structured web citations append sources when an empty Sources heading is followed by another section") - void appendCitationSourcesIfNeeded_whenEmptySourcesHeadingEndsBeforeUrl_appendsSources() throws Exception { - List citations = List.of(CitationRef.builder() - .number(1) - .kind(CitationKind.WEB) - .title("JNA GitHub") - .url("https://github.com/java-native-access/jna") - .build()); - String answer = "Answer\n\nSources:\n\n## More reading\nhttps://example.com/unrelated"; - - String text = invokeAppendCitationSourcesIfNeeded(subject, answer, citations); - - assertThat(text).endsWith("Sources:\n[1] [JNA GitHub]()"); - } - - @Test - @DisplayName("Structured web citations do not duplicate existing provider Sources section") - void appendCitationSourcesIfNeeded_whenAnswerAlreadyHasSourcesSection_keepsAnswer() throws Exception { - List citations = List.of(CitationRef.builder() - .number(1) - .kind(CitationKind.WEB) - .title("JNA GitHub") - .url("https://github.com/java-native-access/jna") - .build()); - String existing = "Answer [1]\n\nSources:\n1. [JNA GitHub]()"; - - String text = invokeAppendCitationSourcesIfNeeded(subject, existing, citations); - - assertThat(text).isEqualTo(existing); - } - - @Test - @DisplayName("Structured web citations do not duplicate existing bold Sources headings with trailing colons") - void appendCitationSourcesIfNeeded_whenAnswerHasBoldSourcesHeadingWithColon_keepsAnswer() throws Exception { - List citations = List.of(CitationRef.builder() - .number(1) - .kind(CitationKind.WEB) - .title("JNA GitHub") - .url("https://github.com/java-native-access/jna") - .build()); - String existing = "Answer [1]\n\n**Sources**:\n- "; - - String text = invokeAppendCitationSourcesIfNeeded(subject, existing, citations); - - assertThat(text).isEqualTo(existing); - } - - @Test - @DisplayName("Structured web citations append sources when a bold heading ends an empty Sources section") - void appendCitationSourcesIfNeeded_whenEmptySourcesHeadingEndsBeforeBoldHeadingWithColon_appendsSources() throws Exception { - List citations = List.of(CitationRef.builder() - .number(1) - .kind(CitationKind.WEB) - .title("JNA GitHub") - .url("https://github.com/java-native-access/jna") - .build()); - String answer = "Answer\n\n**Sources**:\n\n**More reading**:\nhttps://example.com/unrelated"; - - String text = invokeAppendCitationSourcesIfNeeded(subject, answer, citations); - - assertThat(text).endsWith("Sources:\n[1] [JNA GitHub]()"); - } - - @Test - @DisplayName("Structured web citations do not duplicate existing markdown source links") - void appendCitationSourcesIfNeeded_whenAnswerAlreadyHasMarkdownSourceLinks_keepsAnswer() throws Exception { - List citations = List.of(CitationRef.builder() - .number(1) - .kind(CitationKind.WEB) - .title("JNA GitHub") - .url("https://github.com/java-native-access/jna") - .build()); - String existing = "Answer [1]\n\nSources:\n[1](https://github.com/java-native-access/jna)"; - - String text = invokeAppendCitationSourcesIfNeeded(subject, existing, citations); - - assertThat(text).isEqualTo(existing); - } - @Test @DisplayName("Composer is centered and constrained inside workspace") void layout_whenWidePanel_constrainsComposerWidth() throws Exception { @@ -8685,34 +8449,6 @@ private static boolean invokeNativeWebSearchEnabled(ChatPanel chatPanel, SendJob return (boolean) method.invoke(chatPanel, sendJob); } - private static String invokeAppendCitationSourcesIfNeeded( - ChatPanel chatPanel, - String assistantText, - List citations - ) throws Exception { - Method method = ChatPanel.class.getDeclaredMethod("appendCitationSourcesIfNeeded", String.class, List.class); - method.setAccessible(true); - return (String) method.invoke(chatPanel, assistantText, citations); - } - - private static String invokeMergeAssistantWebSearchWithAnswerSources( - ChatPanel chatPanel, - SendJob sendJob, - String assistantText, - String existingActivity, - List citations - ) throws Exception { - Method method = ChatPanel.class.getDeclaredMethod( - "mergeAssistantWebSearchWithAnswerSources", - SendJob.class, - String.class, - String.class, - List.class - ); - method.setAccessible(true); - return (String) method.invoke(chatPanel, sendJob, assistantText, existingActivity, citations); - } - private static SendJob deepSeekWebSearchSendJob(UUID conversationId) { return webSearchSendJob( 2L,