diff --git a/CHANGELOG.md b/CHANGELOG.md index 61ad09f..d594ae9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ All notable Doc2FAQ changes are recorded here. Version numbers follow semantic versioning; entries describe shipped source functionality and do not imply an external AI service. +## Unreleased + +### Fixed + +- Avoid excessive regex backtracking on whitespace-heavy input in both FAQ modes. +- Discard pending draft results and errors after the source, example, imported text, or mode changes. +- Preserve separate client and provider responsibility suggestions while still filtering duplicates for the same party. + ## 0.3.0 ### Added diff --git a/src/main/java/com/okansoftware/doc2faq/service/FaqDraftService.java b/src/main/java/com/okansoftware/doc2faq/service/FaqDraftService.java index f2d6a80..7b7c5e9 100644 --- a/src/main/java/com/okansoftware/doc2faq/service/FaqDraftService.java +++ b/src/main/java/com/okansoftware/doc2faq/service/FaqDraftService.java @@ -19,8 +19,8 @@ public class FaqDraftService { private static final int MAX_CHARACTERS = 50_000; private static final int MAX_ITEMS = 30; - private static final Pattern QUESTION = Pattern.compile( - "^(?:#{1,6}\\s+|(?:Q|Question):\\s*)(.+\\?)\\s*$", + private static final Pattern QUESTION_PREFIX = Pattern.compile( + "^(?:#{1,6}\\s+|(?:Q|Question):\\s*)", Pattern.CASE_INSENSITIVE); private static final Pattern ANSWER_PREFIX = Pattern.compile( "^(?:A|Answer):\\s*", Pattern.CASE_INSENSITIVE); @@ -42,10 +42,10 @@ public PreviewResponse preview(String content) { for (String rawLine : content.split("\\R", -1)) { String line = rawLine.strip(); - Matcher match = QUESTION.matcher(line); - if (match.matches()) { + String nextQuestion = questionFrom(line); + if (nextQuestion != null) { addIfValid(items, seen, question, answer); - question = match.group(1).strip(); + question = nextQuestion; answer.setLength(0); continue; } @@ -74,6 +74,16 @@ public PreviewResponse preview(String content) { return new PreviewResponse(List.copyOf(items), items.size(), NOTICE); } + private static String questionFrom(String line) { + if (!line.endsWith("?")) return null; + // Match only the prefix: overlapping whitespace and question captures can + // otherwise backtrack quadratically on a long line without a question mark. + Matcher prefix = QUESTION_PREFIX.matcher(line); + if (!prefix.find()) return null; + String question = line.substring(prefix.end()).strip(); + return question.length() > 1 ? question : null; + } + private static void addIfValid(List items, Set seen, String question, StringBuilder answer) { if (question == null || answer.isEmpty() || items.size() >= MAX_ITEMS) { diff --git a/src/main/java/com/okansoftware/doc2faq/service/FaqGenerateService.java b/src/main/java/com/okansoftware/doc2faq/service/FaqGenerateService.java index 1ecbae9..88447db 100644 --- a/src/main/java/com/okansoftware/doc2faq/service/FaqGenerateService.java +++ b/src/main/java/com/okansoftware/doc2faq/service/FaqGenerateService.java @@ -156,7 +156,7 @@ private static Topic topicForHeading(String heading) { if (h.contains("terminat") || h.contains("cancellat")) return Topic.TERMINATION; if (h.contains("payment") || h.contains("invoic")) return Topic.PAYMENT; if (h.contains("fee") || h.contains("pricing") || h.contains("cost")) return Topic.FEES; - if (h.contains("responsibilit") || h.contains("obligation")) return Topic.RESPONSIBILITIES; + if (h.contains("responsibilit") || h.contains("obligation")) return responsibilityTopic(h); if (h.contains("ownership") || h.contains("intellectual property")) return Topic.OWNERSHIP; if (h.contains("confidential")) return Topic.CONFIDENTIALITY; if (h.contains("scope") || h.contains("service")) return Topic.SERVICES; @@ -178,7 +178,7 @@ private static Topic topicForQuestion(String question) { || q.contains("invoice") || q.contains("how are payments")) return Topic.PAYMENT; if (q.contains("fee") || q.contains("pricing") || q.contains("price") || q.contains("cost")) return Topic.FEES; - if (q.contains("responsibilit") || q.contains("obligation")) return Topic.RESPONSIBILITIES; + if (q.contains("responsibilit") || q.contains("obligation")) return responsibilityTopic(q); if (q.contains("ownership") || q.contains("owns the work") || q.contains("deliverables")) return Topic.OWNERSHIP; if (q.contains("confidential")) return Topic.CONFIDENTIALITY; @@ -194,6 +194,14 @@ private static Topic topicForQuestion(String question) { return Topic.OTHER; } + private static Topic responsibilityTopic(String text) { + // Separate parties so an explicit or generated client FAQ cannot hide + // the provider's distinct obligations (and vice versa). + if (text.contains("client") && !text.contains("provider")) return Topic.CLIENT_RESPONSIBILITIES; + if (text.contains("provider") && !text.contains("client")) return Topic.PROVIDER_RESPONSIBILITIES; + return Topic.RESPONSIBILITIES; + } + private static String questionFor(String heading) { String h = heading.toLowerCase(Locale.ROOT); if (h.contains("termination") || h.contains("cancellation")) @@ -231,6 +239,7 @@ private static String questionFor(String heading) { private enum Topic { SERVICES, TERM, FEES, PAYMENT, TERMINATION, RESPONSIBILITIES, + CLIENT_RESPONSIBILITIES, PROVIDER_RESPONSIBILITIES, OWNERSHIP, CONFIDENTIALITY, SUMMARY, SKILLS, EXPERIENCE, EDUCATION, OTHER } diff --git a/src/main/resources/static/app.js b/src/main/resources/static/app.js index 7bf9b8a..f3e20e2 100644 --- a/src/main/resources/static/app.js +++ b/src/main/resources/static/app.js @@ -44,6 +44,7 @@ const formatSelect = $("export-format"); const addButton = $("add-item-btn"); const modeSelect = $("draft-mode"); let items = []; +let draftRevision = 0; function setStatus(message, kind = "") { status.textContent = message; @@ -231,6 +232,7 @@ function exportData() { } function clearDraft() { + draftRevision++; items = []; search.value = ""; render(); @@ -333,7 +335,11 @@ draftButton.addEventListener("click", async () => { return; } - const generating = selectedMode() === "generate"; + const mode = selectedMode(); + const generating = mode === "generate"; + const revision = draftRevision; + const isCurrentDraft = () => revision === draftRevision + && source.value === content && selectedMode() === mode; draftButton.disabled = true; draftButton.textContent = generating ? "Generating draft…" : "Extracting pairs…"; setStatus(generating ? "Building source-grounded FAQ suggestions…" : "Extracting explicit Q&A pairs…"); @@ -344,6 +350,7 @@ draftButton.addEventListener("click", async () => { body: JSON.stringify({content}) }); const data = await response.json().catch(() => ({})); + if (!isCurrentDraft()) return; if (!response.ok) throw new Error(data.message || "Could not create an FAQ draft."); items = Array.isArray(data.items) ? data.items.filter((item) => @@ -357,7 +364,9 @@ draftButton.addEventListener("click", async () => { : "No complete Q&A pairs found. Switch to Generate Mode for ordinary documents.", items.length ? "success" : ""); } catch (error) { - setStatus(error.message || "The FAQ service is unavailable.", "error"); + if (isCurrentDraft()) { + setStatus(error.message || "The FAQ service is unavailable.", "error"); + } } finally { draftButton.disabled = false; updateModeText(); diff --git a/src/test/java/com/okansoftware/doc2faq/service/FaqDraftServiceTest.java b/src/test/java/com/okansoftware/doc2faq/service/FaqDraftServiceTest.java index d8ce46e..171d0a4 100644 --- a/src/test/java/com/okansoftware/doc2faq/service/FaqDraftServiceTest.java +++ b/src/test/java/com/okansoftware/doc2faq/service/FaqDraftServiceTest.java @@ -1,6 +1,7 @@ package com.okansoftware.doc2faq.service; import com.okansoftware.doc2faq.model.PreviewResponse; +import java.time.Duration; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.*; @@ -87,4 +88,38 @@ void capsNumberOfExtractedPairs() { } assertEquals(30, service.preview(input.toString()).count()); } + + @Test + void handlesLongWhitespacePrefixesWithoutExcessiveBacktracking() { + assertTimeoutPreemptively(Duration.ofSeconds(2), () -> { + for (String prefix : new String[]{"Q:", "Question:", "######"}) { + String whitespace = " ".repeat(49_900); + assertEquals(0, service.preview(prefix + whitespace + "x").count()); + PreviewResponse result = service.preview(prefix + whitespace + "Why?\nA: Yes."); + assertEquals(1, result.count()); + assertEquals("Why?", result.items().getFirst().question()); + } + }); + } + + @Test + void preservesSupportedQuestionPrefixesAndRejectsUnrelatedLines() { + PreviewResponse result = service.preview(""" + q:Why now? + A: Today. + qUeStIoN:\tWhere next? + A: Fairfax. + ######\tWho decides? + A: The team. + # Unrelated heading + ####### Invalid heading? + Ignored answer. + An ordinary question? + Ignored answer. + """); + assertEquals(3, result.count()); + assertEquals("Why now?", result.items().get(0).question()); + assertEquals("Where next?", result.items().get(1).question()); + assertEquals("Who decides?", result.items().get(2).question()); + } } diff --git a/src/test/java/com/okansoftware/doc2faq/service/FaqGenerateServiceTest.java b/src/test/java/com/okansoftware/doc2faq/service/FaqGenerateServiceTest.java index 9c9d95f..e47bda0 100644 --- a/src/test/java/com/okansoftware/doc2faq/service/FaqGenerateServiceTest.java +++ b/src/test/java/com/okansoftware/doc2faq/service/FaqGenerateServiceTest.java @@ -1,6 +1,7 @@ package com.okansoftware.doc2faq.service; import com.okansoftware.doc2faq.model.PreviewResponse; +import java.time.Duration; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.*; @@ -200,4 +201,52 @@ void usesNaturalQuestionsForResponsibilitiesAndOwnership() { result.items().get(1).answer()); } + @Test + void retainsClientAndProviderResponsibilitiesInEitherOrder() { + for (boolean clientFirst : new boolean[]{true, false}) { + String client = "## Client Responsibilities\nThe client supplies backups.\n"; + String provider = "## Provider Responsibilities\nThe provider patches software.\n"; + PreviewResponse result = service.generate(clientFirst ? client + provider : provider + client); + assertEquals(2, result.count()); + assertTrue(result.items().stream().anyMatch(item -> + item.question().contains("client") && item.answer().equals("The client supplies backups."))); + assertTrue(result.items().stream().anyMatch(item -> + item.question().contains("provider") && item.answer().equals("The provider patches software."))); + } + } + + @Test + void explicitClientFaqOnlySuppressesDuplicateClientSuggestions() { + PreviewResponse result = service.generate(""" + ## Client Responsibilities + The client supplies backups. + ## Provider Responsibilities + The provider patches software. + ## FAQ + Q: What responsibilities does the client have under the agreement? + A: The client retains backups. + """); + assertEquals(2, result.count()); + assertEquals("The client retains backups.", result.items().getFirst().answer()); + assertEquals("What responsibilities does the provider have under the agreement?", + result.items().get(1).question()); + assertEquals("The provider patches software.", result.items().get(1).answer()); + } + + @Test + void stillDeduplicatesRepeatedResponsibilitiesForTheSameParty() { + PreviewResponse result = service.generate(""" + ## Provider Responsibilities + The provider patches software. + ## Provider Obligations + The provider monitors uptime. + """); + assertEquals(1, result.count()); + } + + @Test + void generationAlsoHandlesWhitespaceHeavyInputPromptly() { + assertTimeoutPreemptively(Duration.ofSeconds(2), () -> + assertEquals(1, service.generate("Q:" + " ".repeat(49_990) + "x").count())); + } } diff --git a/src/test/js/app-requests.test.cjs b/src/test/js/app-requests.test.cjs new file mode 100644 index 0000000..c83880a --- /dev/null +++ b/src/test/js/app-requests.test.cjs @@ -0,0 +1,128 @@ +const {readFileSync} = require("node:fs"); +const path = require("node:path"); +const test = require("node:test"); +const assert = require("node:assert/strict"); +const vm = require("node:vm"); + +const source = readFileSync(path.resolve(__dirname, "../../main/resources/static/app.js"), "utf8"); +const withoutBoot = source.replace(/updateModeText\(\);\s*updateCount\(\);\s*render\(\);\s*$/, ""); + +function pendingDraft() { + const elements = new Map(); + function element(id) { + if (!elements.has(id)) elements.set(id, { + value: "", disabled: false, textContent: "", handlers: {}, + addEventListener(type, handler) { this.handlers[type] = handler; }, + focus() {} + }); + return elements.get(id); + } + let complete; + let fail; + const response = new Promise((resolve, reject) => { complete = resolve; fail = reject; }); + const context = {document: {getElementById: element}, fetch: () => response}; + // Keep the actual event handlers and export controls; these tests do not need card layout. + vm.runInNewContext(withoutBoot + "\nrender = () => updateControls();", context); + element("source-text").value = "Q: Old question?\nA: Old answer."; + element("draft-mode").value = "extract"; + const finished = element("generate-btn").handlers.click(); + return { + element, finished, + fail() { fail(new Error("Network unavailable.")); }, + complete(ok = true) { + complete({ok, json: async () => ({ + items: [{question: "Old question?", answer: "Old answer."}], + message: "Old request failed." + })}); + } + }; +} + +test("a response for unchanged source enables export", async () => { + const draft = pendingDraft(); + draft.complete(); + await draft.finished; + assert.equal(draft.element("faq-count").textContent, "1 ITEM"); + assert.equal(draft.element("copy-btn").disabled, false); + assert.equal(draft.element("generate-btn").disabled, false); +}); + +test("editing the source discards an in-flight response even if the text is restored", async () => { + const draft = pendingDraft(); + const input = draft.element("source-text"); + const original = input.value; + input.value = "A different document."; + input.handlers.input(); + input.value = original; + input.handlers.input(); + draft.complete(); + await draft.finished; + assert.equal(draft.element("faq-count").textContent, "0 ITEMS"); + assert.equal(draft.element("copy-btn").disabled, true); + assert.equal(draft.element("download-btn").disabled, true); + assert.equal(draft.element("generate-btn").disabled, false); +}); + +test("switching modes discards results and preserves the new mode status", async () => { + const draft = pendingDraft(); + const mode = draft.element("draft-mode"); + mode.value = "generate"; + mode.handlers.change(); + const status = draft.element("status").textContent; + draft.complete(); + await draft.finished; + assert.equal(draft.element("faq-count").textContent, "0 ITEMS"); + assert.equal(draft.element("copy-btn").disabled, true); + assert.equal(draft.element("status").textContent, status); +}); + +test("loading an example invalidates the previous request", async () => { + const draft = pendingDraft(); + draft.element("sample-btn").handlers.click(); + draft.complete(); + await draft.finished; + assert.equal(draft.element("faq-count").textContent, "0 ITEMS"); + assert.equal(draft.element("copy-btn").disabled, true); +}); + +test("an outdated failure cannot overwrite the current mode status", async () => { + const draft = pendingDraft(); + draft.element("draft-mode").value = "generate"; + draft.element("draft-mode").handlers.change(); + const status = draft.element("status").textContent; + draft.complete(false); + await draft.finished; + assert.equal(draft.element("status").textContent, status); + assert.equal(draft.element("generate-btn").disabled, false); +}); + +test("importing replacement text invalidates the previous request", async () => { + const draft = pendingDraft(); + const input = draft.element("file-input"); + input.files = [{name: "replacement.txt", size: 30, text: async () => "New imported document."}]; + await input.handlers.change({target: input}); + draft.complete(); + await draft.finished; + assert.equal(draft.element("source-text").value, "New imported document."); + assert.equal(draft.element("faq-count").textContent, "0 ITEMS"); + assert.equal(draft.element("copy-btn").disabled, true); +}); + +test("an outdated network failure cannot replace the current status", async () => { + const draft = pendingDraft(); + draft.element("sample-btn").handlers.click(); + const status = draft.element("status").textContent; + draft.fail(); + await draft.finished; + assert.equal(draft.element("status").textContent, status); + assert.equal(draft.element("generate-btn").disabled, false); +}); + +test("a current network failure remains visible and allows retry", async () => { + const draft = pendingDraft(); + draft.fail(); + await draft.finished; + assert.equal(draft.element("status").textContent, "Network unavailable."); + assert.equal(draft.element("status").className, "status error"); + assert.equal(draft.element("generate-btn").disabled, false); +});