Merge branch '415358-auto-generate-review-summary' into 'master'

Auto-generate review summary when submitted

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/124581



Merged-by: Kerri Miller <kerrizor@kerrizor.com>
Approved-by: Marc Shaw <mshaw@gitlab.com>
Approved-by: Kerri Miller <kerrizor@kerrizor.com>
Reviewed-by: Patrick Bajao <ebajao@gitlab.com>
Reviewed-by: Marc Shaw <mshaw@gitlab.com>
Co-authored-by: Patrick Bajao <ebajao@gitlab.com>

app/services/draft_notes/publish_service.rb

@@ -49,6 +49,7 @@ def publish_draft_notes
       notification_service.async.new_review(review)
       MergeRequests::ResolvedDiscussionNotificationService.new(project: project, current_user: current_user).execute(merge_request)
       GraphqlTriggers.merge_request_merge_status_updated(merge_request)
+      after_publish(review)
     end
 
     def create_note_from_draft(draft, skip_capture_diff_note_position: false, skip_keep_around_commits: false, skip_merge_status_trigger: false)
@@ -108,5 +109,11 @@ def keep_around_commits(notes)
         project.repository.keep_around(*shas)
       end
     end
+
+    def after_publish(review)
+      # Overridden in EE
+    end
   end
 end
+
+DraftNotes::PublishService.prepend_mod

ee/app/models/gitlab_subscriptions/features.rb

@@ -253,6 +253,7 @@ class Features
       summarize_mr_changes
       summarize_my_mr_code_review
       summarize_notes
+      summarize_submitted_review
       stale_runner_cleanup_for_namespace
       status_page
       suggested_reviewers

ee/app/models/merge_request/review_llm_summary.rb

@@ -9,7 +9,7 @@ class MergeRequest::ReviewLlmSummary < ApplicationRecord
   validates :provider, presence: true
   validates :content, presence: true, length: { maximum: 2056 }
 
-  enum provider: { open_ai: 0 }
+  enum provider: { open_ai: 0, vertex_ai: 1 }
 
   def reviewer
     review.author

ee/app/policies/ee/merge_request_policy.rb

@@ -46,6 +46,18 @@ module MergeRequestPolicy
         @subject.target_project.licensed_feature_available?(:report_approver_rules)
       end
 
+      with_scope :subject
+      condition(:ai_features_enabled) do
+        ::Feature.enabled?(:openai_experimentation)
+      end
+
+      with_scope :subject
+      condition(:summarize_submitted_review_enabled) do
+        ::Feature.enabled?(:automatically_summarize_mr_review, subject.project) &&
+          subject.project.licensed_feature_available?(:summarize_submitted_review) &&
+          ::Gitlab::Llm::StageCheck.available?(subject.project.root_ancestor, :summarize_submitted_review)
+      end
+
       def read_only?
         @subject.target_project&.namespace&.read_only?
       end
@@ -83,6 +95,10 @@ def group_access?(protected_branch)
       end
 
       rule { approval_rules_licence_enabled }.enable :create_merge_request_approval_rules
+
+      rule do
+        ai_features_enabled & summarize_submitted_review_enabled & can?(:read_merge_request)
+      end.enable :summarize_submitted_review
     end
 
     private

ee/app/services/ee/draft_notes/publish_service.rb

+# frozen_string_literal: true
+
+module EE
+  module DraftNotes
+    module PublishService
+      def after_publish(review)
+        Llm::SummarizeSubmittedReviewService
+          .new(
+            current_user,
+            merge_request,
+            review_id: review.id,
+            diff_id: merge_request.latest_merge_request_diff_id
+          )
+          .execute
+      end
+    end
+  end
+end

ee/app/services/llm/summarize_submitted_review_service.rb

+# frozen_string_literal: true
+
+module Llm
+  class SummarizeSubmittedReviewService < ::Llm::BaseService
+    private
+
+    def perform
+      worker_perform(user, resource, :summarize_submitted_review, options)
+    end
+
+    def valid?
+      super &&
+        resource.to_ability_name == "merge_request" &&
+        Ability.allowed?(user, :summarize_submitted_review, resource)
+    end
+  end
+end

ee/config/feature_flags/development/automatically_summarize_mr_review.yml

+---
+name: automatically_summarize_mr_review
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/124581
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/416706
+milestone: '16.2'
+type: development
+group: group::code review
+default_enabled: false

ee/lib/gitlab/llm/completions_factory.rb

