DEV: Improve explain suggestion footnote replacement (#999)

Previously, when clicking add footnote on an explain suggestion it would replace the selected word by finding the first occurrence of the word. This results in issues when there are more than one occurrences of a word in a post. This is not trivial to solve, so this PR instead prevents incorrect text replacements by only allowing the replacement if it's unique. We use the same logic here that we use to determine if something can be fast edited.

In this PR we also update tests for post helper explain suggestions. For a while, we haven't had tests here due to streaming/timing issues, we've been skipping our system specs. In this PR, we add acceptance tests to handle this which gives us improved ability to publish message bus updates in the testing environment so that it can be better tested without issues.

Author: keegangeorge

assets/javascripts/discourse/components/ai-post-helper-menu.gjs

@@ -4,6 +4,7 @@ import { action } from "@ember/object";
 import didInsert from "@ember/render-modifiers/modifiers/did-insert";
 import willDestroy from "@ember/render-modifiers/modifiers/will-destroy";
 import { service } from "@ember/service";
+import { modifier } from "ember-modifier";
 import { and } from "truth-helpers";
 import CookText from "discourse/components/cook-text";
 import DButton from "discourse/components/d-button";
@@ -26,6 +27,7 @@ export default class AiPostHelperMenu extends Component {
   @service siteSettings;
   @service currentUser;
   @service menu;
+  @service tooltip;
 
   @tracked menuState = this.MENU_STATES.options;
   @tracked loading = false;
@@ -38,15 +40,39 @@ export default class AiPostHelperMenu extends Component {
   @tracked streaming = false;
   @tracked lastSelectedOption = null;
   @tracked isSavingFootnote = false;
+  @tracked supportsAddFootnote = this.args.data.supportsFastEdit;
 
   MENU_STATES = {
     options: "OPTIONS",
     loading: "LOADING",
     result: "RESULT",
   };
 
+  showFootnoteTooltip = modifier((element) => {
+    if (this.supportsAddFootnote || this.streaming) {
+      return;
+    }
+
+    const instance = this.tooltip.register(element, {
+      identifier: "cannot-add-footnote-tooltip",
+      content: I18n.t(
+        "discourse_ai.ai_helper.post_options_menu.footnote_disabled"
+      ),
+      placement: "top",
+      triggers: "hover",
+    });
+
+    return () => {
+      instance.destroy();
+    };
+  });
+
   @tracked _activeAiRequest = null;
 
+  get footnoteDisabled() {
+    return this.streaming || !this.supportsAddFootnote;
+  }
+
   get helperOptions() {
     let prompts = this.currentUser?.ai_helper_prompts;
 
@@ -329,8 +355,9 @@ export default class AiPostHelperMenu extends Component {
                     @label="discourse_ai.ai_helper.post_options_menu.insert_footnote"
                     @action={{this.insertFootnote}}
                     @isLoading={{this.isSavingFootnote}}
-                    @disabled={{this.streaming}}
+                    @disabled={{this.footnoteDisabled}}
                     class="btn-flat ai-post-helper__suggestion__insert-footnote"
+                    {{this.showFootnoteTooltip}}
                   />
                 {{/if}}
               </div>

assets/javascripts/discourse/connectors/post-text-buttons/ai-post-helper-trigger.gjs

@@ -135,6 +135,7 @@ export default class AiPostHelperTrigger extends Component {
         identifier: "ai-post-helper-menu",
         component: AiPostHelperMenu,
         inline: true,
+        interactive: true,
         placement: this.shouldRenderUnder ? "bottom-start" : "top-start",
         fallbackPlacements: this.shouldRenderUnder
           ? ["bottom-end", "top-start"]

config/locales/client.en.yml

@@ -145,7 +145,6 @@ en:
         response_tokens: "Response tokens"
         cached_tokens: "Cached tokens"
 
-
       ai_persona:
         tool_strategies:
           all: "Apply to all replies"
@@ -395,6 +394,7 @@ en:
           copied: "Copied!"
           cancel: "Cancel"
           insert_footnote: "Add footnote"
+          footnote_disabled: "Automatic insertion disabled, click copy button and edit it in manually"
           footnote_credits: "Explanation by AI"
         fast_edit:
           suggest_button: "Suggest edit"

spec/system/ai_helper/ai_post_helper_spec.rb

@@ -79,77 +79,6 @@ def select_post_text(selected_post)
       expect(post.like_count).to eq(1)
     end
 
-    context "when using explain mode" do
-      let(:mode) { CompletionPrompt::EXPLAIN }
-
-      let(:explain_response) { <<~STRING }
-        In this context, pie refers to a baked dessert typically consisting of a pastry crust and filling.
-        The person states they enjoy eating pie, considering it a good dessert. They note that some people wastefully
-        throw pie at others, but the person themselves chooses to eat the pie rather than throwing it. Overall, pie
-        is being used to refer the the baked dessert food item.
-      STRING
-
-      skip "TODO: Streaming causing timing issue in test" do
-        it "shows an explanation of the selected text" do
-          select_post_text(post)
-          post_ai_helper.click_ai_button
-
-          DiscourseAi::Completions::Llm.with_prepared_responses([explain_response]) do
-            expected_value = explain_response.gsub(/"/, "").strip
-
-            post_ai_helper.select_helper_model(mode)
-            Jobs.run_immediately!
-
-            wait_for(timeout: 10) do
-              post_ai_helper.suggestion_value.gsub(/"/, "").strip == expected_value
-            end
-
-            expect(post_ai_helper.suggestion_value.gsub(/"/, "").strip).to eq(expected_value)
-          end
-        end
-
-        it "adds explained text as footnote to post" do
-          select_post_text(post)
-          post_ai_helper.click_ai_button
-
-          DiscourseAi::Completions::Llm.with_prepared_responses([explain_response]) do
-            expected_value = explain_response.gsub(/"/, "").strip
-
-            post_ai_helper.select_helper_model(mode)
-            Jobs.run_immediately!
-
-            wait_for(timeout: 10) do
-              post_ai_helper.suggestion_value.gsub(/"/, "").strip == expected_value
-            end
-
-            post_ai_helper.click_add_footnote
-            expect(page.has_css?(".expand-footnote")).to eq(true)
-          end
-        end
-      end
-    end
-
-    context "when using translate mode" do
-      let(:mode) { CompletionPrompt::TRANSLATE }
-
-      let(:translated_input) { "The rain in Spain, stays mainly in the Plane." }
-
-      skip "TODO: Streaming causing timing issue in test" do
-        it "shows a translation of the selected text" do
-          select_post_text(post_2)
-          post_ai_helper.click_ai_button
-
-          DiscourseAi::Completions::Llm.with_prepared_responses([translated_input]) do
-            post_ai_helper.select_helper_model(mode)
-
-            wait_for { post_ai_helper.suggestion_value == translated_input }
-
-            expect(post_ai_helper.suggestion_value).to eq(translated_input)
-          end
-        end
-      end
-    end
-
     context "when using proofread mode" do
       let(:mode) { CompletionPrompt::PROOFREAD }
       let(:proofread_response) do

test/javascripts/acceptance/post-helper-menu-test.js

@@ -0,0 +1,116 @@
+import { click, settled, visit } from "@ember/test-helpers";
+import { test } from "qunit";
+import { AUTO_GROUPS } from "discourse/lib/constants";
+import topicFixtures from "discourse/tests/fixtures/topic";
+import {
+  acceptance,
+  publishToMessageBus,
+  query,
+  selectText,
+} from "discourse/tests/helpers/qunit-helpers";
+import { cloneJSON } from "discourse-common/lib/object";
+import aiHelperPrompts from "../fixtures/ai-helper-prompts";
+
+acceptance("AI Helper - Post Helper Menu", function (needs) {
+  needs.settings({
+    discourse_ai_enabled: true,
+    ai_helper_enabled: true,
+    post_ai_helper_allowed_groups: "1|2",
+    ai_helper_enabled_features: "suggestions|context_menu",
+    share_quote_visibility: "anonymous",
+    enable_markdown_footnotes: true,
+    display_footnotes_inline: true,
+  });
+  needs.user({
+    admin: true,
+    moderator: true,
+    groups: [AUTO_GROUPS.admins],
+    can_use_assistant_in_post: true,
+    ai_helper_prompts: aiHelperPrompts,
+    trust_level: 4,
+  });
+  needs.pretender((server, helper) => {
+    server.get("/t/1.json", () => {
+      const json = cloneJSON(topicFixtures["/t/28830/1.json"]);
+      json.post_stream.posts[0].can_edit_post = true;
+      json.post_stream.posts[0].can_edit = true;
+      return helper.response(json);
+    });
+
+    server.get("/t/2.json", () => {
+      const json = cloneJSON(topicFixtures["/t/28830/1.json"]);
+      json.post_stream.posts[0].cooked =
+        "<p>La lluvia en España se queda principalmente en el avión.</p>";
+      return helper.response(json);
+    });
+
+    server.post(`/discourse-ai/ai-helper/stream_suggestion/`, () => {
+      return helper.response({
+        result: "This is a suggestio",
+        done: false,
+      });
+    });
+  });
+
+  test("displays streamed explanation", async function (assert) {
+    await visit("/t/-/1");
+    const suggestion = "This is a suggestion that is completed";
+    const textNode = query("#post_1 .cooked p").childNodes[0];
+    await selectText(textNode, 9);
+    await click(".ai-post-helper__trigger");
+    await click(".ai-helper-options__button[data-name='explain']");
+    await publishToMessageBus(
+      `/discourse-ai/ai-helper/stream_suggestion/118591`,
+      {
+        done: true,
+        result: suggestion,
+      }
+    );
+    assert.dom(".ai-post-helper__suggestion__text").hasText(suggestion);
+  });
+
+  async function selectSpecificText(textNode, start, end) {
+    const range = document.createRange();
+    range.setStart(textNode, start);
+    range.setEnd(textNode, end);
+    const selection = window.getSelection();
+    selection.removeAllRanges();
+    selection.addRange(range);
+    await settled();
+  }
+
+  test("adds explained text as footnote to post", async function (assert) {
+    await visit("/t/-/1");
+    const suggestion = "This is a suggestion that is completed";
+
+    const textNode = query("#post_1 .cooked p").childNodes[0];
+    await selectSpecificText(textNode, 72, 77);
+    await click(".ai-post-helper__trigger");
+    await click(".ai-helper-options__button[data-name='explain']");
+    await publishToMessageBus(
+      `/discourse-ai/ai-helper/stream_suggestion/118591`,
+      {
+        done: true,
+        result: suggestion,
+      }
+    );
+
+    assert.dom(".ai-post-helper__suggestion__insert-footnote").isDisabled();
+  });
+
+  test("shows translated post", async function (assert) {
+    await visit("/t/-/2");
+    const translated = "The rain in Spain, stays mainly in the Plane.";
+    await selectText(query("#post_1 .cooked p"));
+    await click(".ai-post-helper__trigger");
+    await click(".ai-helper-options__button[data-name='translate']");
+    await publishToMessageBus(
+      `/discourse-ai/ai-helper/stream_suggestion/118591`,
+      {
+        done: true,
+        result: translated,
+      }
+    );
+    assert.dom(".ai-post-helper__suggestion__text").hasText(translated);
+  });
+});

test/javascripts/fixtures/ai-helper-prompts.js

@@ -0,0 +1,66 @@
+export default [
+  {
+    id: -301,
+    name: "translate",
+    translated_name: "Translate to English (US)",
+    prompt_type: "text",
+    icon: "language",
+    location: ["composer", "post"],
+  },
+  {
+    id: -303,
+    name: "proofread",
+    translated_name: "Proofread text",
+    prompt_type: "diff",
+    icon: "spell-check",
+    location: ["composer", "post"],
+  },
+  {
+    id: -304,
+    name: "markdown_table",
+    translated_name: "Generate Markdown table",
+    prompt_type: "diff",
+    icon: "table",
+    location: ["composer"],
+  },
+  {
+    id: -305,
+    name: "custom_prompt",
+    translated_name: "Custom Prompt",
+    prompt_type: "diff",
+    icon: "comment",
+    location: ["composer", "post"],
+  },
+  {
+    id: -306,
+    name: "explain",
+    translated_name: "Explain",
+    prompt_type: "text",
+    icon: "question",
+    location: ["post"],
+  },
+  {
+    id: -307,
+    name: "generate_titles",
+    translated_name: "Suggest topic titles",
+    prompt_type: "list",
+    icon: "heading",
+    location: ["composer"],
+  },
+  {
+    id: -308,
+    name: "illustrate_post",
+    translated_name: "Illustrate Post",
+    prompt_type: "list",
+    icon: "images",
+    location: ["composer"],
+  },
+  {
+    id: -309,
+    name: "detect_text_locale",
+    translated_name: "detect_text_locale",
+    prompt_type: "text",
+    icon: null,
+    location: [],
+  },
+];