Merge pull request #1079 from alphagov/523-always-rephrase-question

[CHAT-523] Always rephrase question for Sonnet 4.5

Author: davidgisbey

lib/answer_composition/pipeline/question_rephraser.rb

@@ -18,11 +18,11 @@ def initialize(context)
       end
 
       def call
-        return if message_records.blank?
+        return if message_records.blank? && model_name == :claude_sonnet_4_0
 
         start_time = Clock.monotonic_time
         response = anthropic_bedrock_client.messages.create(
-          system: [{ type: "text", text: config[:system_prompt] }],
+          system: [{ type: "text", text: system_prompt }],
           model: model_id,
           messages:,
           **inference_config,
@@ -78,10 +78,25 @@ def inference_config
         }
       end
 
+      def system_prompt
+        return config[:system_prompt] if model_name == :claude_sonnet_4_0
+
+        config[:system_prompt_always_rephrase]
+      end
+
       def user_prompt
-        config[:user_prompt]
-          .sub("{question}", question_message)
-          .sub("{message_history}", message_history)
+        if model_name == :claude_sonnet_4_0
+          return config[:user_prompt].sub("{question}", question_message)
+                                     .sub("{message_history}", message_history)
+
+        end
+
+        if message_records.present?
+          config[:user_prompt_with_history].sub("{question}", question_message)
+                                           .sub("{message_history}", message_history)
+        else
+          config[:user_prompt_without_history].sub("{question}", question_message)
+        end
       end
 
       def messages

spec/lib/answer_composition/pipeline/question_rephraser_spec.rb

@@ -30,96 +30,100 @@
     end
   end
 
-  context "when the question is the beginning of the conversation" do
-    let(:context) { build(:answer_pipeline_context) }
-
-    it "returns nil" do
-      expect(described_class.call(context)).to be_nil
-    end
+  it "includes the current question in the user prompt" do
+    described_class.call(context)
+    expect(stub).to have_been_requested
   end
 
-  context "when all other recent answers have statuses in Answer::STATUSES_EXCLUDED_FROM_REPHRASING" do
-    it "returns nil" do
-      conversation = create(:conversation)
-      create(:question, conversation:)
-      Answer::STATUSES_EXCLUDED_FROM_REPHRASING.sample(4) do |status|
-        question = create(:question, conversation:)
-        create(:answer, question:, status:)
-      end
-      latest_question = create(:question, conversation:)
-      context = build(:answer_pipeline_context, question: latest_question)
+  it "includes the message_history in the user prompt" do
+    message_history = <<~HISTORY.strip
+      user:
+      """
+      How do I pay my tax
+      """
+      assistant:
+      """
+      What type of tax
+      """
+      user:
+      """
+      What types are there
+      """
+      assistant:
+      """
+      Self-assessment, PAYE, Corporation tax
+      """
+    HISTORY
 
-      expect(described_class.call(context)).to be_nil
-    end
+    anthropic_request = stub_claude_question_rephrasing(
+      Regexp.new(message_history),
+      rephrased,
+    )
+
+    described_class.call(context)
+
+    expect(anthropic_request).to have_been_made
   end
 
-  context "when the question is part of an ongoing chat" do
-    it "includes the current question in the user prompt" do
-      described_class.call(context)
-      expect(stub).to have_been_requested
-    end
+  it "updates the context's question_message with the rephrased question" do
+    described_class.call(context)
+    expect(context.question_message).to eq(rephrased)
+  end
 
-    it "includes the message_history in the user prompt" do
-      message_history = <<~HISTORY.strip
-        user:
-        """
-        How do I pay my tax
-        """
-        assistant:
-        """
-        What type of tax
-        """
-        user:
-        """
-        What types are there
-        """
-        assistant:
-        """
-        Self-assessment, PAYE, Corporation tax
-        """
-      HISTORY
+  it "assigns metrics to the answer" do
+    allow(Clock).to receive(:monotonic_time).and_return(100.0, 101.5)
 
-      anthropic_request = stub_claude_question_rephrasing(
-        Regexp.new(message_history),
-        rephrased,
-      )
+    described_class.call(context)
 
-      described_class.call(context)
+    expect(context.answer.metrics["question_rephrasing"])
+      .to eq({
+        duration: 1.5,
+        llm_prompt_tokens: 10,
+        llm_completion_tokens: 20,
+        llm_cached_tokens: nil,
+        model: BedrockModels.model_id(described_class::DEFAULT_MODEL),
+      })
+  end
 
-      expect(anthropic_request).to have_been_made
-    end
+  it "assigns the llm response to the answer" do
+    described_class.call(context)
 
-    it "updates the context's question_message with the rephrased question" do
-      described_class.call(context)
-      expect(context.question_message).to eq(rephrased)
-    end
+    expected_llm_response = claude_messages_response(
+      content: [claude_messages_text_block(rephrased)],
+      usage: claude_messages_usage_block(input_tokens: 10, output_tokens: 20),
+      bedrock_model: described_class::DEFAULT_MODEL,
+    ).to_h
 
-    it "assigns metrics to the answer" do
-      allow(Clock).to receive(:monotonic_time).and_return(100.0, 101.5)
+    expect(context.answer.llm_responses["question_rephrasing"])
+      .to eq(expected_llm_response)
+  end
 