@@ -51,6 +51,10 @@ class CompletionsFactory
         fill_in_merge_request_template: {
           service_class: ::Gitlab::Llm::VertexAi::Completions::FillInMergeRequestTemplate,
           prompt_class: ::Gitlab::Llm::Templates::FillInMergeRequestTemplate
+        },
+        summarize_submitted_review: {
+          service_class: ::Gitlab::Llm::VertexAi::Completions::SummarizeSubmittedReview,
+          prompt_class: ::Gitlab::Llm::Templates::SummarizeSubmittedReview
         }
       }.freeze
 

ee/lib/gitlab/llm/stage_check.rb

@@ -14,7 +14,8 @@ class StageCheck
         :explain_vulnerability,
         :generate_commit_message,
         :chat,
-        :fill_in_merge_request_template
+        :fill_in_merge_request_template,
+        :summarize_submitted_review
       ].freeze
       BETA_FEATURES = [].freeze
       THIRD_PARTY_FEATURES = EXPERIMENTAL_FEATURES + BETA_FEATURES

ee/lib/gitlab/llm/templates/summarize_submitted_review.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Llm
+    module Templates
+      class SummarizeSubmittedReview
+        # We're using the chat model for this.
+        # Based on https://cloud.google.com/vertex-ai/docs/generative-ai/learn/models#foundation_models,
+        # the input token limit is 4,096 tokens.
+        MODEL_INPUT_TOKEN_LIMIT = 4096
+
+        # Based on https://cloud.google.com/vertex-ai/docs/generative-ai/learn/models#chat_model_parameters,
+        # a token is approximately 4 characters. Since we also need to take the
+        # prompt we send into consideration, we reduce the limit by a bit and only
+        # take 95% of the model input token limit.
+        INPUT_CONTENT_LIMIT = (MODEL_INPUT_TOKEN_LIMIT * 0.95) * 4
+
+        def initialize(review)
+          @review = review
+        end
+
+        def to_prompt
+          <<-PROMPT
+            You are a sophisticated code review assistant.
+            You are acting as the reviewer for this merge request and MUST respond in first person as if you reviewed it and should always use 'I'.
+            You are provided with the corresponding code comment.
+            Use this information to create an overall summary which MUST mention the types of comments left, a comment can be either: question or recommendation.
+            This summary MUST NOT be longer than 3 sentences.
+            This summary MUST give an indication of the topics the review covered.
+            The summary MUST be written in present simple tense and MUST be as concise as possible.
+            The summary MUST also include an estimate of the overall work needed, using any of the following: "small amount of work, decent amount or significant work required" but the comment MUST make sure to note this is only an estimate, for example, "I estimate there is...".
+
+            Code review comments:
+            #{review_content}
+          PROMPT
+        end
+
+        private
+
+        attr_reader :review
+
+        def review_content
+          return unless review.present?
+
+          content = []
+
+          review.notes.each do |note|
+            note_line = "Comment: #{note.note}\n\n"
+
+            content << note_line if (content.length + note_line.length) < INPUT_CONTENT_LIMIT
+          end
+
+          content.join("\n")
+        end
+      end
+    end
+  end
+end

ee/lib/gitlab/llm/vertex_ai/completions/summarize_submitted_review.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Llm
+    module VertexAi
+      module Completions
+        class SummarizeSubmittedReview < Gitlab::Llm::Completions::Base
+          # rubocop:disable CodeReuse/ActiveRecord
+          def execute(user, merge_request, options)
+            review = merge_request.reviews.find_by(id: options[:review_id])
+            mr_diff = merge_request.merge_request_diffs.find_by(id: options[:diff_id])
+
+            return unless review.present? && mr_diff.present?
+
+            response = response_for(user, review)
+            response_modifier = ::Gitlab::Llm::VertexAi::ResponseModifiers::Predictions.new(response)
+
+            store_response(response_modifier, review, mr_diff)
+          end
+          # rubocop:enable CodeReuse/ActiveRecord
+
+          private
+
+          def response_for(user, review)
+            template = ai_prompt_class.new(review)
+            request(user, template)
+          end
+
+          def request(user, template)
+            ::Gitlab::Llm::VertexAi::Client
+              .new(user)
+              .chat(content: template.to_prompt)
+          end
+
+          def store_response(response_modifier, review, mr_diff)
+            return if response_modifier.errors.any?
+
+            ::MergeRequest::ReviewLlmSummary.create!(
+              review: review,
+              merge_request_diff: mr_diff,
+              user: User.llm_bot,
+              provider: MergeRequest::ReviewLlmSummary.providers[:vertex_ai],
+              content: response_modifier.response_body
+            )
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/lib/gitlab/llm/templates/summarize_submitted_review_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Llm::Templates::SummarizeSubmittedReview, feature_category: :code_review_workflow do
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
+  let_it_be(:review) { create(:review, merge_request: merge_request, author: merge_request.author) }
+  let_it_be(:note_1) { create(:note_on_merge_request, project: project, noteable: merge_request, review: review) }
+  let_it_be(:note_2) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, review: review) }
+
+  subject { described_class.new(review) }
+
+  describe '#to_prompt' do
+    it 'includes lines per note' do
+      prompt = subject.to_prompt
+
+      expect(prompt).to include("Comment: #{note_1.note}")
+      expect(prompt).to include("Comment: #{note_2.note}")
+    end
+  end
+end

