FEATURE: Regenerate outdated summaries. (#22718)

Users unable to generate new summaries won't be able to regenerate them. They'll only see the warning saying it's outdated.

app/assets/javascripts/discourse/app/widgets/summary-box.js

@@ -50,54 +50,86 @@ export default createWidget("summary-box", {
   html(attrs) {
     const html = [];
 
-    if (attrs.summary) {
+    const summary = attrs.summary;
+
+    if (summary && !attrs.skipAgeCheck) {
       html.push(
         new RawHtml({
-          html: `<div class="generated-summary">${attrs.summary}</div>`,
+          html: `<div class="generated-summary">${summary.summarized_text}</div>`,
         })
       );
-      html.push(
+
-        h("div.summarized-on", {}, [
+      const summarizationInfo = [
-          new RenderGlimmer(
+        h("p", {}, [
-            this,
+          I18n.t("summary.summarized_on", { date: summary.summarized_on }),
-            "div",
+          this.buildTooltip(attrs),
-            hbs`{{@data.summarizedOn}}
+        ]),
-            {{d-icon "info-circle"}}
+      ];
-            <DTooltip @placement="top-end">
+
-              {{i18n "summary.model_used" model=@data.attrs.summarizedBy}}
+      if (summary.outdated) {
-            </DTooltip>`,
+        summarizationInfo.push(this.outdatedSummaryWarning(attrs));
-            {
+      }
-              attrs,
+
-              summarizedOn: I18n.t("summary.summarized_on", {
+      html.push(h("div.summarized-on", {}, summarizationInfo));
-                date: attrs.summarizedOn,
-              }),
-            }
-          ),
-        ])
-      );
     } else {
       html.push(this.attach("summary-skeleton"));
-      this.fetchSummary(attrs.topicId);
+      this.fetchSummary(attrs.topicId, attrs.skipAgeCheck);
     }
 
     return html;
   },
 
-  showFullSummarizedOn() {
+  buildTooltip(attrs) {
-    this.state.expandSummarizedOn = true;
+    return new RenderGlimmer(
-    this.scheduleRerender();
+      this,
+      "span",
+      hbs`{{d-icon "info-circle"}}<DTooltip @placement="top-end">
+        {{i18n "summary.model_used" model=@data.summarizedBy}}
+      </DTooltip>`,
+      {
+        summarizedBy: attrs.summary.summarized_by,
+      }
+    );
   },
 
-  fetchSummary(topicId) {
+  outdatedSummaryWarning(attrs) {
-    ajax(`/t/${topicId}/strategy-summary`)
+    let outdatedText = I18n.t("summary.outdated");
+
+    if (
+      !attrs.hasTopRepliesSummary &&
+      attrs.summary.new_posts_since_summary > 0
+    ) {
+      outdatedText += " ";
+      outdatedText += I18n.t("summary.outdated_posts", {
+        count: attrs.summary.new_posts_since_summary,
+      });
+    }
+
+    return h("p.outdated-summary", {}, [
+      outdatedText,
+      iconNode("exclamation-triangle", { class: "info-icon" }),
+    ]);
+  },
+
+  fetchSummary(topicId, skipAgeCheck) {
+    let fetchURL = `/t/${topicId}/strategy-summary`;
+
+    if (skipAgeCheck) {
+      fetchURL += "?skip_age_check=true";
+    }
+
+    ajax(fetchURL)
       .then((data) => {
         cookAsync(data.summary).then((cooked) => {
           // We store the summary in the parent so we can re-render it without doing a new request.
-          this.sendWidgetEvent("summaryUpdated", {
+          data.summarized_text = cooked.string;
-            summary: cooked.string,
+          data.summarized_on = shortDateNoYear(data.summarized_on);
-            summarizedOn: shortDateNoYear(data.summarized_on),
+
-            summarizedBy: data.summarized_by,
+          if (skipAgeCheck) {
-          });
+            data.regenerated = true;
+          }
+
+          this.sendWidgetEvent("summaryUpdated", data);
         });
       })
       .catch(popupAjaxError);

app/assets/javascripts/discourse/app/widgets/toggle-topic-summary.js

@@ -51,22 +51,17 @@ export default createWidget("toggle-topic-summary", {
     const summarizationButtons = [];
 
     if (attrs.summarizable) {
-      const expandTitle = I18n.t("summary.strategy.button_title");
+      const canRegenerate =
-      const collapseTitle = I18n.t("summary.strategy.hide_button_title");
+        !state.regenerate &&
-      const canCollapse = !this.loadingSummary() && this.summaryBoxVisble();
+        state.summary.outdated &&
+        state.summary.can_regenerate;
+      const canCollapse =
+        !canRegenerate && !this.loadingSummary() && this.summaryBoxVisble();
+      const summarizeButton = canCollapse
+        ? this.hideSummaryButton()
+        : this.generateSummaryButton(canRegenerate);
 
-      summarizationButtons.push(
+      summarizationButtons.push(summarizeButton);
-        this.attach("button", {
-          className: "btn btn-primary topic-strategy-summarization",
-          icon: canCollapse ? "chevron-up" : "magic",
-          translatedTitle: canCollapse ? collapseTitle : expandTitle,
-          translatedLabel: canCollapse ? collapseTitle : expandTitle,
-          action: state.expandSummaryBox
-            ? "toggleSummaryBox"
-            : "expandSummaryBox",
-          disabled: this.loadingSummary(),
-        })
-      );
     }
 
     if (attrs.hasTopRepliesSummary) {
@@ -89,23 +84,55 @@ export default createWidget("toggle-topic-summary", {
     }
 
     if (this.summaryBoxVisble()) {
-      attrs.summary = this.state.summary;
+      attrs.summary = state.summary;
-      attrs.summarizedOn = this.state.summarizedOn;
+      attrs.skipAgeCheck = state.regenerate;
-      attrs.summarizedBy = this.state.summarizedBy;
+
       html.push(this.attach("summary-box", attrs));
     }
 
     return html;
   },
 
-  loadingSummary() {
+  generateSummaryButton(canRegenerate) {
-    return this.summaryBoxVisble() && !this.state.summary;
+    const title = canRegenerate
+      ? "summary.buttons.regenerate"
+      : "summary.buttons.generate";
+    const icon = canRegenerate ? "sync" : "magic";
+
+    return this.attach("button", {
+      className: "btn btn-primary topic-strategy-summarization",
+      icon,
+      title: I18n.t(title),
+      translatedTitle: I18n.t(title),
+      translatedLabel: I18n.t(title),
+      action: canRegenerate ? "regenerateSummary" : "expandSummaryBox",
+      disabled: this.loadingSummary(),
+    });
   },
 
-  summaryUpdatedEvent(update) {
+  hideSummaryButton() {
-    this.state.summary = update.summary;
+    return this.attach("button", {
-    this.state.summarizedOn = update.summarizedOn;
+      className: "btn btn-primary topic-strategy-summarization",
-    this.state.summarizedBy = update.summarizedBy;
+      icon: "chevron-up",
+      title: "summary.buttons.hide",
+      label: "summary.buttons.hide",
+      action: "toggleSummaryBox",
+      disabled: this.loadingSummary(),
+    });
+  },
+
+  loadingSummary() {
+    return (
+      this.summaryBoxVisble() && (!this.state.summary || this.state.regenerate)
+    );
+  },
+
+  summaryUpdatedEvent(summary) {
+    this.state.summary = summary;
+
+    if (summary.regenerated) {
+      this.state.regenerate = false;
+    }
   },
 
   summaryBoxVisble() {
@@ -117,6 +144,10 @@ export default createWidget("toggle-topic-summary", {
     this.state.summaryBoxHidden = false;
   },
 
+  regenerateSummary() {
+    this.state.regenerate = true;
+  },
+
   toggleSummaryBox() {
     this.state.summaryBoxHidden = !this.state.summaryBoxHidden;
   },

app/assets/stylesheets/common/base/topic-summary.scss

@@ -30,9 +30,13 @@
     .summarized-on {
       text-align: right;
 
-      .model-used {
+      .info-icon {
         margin-left: 3px;
       }
     }
+
+    .outdated-summary {
+      color: var(--primary-medium);
+    }
   }
 }

app/controllers/topics_controller.rb

@@ -1179,13 +1179,19 @@ class TopicsController < ApplicationController
 
     RateLimiter.new(current_user, "summary", 6, 5.minutes).performed! if current_user
 
+    opts = params.permit(:skip_age_check)
+
     hijack do
-      summary = TopicSummarization.new(strategy).summarize(topic, current_user)
+      summary = TopicSummarization.new(strategy).summarize(topic, current_user, opts)
 
       render json: {
                summary: summary.summarized_text,
                summarized_on: summary.updated_at,
                summarized_by: summary.algorithm,
+               outdated: summary.outdated,
+               can_regenerate: Summarization::Base.can_request_summary_for?(current_user),
+               new_posts_since_summary:
+                 topic.highest_post_number.to_i - summary.content_range&.max.to_i,
              }
     end
   end

app/models/summary_section.rb

@@ -2,6 +2,14 @@
 
 class SummarySection < ActiveRecord::Base
   belongs_to :target, polymorphic: true
+
+  def mark_as_outdated
+    @outdated = true
+  end
+
+  def outdated
+    @outdated || false
+  end
 end
 
 # == Schema Information

app/services/topic_summarization.rb

@@ -5,14 +5,25 @@ class TopicSummarization
     @strategy = strategy
   end
 
-  def summarize(topic, user)
+  def summarize(topic, user, opts = {})
     existing_summary = SummarySection.find_by(target: topic, meta_section_id: nil)
 
-    # For users without permissions to generate a summary, we return what we have cached.
     # Existing summary shouldn't be nil in this scenario because the controller checks its existence.
-    return existing_summary if !user || !Summarization::Base.can_request_summary_for?(user)
+    return if !user && !existing_summary
 
-    return existing_summary if existing_summary && fresh?(existing_summary, topic)
+    targets_data = summary_targets(topic).pluck(:post_number, :raw, :username)
+
+    current_topic_sha = build_sha(targets_data.map(&:first))
+    can_summarize = Summarization::Base.can_request_summary_for?(user)
+
+    if use_cached?(existing_summary, can_summarize, current_topic_sha, !!opts[:skip_age_check])
+      # It's important that we signal a cached summary is outdated
+      if can_summarize && new_targets?(existing_summary, current_topic_sha)
+        existing_summary.mark_as_outdated
+      end
+
+      return existing_summary
+    end
 
     delete_cached_summaries_of(topic) if existing_summary
 
@@ -22,8 +33,6 @@ class TopicSummarization
       contents: [],
     }
 
-    targets_data = summary_targets(topic).pluck(:post_number, :raw, :username)
-
     targets_data.map do |(pn, raw, username)|
       content[:contents] << { poster: username, id: pn, text: raw }
     end
@@ -34,7 +43,7 @@ class TopicSummarization
   end
 
   def summary_targets(topic)
-    @targets ||= topic.has_summary? ? best_replies(topic) : pick_selection(topic)
+    topic.has_summary? ? best_replies(topic) : pick_selection(topic)
   end
 
   private
@@ -73,11 +82,17 @@ class TopicSummarization
     SummarySection.where(target: topic).destroy_all
   end
 
-  def fresh?(summary, topic)
+  # For users without permissions to generate a summary or fresh summaries, we return what we have cached.
-    return true if summary.created_at > 12.hours.ago
+  def use_cached?(existing_summary, can_summarize, current_sha, skip_age_check)
-    latest_post_to_summarize = summary_targets(topic).last.post_number
+    existing_summary &&
+      !(
+        can_summarize && new_targets?(existing_summary, current_sha) &&
+          (skip_age_check || existing_summary.created_at < 1.hour.ago)
+      )
+  end
 
-    latest_post_to_summarize <= summary.content_range.to_a.last
+  def new_targets?(summary, current_sha)
+    summary.original_content_sha != current_sha
   end
 
   def cache_summary(result, post_numbers, topic)
@@ -87,7 +102,7 @@ class TopicSummarization
         algorithm: strategy.model,
         content_range: (post_numbers.first..post_numbers.last),
         summarized_text: result[:summary],
-        original_content_sha: Digest::SHA256.hexdigest(post_numbers.join),
+        original_content_sha: build_sha(post_numbers),
       )
 
     result[:chunks].each do |chunk|
@@ -96,11 +111,15 @@ class TopicSummarization
         algorithm: strategy.model,
         content_range: chunk[:ids].min..chunk[:ids].max,
         summarized_text: chunk[:summary],
-        original_content_sha: Digest::SHA256.hexdigest(chunk[:ids].join),
+        original_content_sha: build_sha(chunk[:ids]),
         meta_section_id: main_summary.id,
       )
     end
 
     main_summary
   end
+
+  def build_sha(ids)
+    Digest::SHA256.hexdigest(ids.join)
+  end
 end

config/locales/client.en.yml

@@ -2043,10 +2043,18 @@ en:
       in_progress: "Summarizing topic using AI..."
       summarized_on: "Summarized with AI on %{date}"
       model_used: "AI used: %{model}"
+      outdated: "Summary is outdated"
+      outdated_posts: 
+        one: "(%{count} post missing)"
+        other: "(%{count} posts missing)"
       enabled_description: "You're viewing this topic top replies: the most interesting posts as determined by the community."
       description:
         one: "There is <b>%{count}</b> reply."
         other: "There are <b>%{count}</b> replies."
+      buttons:
+        hide: "Hide summary"
+        generate: "Summarize with AI"
+        regenerate: "Regenerate summary"
 
       # This string uses the ICU Message Format. See https://meta.discourse.org/t/7035 for translation guidelines.
       description_time_MF: |
@@ -2062,9 +2070,6 @@ en:
       disable: "Show All Posts"
       short_label: "Top replies"
       short_title: "Show this topic top replies: the most interesting posts as determined by the community"
-      strategy:
-        hide_button_title: "Hide summary"
-        button_title: "Summarize with AI"
 
     deleted_filter:
       enabled_description: "This topic contains deleted posts, which have been hidden."

spec/services/topic_summarization_spec.rb

@@ -147,29 +147,42 @@ describe TopicSummarization do
         cached_summary.update!(summarized_text: cached_text, created_at: 24.hours.ago)
       end
 
-      context "when the summary targets changed" do
+      context "when the user can requests new summaries" do
-        before { cached_summary.update!(content_range: (1..1)) }
+        context "when there are no new posts" do
+          it "returns the cached summary" do
+            section = summarization.summarize(topic, user)
 
-        it "deletes existing summaries and create a new one" do
+            expect(section.summarized_text).to eq(cached_text)
-          section = summarization.summarize(topic, user)
+          end
-
-          expect(section.summarized_text).to eq(summarized_text)
         end
 
-        it "does nothing if the last summary is less than 12 hours old" do
+        context "when there are new posts" do
-          cached_summary.update!(created_at: 6.hours.ago)
+          before { cached_summary.update!(original_content_sha: "outdated_sha") }
 
-          section = summarization.summarize(topic, user)
+          it "returns a new summary" do
+            section = summarization.summarize(topic, user)
 
-          expect(section.summarized_text).to eq(cached_text)
+            expect(section.summarized_text).to eq(summarized_text)
-        end
+          end
-      end
 
-      context "when the summary targets are still the same" do
+          context "when the cached summary is less than one hour old" do
-        it "doesn't create a new summary" do
+            before { cached_summary.update!(created_at: 30.minutes.ago) }
-          section = summarization.summarize(topic, user)
 
-          expect(section.summarized_text).to eq(cached_text)
+            it "returns the cached summary" do
+              cached_summary.update!(created_at: 30.minutes.ago)
+
+              section = summarization.summarize(topic, user)
+
+              expect(section.summarized_text).to eq(cached_text)
+              expect(section.outdated).to eq(true)
+            end
+
+            it "returns a new summary if the skip_age_check flag is passed" do
+              section = summarization.summarize(topic, user, skip_age_check: true)
+
+              expect(section.summarized_text).to eq(summarized_text)
+            end
+          end
         end
       end
     end