+  context "when the question is the first in the conversation" do
+    let(:question) { create(:question) }
+    let(:context) { build(:answer_pipeline_context, question:) }
+    let!(:stub) { stub_claude_question_rephrasing(question.message, rephrased) }
+
+    it "calls the llm and rephrases the question" do
       described_class.call(context)
 
-      expect(context.answer.metrics["question_rephrasing"])
-        .to eq({
-          duration: 1.5,
-          llm_prompt_tokens: 10,
-          llm_completion_tokens: 20,
-          llm_cached_tokens: nil,
-          model: BedrockModels.model_id(described_class::DEFAULT_MODEL),
-        })
+      expect(stub).to have_been_requested
+      expect(context.question_message).to eq(rephrased)
     end
 
-    it "assigns the llm response to the answer" do
-      described_class.call(context)
+    it "uses the user_prompt_without_history prompt" do
+      expected_prompt = AnswerComposition::Pipeline::Prompts.config(
+        :question_rephraser, described_class::DEFAULT_MODEL
+      )[:user_prompt_without_history]
+      .sub("{question}", question.message)
+
+      anthropic_request = stub_claude_question_rephrasing(
+        expected_prompt,
+        rephrased,
+      )
 
-      expected_llm_response = claude_messages_response(
-        content: [claude_messages_text_block(rephrased)],
-        usage: claude_messages_usage_block(input_tokens: 10, output_tokens: 20),
-        bedrock_model: described_class::DEFAULT_MODEL,
-      ).to_h
+      described_class.call(context)
 
-      expect(context.answer.llm_responses["question_rephrasing"])
-        .to eq(expected_llm_response)
+      expect(anthropic_request).to have_been_made
     end
   end
 
@@ -225,4 +229,60 @@
       expect(anthropic_request).to have_been_made
     end
   end
+
+  context "when the model is claude_sonnet_4_0" do
+    let!(:stub) do
+      stub_claude_question_rephrasing(
+        question.message, rephrased, chat_options: { bedrock_model: :claude_sonnet_4_0 }
+      )
+    end
+
+    before { stub_const("#{described_class}::DEFAULT_MODEL", :claude_sonnet_4_0) }
+
+    it "uses the system prompt configured for claude_sonnet_4_0" do
+      allow(AnswerComposition::Pipeline::Prompts.config(:question_rephraser, :claude_sonnet_4_0))
+        .to receive(:[]).and_call_original
+
+      described_class.call(context)
+
+      expect(AnswerComposition::Pipeline::Prompts.config(:question_rephraser, :claude_sonnet_4_0))
+        .to have_received(:[]).with(:system_prompt)
+    end
+
+    it "calls the llm when there is message history" do
+      described_class.call(context)
+
+      expect(stub).to have_been_requested
+      expect(context.question_message).to eq(rephrased)
+    end
+
+    context "and all other recent answers have statuses in Answer::STATUSES_EXCLUDED_FROM_REPHRASING" do
+      it "returns nil" do
+        conversation = create(:conversation)
+        create(:question, conversation:)
+        Answer::STATUSES_EXCLUDED_FROM_REPHRASING.sample(4) do |status|
+          question = create(:question, conversation:)
+          create(:answer, question:, status:)
+        end
+        latest_question = create(:question, conversation:)
+        context = build(:answer_pipeline_context, question: latest_question)
+
+        expect(described_class.call(context)).to be_nil
+        expect(stub).not_to have_been_requested
+      end
+    end
+
+    context "and there is no message history" do
+      let(:conversation) { create(:conversation) }
+      let(:question) { create(:question, conversation:) }
+      let(:context) { build(:answer_pipeline_context, question:) }
+
+      it "returns nil" do
+        result = described_class.call(context)
+
+        expect(stub).not_to have_been_requested
+        expect(result).to be_nil
+      end
+    end
+  end
 end

spec/support/system_spec_helpers.rb

@@ -35,18 +35,13 @@ def given_i_am_an_admin_with_the_settings_permission
 
   def stubs_for_mock_answer(question,
                             answer,
-                            rephrase_question: false,
                             sources_used: [],
                             create_content_chunk: true)
     stub_claude_jailbreak_guardrails(question)
+    rephrased_question = "Rephrased #{question}"
+    stub_claude_question_rephrasing(question, rephrased_question)
 
-    if rephrase_question
-      rephrased_question = "Rephrased #{question}"
-
-      stub_claude_question_rephrasing(question, rephrased_question)
-
-      question = rephrased_question
-    end
+    question = rephrased_question
 
     stub_bedrock_titan_embedding(question)
 

spec/system/conversation_js_features_spec.rb

@@ -182,7 +182,6 @@ def when_the_second_answer_is_generated
 
     stubs_for_mock_answer(@second_question,
                           @second_answer,
-                          rephrase_question: true,
                           sources_used: %w[link_1],
                           create_content_chunk: false)
 

spec/system/conversation_with_claude_structured_answer_spec.rb

@@ -66,7 +66,6 @@ def when_the_second_answer_is_generated
 
     stubs_for_mock_answer(@second_question,
                           @second_answer,
-                          rephrase_question: true,
                           sources_used: %w[link_1],
                           create_content_chunk: false)