ee/spec/lib/gitlab/llm/vertex_ai/completions/summarize_submitted_review_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Llm::VertexAi::Completions::SummarizeSubmittedReview, feature_category: :code_review_workflow do
+  let_it_be(:llm_bot) { create(:user, :llm_bot) }
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
+  let_it_be(:mr_diff) { merge_request.merge_request_diff }
+  let_it_be(:user) { merge_request.author }
+  let_it_be(:review) { create(:review, merge_request: merge_request, author: user) }
+  let_it_be(:note_1) { create(:note_on_merge_request, project: project, noteable: merge_request, review: review) }
+  let_it_be(:note_2) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, review: review) }
+
+  let(:prompt_class) { Gitlab::Llm::Templates::SummarizeSubmittedReview }
+  let(:options) do
+    {
+      diff_id: mr_diff.id,
+      review_id: review.id,
+      request_id: 'uuid'
+    }
+  end
+
+  subject { described_class.new(prompt_class, options) }
+
+  describe '#execute' do
+    context 'when the chat client returns a successful response' do
+      let(:example_answer) { 'Summary generated by AI' }
+
+      let(:example_response) do
+        {
+          "predictions" => [
+            {
+              "candidates" => [
+                {
+                  "author" => "",
+                  "content" => example_answer
+                }
+              ],
+              "safetyAttributes" => {
+                "categories" => ["Violent"],
+                "scores" => [0.4000000059604645],
+                "blocked" => false
+              }
+            }
+          ]
+        }
+      end
+
+      before do
+        allow_next_instance_of(Gitlab::Llm::VertexAi::Client) do |client|
+          allow(client).to receive(:chat).and_return(example_response.to_json)
+        end
+      end
+
+      it 'stores the content from the AI response' do
+        expect { subject.execute(user, merge_request, options) }
+          .to change { mr_diff.merge_request_review_llm_summaries.count }
+          .by(1)
+
+        latest_review_llm_summary = mr_diff.merge_request_review_llm_summaries.last
+
+        aggregate_failures do
+          expect(latest_review_llm_summary.merge_request_diff).to eq(mr_diff)
+          expect(latest_review_llm_summary.review).to eq(review)
+          expect(latest_review_llm_summary.user).to eq(llm_bot)
+          expect(latest_review_llm_summary.provider).to eq('vertex_ai')
+          expect(latest_review_llm_summary.content).to eq('Summary generated by AI')
+        end
+      end
+    end
+
+    context 'when the chat client returns an unsuccessful response' do
+      let(:error) { { error: { message: 'Error' } } }
+
+      before do
+        allow_next_instance_of(Gitlab::Llm::VertexAi::Client) do |client|
+          allow(client).to receive(:chat).and_return(error.to_json)
+        end
+      end
+
+      it 'does not store the content' do
+        expect { subject.execute(user, merge_request, options) }
+          .not_to change { mr_diff.merge_request_review_llm_summaries.count }
+      end
+    end
+  end
+end

ee/spec/policies/merge_request_policy_spec.rb

@@ -469,4 +469,83 @@ def policy_for(user)
       end
     end
   end
+
+  describe 'summarize_submitted_review policy', :saas do
+    let_it_be(:namespace) { create(:group_with_plan, plan: :ultimate_plan) }
+    let_it_be(:project) { create(:project, group: namespace) }
+    let_it_be(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
+    let_it_be(:current_user) { developer }
+
+    subject { described_class.new(current_user, merge_request) }
+
+    before do
+      stub_ee_application_setting(should_check_namespace_plan: true)
+
+      stub_licensed_features(
+        summarize_submitted_review: true,
+        ai_features: true
+      )
+
+      stub_feature_flags(
+        openai_experimentation: true,
+        automatically_summarize_mr_review: true
+      )
+
+      namespace.namespace_settings.update!(
+        experiment_features_enabled: true,
+        third_party_ai_features_enabled: true
+      )
+    end
+
+    it { is_expected.to be_allowed(:summarize_submitted_review) }
+
+    context 'when global AI feature flag is disabled' do
+      before do
+        stub_feature_flags(openai_experimentation: false)
+      end
+
+      it { is_expected.to be_disallowed(:summarize_submitted_review) }
+    end
+
+    context 'when automatically_summarize_mr_review feature flag is disabled' do
+      before do
+        stub_feature_flags(
+          openai_experimentation: true,
+          automatically_summarize_mr_review: false
+        )
+      end
+
+      it { is_expected.to be_disallowed(:summarize_submitted_review) }
+    end
+
+    context 'when license is not set' do
+      before do
+        stub_licensed_features(summarize_submitted_review: false)
+      end
+
+      it { is_expected.to be_disallowed(:summarize_submitted_review) }
+    end
+
+    context 'when experiment features are disabled' do
+      before do
+        namespace.namespace_settings.update!(experiment_features_enabled: false)
+      end
+
+      it { is_expected.to be_disallowed(:summarize_submitted_review) }
+    end
+
+    context 'when third party ai features are disabled' do
+      before do
+        namespace.namespace_settings.update!(third_party_ai_features_enabled: false)
+      end
+
+      it { is_expected.to be_disallowed(:summarize_submitted_review) }
+    end
+
+    context 'when user cannot read merge request' do
+      let(:current_user) { guest }
+
+      it { is_expected.to be_disallowed(:summarize_submitted_review) }
+    end
+  end
 end

ee/spec/services/ee/draft_notes/publish_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe DraftNotes::PublishService, feature_category: :code_review_workflow do
+  let(:merge_request) { create(:merge_request) }
+  let(:project) { merge_request.target_project }
+  let(:user) { merge_request.author }
+
+  def publish
+    DraftNotes::PublishService.new(merge_request, user).execute
+  end
+
+  before do
+    create(:draft_note_on_text_diff, merge_request: merge_request, author: user)
+  end
+
+  it 'executes Llm::SummarizeSubmittedReviewService and returns success' do
+    expect_next_instance_of(
+      Llm::SummarizeSubmittedReviewService,
+      user,
+      merge_request,
+      review_id: kind_of(Numeric),
+      diff_id: merge_request.latest_merge_request_diff_id
+    ) do |svc|
+      expect(svc).to receive(:execute)
+    end
+
+    expect(publish[:status]).to eq(:success)
+  end
+end

ee/spec/services/llm/summarize_submitted_review_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Llm::SummarizeSubmittedReviewService, feature_category: :code_review_workflow do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:group) { create(:group, :public) }
+  let_it_be(:project) { create(:project, :public, group: group) }
+  let_it_be(:resource) { create(:merge_request, source_project: project, target_project: project, author: user) }
+
+  let(:summarize_submitted_review_enabled) { true }
+  let(:current_user) { user }
+  let(:options) { {} }
+
+  describe '#perform' do
+    before_all do
+      group.add_guest(user)
+    end
+
+    before do
+      allow(Ability).to receive(:allowed?).and_call_original
+      allow(Ability)
+        .to receive(:allowed?)
+        .with(user, :summarize_submitted_review, resource)
+        .and_return(summarize_submitted_review_enabled)
+    end
+
+    subject { described_class.new(current_user, resource, options).execute }
+
+    it_behaves_like 'completion worker sync and async' do
+      subject { described_class.new(current_user, resource, options) }
+
+      let(:action_name) { :summarize_submitted_review }
+      let(:content) { 'Summarize submitted review' }
+    end
+
+    context 'when user is not member of project group' do
+      let(:current_user) { create(:user) }
+
+      it { is_expected.to be_error.and have_attributes(message: eq(described_class::INVALID_MESSAGE)) }
+    end
+
+    context 'when general feature flag is disabled' do
+      before do
+        stub_feature_flags(openai_experimentation: false)
+      end
+
+      it { is_expected.to be_error.and have_attributes(message: eq(described_class::INVALID_MESSAGE)) }
+    end
+
+    context 'when resource is not a merge_request' do
+      let(:resource) { create(:epic, group: group) }
+
+      it { is_expected.to be_error.and have_attributes(message: eq(described_class::INVALID_MESSAGE)) }
+    end
+
+    context 'when user has no ability to summarize_submitted_review' do
+      let(:summarize_submitted_review_enabled) { false }
+
+      it { is_expected.to be_error.and have_attributes(message: eq(described_class::INVALID_MESSAGE)) }
+    end
+  end
+end