From 397e3d8c6e6177b10b62d5507177ac2edb8e0155 Mon Sep 17 00:00:00 2001
From: Marc Shaw <mshaw@gitlab.com>
Date: Mon, 8 Apr 2024 17:41:15 +0000
Subject: [PATCH] Remove the backend changes for automatic diff summary
MR: gitlab.com/gitlab-org/gitlab/-/merge_requests/148122
Changelog: removed
---
.../rspec/before_all_role_assignment.yml | 2 -
.rubocop_todo/rspec/named_subject.yml | 2 -
.../style/inline_disable_annotation.yml | 3 -
doc/api/graphql/reference/index.md | 40 ----
.../queries/summary_notes.query.graphql | 32 ----
ee/app/graphql/ee/types/merge_request_type.rb | 10 -
.../graphql/types/merge_request_diff_type.rb | 5 -
.../merge_requests/diff_llm_summary_type.rb | 37 ----
ee/app/helpers/ee/merge_requests_helper.rb | 12 --
ee/app/helpers/ee/todos_helper.rb | 10 -
ee/app/models/ee/merge_request.rb | 7 -
ee/app/models/ee/merge_request_diff.rb | 17 --
.../models/merge_request/diff_llm_summary.rb | 2 +-
ee/app/policies/ee/merge_request_policy.rb | 22 ---
.../merge_request/diff_llm_summary_policy.rb | 7 -
ee/app/services/ee/notification_service.rb | 12 --
.../merge_requests/summarize_diff_service.rb | 90 ---------
.../llm/summarize_merge_request_service.rb | 21 ---
ee/app/views/notify/_diff_summary.html.haml | 8 -
ee/app/views/notify/_diff_summary.text.erb | 14 --
.../summarize_diff_automatically.yml | 8 -
ee/lib/gitlab/duo/developments/setup.rb | 1 -
ee/lib/gitlab/llm/completions_factory.rb | 5 -
ee/lib/gitlab/llm/stage_check.rb | 1 -
.../llm/templates/summarize_merge_request.rb | 62 ------
.../completions/summarize_merge_request.rb | 60 ------
ee/spec/factories/ai_messages.rb | 4 -
.../ee/types/merge_request_type_spec.rb | 1 -
.../types/merge_request_diff_type_spec.rb | 1 -
.../diff_llm_summary_type_spec.rb | 19 --
.../helpers/ee/merge_requests_helper_spec.rb | 31 ---
ee/spec/helpers/ee/todos_helper_spec.rb | 38 ----
.../templates/summarize_merge_request_spec.rb | 69 -------
.../summarize_merge_request_spec.rb | 176 ------------------
ee/spec/mailers/notify_spec.rb | 44 -----
ee/spec/models/ee/merge_request_diff_spec.rb | 60 ------
.../merge_request/diff_llm_summary_spec.rb | 18 --
ee/spec/models/merge_request_spec.rb | 15 --
.../diff_llm_summary_policy_spec.rb | 26 ---
ee/spec/policies/merge_request_policy_spec.rb | 82 --------
.../api/graphql/project/merge_request_spec.rb | 74 +-------
.../services/ee/notification_service_spec.rb | 34 ----
.../summarize_diff_service_spec.rb | 151 ---------------
.../summarize_merge_request_service_spec.rb | 51 -----
.../merge_requests_diff_llm_summary.rb | 10 -
45 files changed, 4 insertions(+), 1390 deletions(-)
delete mode 100644 ee/app/assets/javascripts/merge_requests/queries/summary_notes.query.graphql
delete mode 100644 ee/app/graphql/types/merge_requests/diff_llm_summary_type.rb
delete mode 100644 ee/app/policies/merge_request/diff_llm_summary_policy.rb
delete mode 100644 ee/app/services/llm/merge_requests/summarize_diff_service.rb
delete mode 100644 ee/app/services/llm/summarize_merge_request_service.rb
delete mode 100644 ee/app/views/notify/_diff_summary.html.haml
delete mode 100644 ee/app/views/notify/_diff_summary.text.erb
delete mode 100644 ee/config/feature_flags/development/summarize_diff_automatically.yml
delete mode 100644 ee/lib/gitlab/llm/templates/summarize_merge_request.rb
delete mode 100644 ee/lib/gitlab/llm/vertex_ai/completions/summarize_merge_request.rb
delete mode 100644 ee/spec/graphql/types/merge_requests/diff_llm_summary_type_spec.rb
delete mode 100644 ee/spec/lib/gitlab/llm/templates/summarize_merge_request_spec.rb
delete mode 100644 ee/spec/lib/gitlab/llm/vertex_ai/completions/summarize_merge_request_spec.rb
delete mode 100644 ee/spec/models/merge_request/diff_llm_summary_spec.rb
delete mode 100644 ee/spec/policies/merge_request/diff_llm_summary_policy_spec.rb
delete mode 100644 ee/spec/services/llm/merge_requests/summarize_diff_service_spec.rb
delete mode 100644 ee/spec/services/llm/summarize_merge_request_service_spec.rb
delete mode 100644 spec/factories/merge_requests_diff_llm_summary.rb
diff --git a/.rubocop_todo/rspec/before_all_role_assignment.yml b/.rubocop_todo/rspec/before_all_role_assignment.yml
index 7974bb3abb5ac..20ec3996b0795 100644
--- a/.rubocop_todo/rspec/before_all_role_assignment.yml
+++ b/.rubocop_todo/rspec/before_all_role_assignment.yml
@@ -332,7 +332,6 @@ RSpec/BeforeAllRoleAssignment:
- 'ee/spec/policies/incident_management/oncall_schedule_policy_spec.rb'
- 'ee/spec/policies/incident_management/oncall_shift_policy_spec.rb'
- 'ee/spec/policies/issue_policy_spec.rb'
- - 'ee/spec/policies/merge_request/diff_llm_summary_policy_spec.rb'
- 'ee/spec/policies/merge_request_policy_spec.rb'
- 'ee/spec/policies/merge_requests/external_status_check_policy_spec.rb'
- 'ee/spec/policies/packages/policies/project_policy_spec.rb'
@@ -672,7 +671,6 @@ RSpec/BeforeAllRoleAssignment:
- 'ee/spec/services/llm/generate_description_service_spec.rb'
- 'ee/spec/services/llm/generate_summary_service_spec.rb'
- 'ee/spec/services/llm/git_command_service_spec.rb'
- - 'ee/spec/services/llm/merge_requests/summarize_diff_service_spec.rb'
- 'ee/spec/services/llm/merge_requests/summarize_review_service_spec.rb'
- 'ee/spec/services/members/activate_service_spec.rb'
- 'ee/spec/services/merge_trains/create_pipeline_service_spec.rb'
diff --git a/.rubocop_todo/rspec/named_subject.yml b/.rubocop_todo/rspec/named_subject.yml
index bcab28ed9f29b..51a5fad326665 100644
--- a/.rubocop_todo/rspec/named_subject.yml
+++ b/.rubocop_todo/rspec/named_subject.yml
@@ -468,13 +468,11 @@ RSpec/NamedSubject:
- 'ee/spec/lib/gitlab/llm/templates/explain_vulnerability_spec.rb'
- 'ee/spec/lib/gitlab/llm/templates/fill_in_merge_request_template_spec.rb'
- 'ee/spec/lib/gitlab/llm/templates/generate_commit_message_spec.rb'
- - 'ee/spec/lib/gitlab/llm/templates/summarize_merge_request_spec.rb'
- 'ee/spec/lib/gitlab/llm/templates/summarize_review_spec.rb'
- 'ee/spec/lib/gitlab/llm/templates/summarize_submitted_review_spec.rb'
- 'ee/spec/lib/gitlab/llm/vertex_ai/completions/analyze_ci_job_failure_spec.rb'
- 'ee/spec/lib/gitlab/llm/vertex_ai/completions/fill_in_merge_request_template_spec.rb'
- 'ee/spec/lib/gitlab/llm/vertex_ai/completions/generate_commit_message_spec.rb'
- - 'ee/spec/lib/gitlab/llm/vertex_ai/completions/summarize_merge_request_spec.rb'
- 'ee/spec/lib/gitlab/llm/vertex_ai/completions/summarize_review_spec.rb'
- 'ee/spec/lib/gitlab/llm/vertex_ai/completions/summarize_submitted_review_spec.rb'
- 'ee/spec/lib/gitlab/llm/vertex_ai/model_configurations/base_spec.rb'
diff --git a/.rubocop_todo/style/inline_disable_annotation.yml b/.rubocop_todo/style/inline_disable_annotation.yml
index 477d50e61d6d0..f4f422e072b22 100644
--- a/.rubocop_todo/style/inline_disable_annotation.yml
+++ b/.rubocop_todo/style/inline_disable_annotation.yml
@@ -1427,7 +1427,6 @@ Style/InlineDisableAnnotation:
- 'ee/app/models/iteration.rb'
- 'ee/app/models/license.rb'
- 'ee/app/models/members/member_role.rb'
- - 'ee/app/models/merge_request/diff_llm_summary.rb'
- 'ee/app/models/merge_request/predictions.rb'
- 'ee/app/models/merge_request/review_llm_summary.rb'
- 'ee/app/models/merge_requests/compliance_violation.rb'
@@ -1441,7 +1440,6 @@ Style/InlineDisableAnnotation:
- 'ee/app/models/search/namespace_index_assignment.rb'
- 'ee/app/models/security/training.rb'
- 'ee/app/models/vulnerabilities/finding.rb'
- - 'ee/app/policies/merge_request/diff_llm_summary_policy.rb'
- 'ee/app/policies/merge_request/review_llm_summary_policy.rb'
- 'ee/app/policies/merge_request_diff_policy.rb'
- 'ee/app/policies/path_lock_policy.rb'
@@ -1869,7 +1867,6 @@ Style/InlineDisableAnnotation:
- 'ee/lib/gitlab/llm/chat_storage.rb'
- 'ee/lib/gitlab/llm/open_ai/client.rb'
- 'ee/lib/gitlab/llm/templates/explain_vulnerability.rb'
- - 'ee/lib/gitlab/llm/vertex_ai/completions/summarize_merge_request.rb'
- 'ee/lib/gitlab/llm/vertex_ai/completions/summarize_submitted_review.rb'
- 'ee/lib/gitlab/middleware/ip_restrictor.rb'
- 'ee/lib/gitlab/path_locks_finder.rb'
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index efd7353e506ab..e656953213183 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -12358,29 +12358,6 @@ The edge type for [`MergeRequestDiff`](#mergerequestdiff).
| <a id="mergerequestdiffedgecursor"></a>`cursor` | [`String!`](#string) | A cursor for use in pagination. |
| <a id="mergerequestdiffedgenode"></a>`node` | [`MergeRequestDiff`](#mergerequestdiff) | The item at the end of the edge. |
-#### `MergeRequestDiffLlmSummaryConnection`
-
-The connection type for [`MergeRequestDiffLlmSummary`](#mergerequestdiffllmsummary).
-
-##### Fields
-
-| Name | Type | Description |
-| ---- | ---- | ----------- |
-| <a id="mergerequestdiffllmsummaryconnectionedges"></a>`edges` | [`[MergeRequestDiffLlmSummaryEdge]`](#mergerequestdiffllmsummaryedge) | A list of edges. |
-| <a id="mergerequestdiffllmsummaryconnectionnodes"></a>`nodes` | [`[MergeRequestDiffLlmSummary]`](#mergerequestdiffllmsummary) | A list of nodes. |
-| <a id="mergerequestdiffllmsummaryconnectionpageinfo"></a>`pageInfo` | [`PageInfo!`](#pageinfo) | Information to aid in pagination. |
-
-#### `MergeRequestDiffLlmSummaryEdge`
-
-The edge type for [`MergeRequestDiffLlmSummary`](#mergerequestdiffllmsummary).
-
-##### Fields
-
-| Name | Type | Description |
-| ---- | ---- | ----------- |
-| <a id="mergerequestdiffllmsummaryedgecursor"></a>`cursor` | [`String!`](#string) | A cursor for use in pagination. |
-| <a id="mergerequestdiffllmsummaryedgenode"></a>`node` | [`MergeRequestDiffLlmSummary`](#mergerequestdiffllmsummary) | The item at the end of the edge. |
-
#### `MergeRequestDiffRegistryConnection`
The connection type for [`MergeRequestDiffRegistry`](#mergerequestdiffregistry).
@@ -22662,7 +22639,6 @@ Defines which user roles, users, or groups can merge into a protected branch.
| <a id="mergerequestdescriptionhtml"></a>`descriptionHtml` | [`String`](#string) | GitLab Flavored Markdown rendering of `description`. |
| <a id="mergerequestdetailedmergestatus"></a>`detailedMergeStatus` | [`DetailedMergeStatus`](#detailedmergestatus) | Detailed merge status of the merge request. |
| <a id="mergerequestdiffheadsha"></a>`diffHeadSha` | [`String`](#string) | Diff head SHA of the merge request. |
-| <a id="mergerequestdiffllmsummaries"></a>`diffLlmSummaries` **{warning-solid}** | [`MergeRequestDiffLlmSummaryConnection`](#mergerequestdiffllmsummaryconnection) | **Introduced** in GitLab 16.1. **Status**: Experiment. Diff summaries generated by AI. |
| <a id="mergerequestdiffrefs"></a>`diffRefs` | [`DiffRefs`](#diffrefs) | References of the base SHA, the head SHA, and the start SHA for this merge request. |
| <a id="mergerequestdiffstatssummary"></a>`diffStatsSummary` | [`DiffStatsSummary`](#diffstatssummary) | Summary of which files were changed in this merge request. |
| <a id="mergerequestdiscussionlocked"></a>`discussionLocked` | [`Boolean!`](#boolean) | Indicates if comments on the merge request are locked to members only. |
@@ -23458,25 +23434,9 @@ A diff version of a merge request.
| Name | Type | Description |
| ---- | ---- | ----------- |
| <a id="mergerequestdiffcreatedat"></a>`createdAt` | [`Time!`](#time) | Timestamp of when the diff was created. |
-| <a id="mergerequestdiffdiffllmsummary"></a>`diffLlmSummary` | [`MergeRequestDiffLlmSummary`](#mergerequestdiffllmsummary) | Diff summary generated by AI. |
| <a id="mergerequestdiffreviewllmsummaries"></a>`reviewLlmSummaries` | [`MergeRequestReviewLlmSummaryConnection`](#mergerequestreviewllmsummaryconnection) | Review summaries generated by AI. (see [Connections](#connections)) |
| <a id="mergerequestdiffupdatedat"></a>`updatedAt` | [`Time!`](#time) | Timestamp of when the diff was updated. |
-### `MergeRequestDiffLlmSummary`
-
-A diff summary generated by AI.
-
-#### Fields
-
-| Name | Type | Description |
-| ---- | ---- | ----------- |
-| <a id="mergerequestdiffllmsummarycontent"></a>`content` | [`String!`](#string) | Content of the diff summary. |
-| <a id="mergerequestdiffllmsummarycreatedat"></a>`createdAt` | [`Time!`](#time) | Timestamp of when the diff summary was created. |
-| <a id="mergerequestdiffllmsummarymergerequestdiffid"></a>`mergeRequestDiffId` | [`ID!`](#id) | ID of the Merge Request diff associated with the diff summary. |
-| <a id="mergerequestdiffllmsummaryprovider"></a>`provider` | [`String!`](#string) | AI provider that generated the summary. |
-| <a id="mergerequestdiffllmsummaryupdatedat"></a>`updatedAt` | [`Time!`](#time) | Timestamp of when the diff summary was updated. |
-| <a id="mergerequestdiffllmsummaryuser"></a>`user` | [`UserCore`](#usercore) | User associated with the diff summary. |
-
### `MergeRequestDiffRegistry`
Represents the Geo sync and verification state of a Merge Request diff.
diff --git a/ee/app/assets/javascripts/merge_requests/queries/summary_notes.query.graphql b/ee/app/assets/javascripts/merge_requests/queries/summary_notes.query.graphql
deleted file mode 100644
index ceb3b7e855651..0000000000000
--- a/ee/app/assets/javascripts/merge_requests/queries/summary_notes.query.graphql
+++ /dev/null
@@ -1,32 +0,0 @@
-#import "~/graphql_shared/fragments/user.fragment.graphql"
-
-query summaryNotes($projectPath: ID!, $iid: String!, $after: String!) {
- project(fullPath: $projectPath) {
- id
- mergeRequest(iid: $iid) {
- id
- mergeRequestDiffs(after: $after) {
- pageInfo {
- endCursor
- hasNextPage
- }
- nodes {
- diffLlmSummary {
- content
- createdAt
- }
- reviewLlmSummaries {
- nodes {
- reviewer {
- ...User
- webUrl
- }
- contentHtml
- createdAt
- }
- }
- }
- }
- }
- }
-}
diff --git a/ee/app/graphql/ee/types/merge_request_type.rb b/ee/app/graphql/ee/types/merge_request_type.rb
index 031fc9f2f5e44..47e6961132336 100644
--- a/ee/app/graphql/ee/types/merge_request_type.rb
+++ b/ee/app/graphql/ee/types/merge_request_type.rb
@@ -39,11 +39,6 @@ module MergeRequestType
' This flag is disabled by default and only available on GitLab.com' \
' because the feature is experimental and is subject to change without notice.'
- field :diff_llm_summaries, ::Types::MergeRequests::DiffLlmSummaryType.connection_type,
- null: true,
- alpha: { milestone: '16.1' },
- description: 'Diff summaries generated by AI'
-
field :blocking_merge_requests, ::Types::MergeRequests::BlockingMergeRequestsType,
null: true,
alpha: { milestone: '16.5' },
@@ -87,15 +82,10 @@ def merge_request_diffs(lookahead:)
# which can result to N+1.
includes = [:merge_request]
- selects_diff_llm_summary =
- lookahead.selection(:nodes).selects?(:diff_llm_summary) ||
- lookahead.selection(:edges).selection(:node).selects?(:diff_llm_summary)
-
selects_review_llm_summaries =
lookahead.selection(:nodes).selects?(:review_llm_summaries) ||
lookahead.selection(:edges).selection(:node).selects?(:review_llm_summaries)
- includes << [merge_request_diff_llm_summary: [:merge_request_diff, :user]] if selects_diff_llm_summary
includes << [merge_request_review_llm_summaries: [:user, { review: [:author] }]] if selects_review_llm_summaries
object.merge_request_diffs.includes(includes)
diff --git a/ee/app/graphql/types/merge_request_diff_type.rb b/ee/app/graphql/types/merge_request_diff_type.rb
index 85bae3a97834a..248c5740fbc9b 100644
--- a/ee/app/graphql/types/merge_request_diff_type.rb
+++ b/ee/app/graphql/types/merge_request_diff_type.rb
@@ -7,11 +7,6 @@ class MergeRequestDiffType < ::Types::BaseObject
authorize :read_merge_request
- field :diff_llm_summary, ::Types::MergeRequests::DiffLlmSummaryType,
- null: true,
- method: :merge_request_diff_llm_summary,
- description: 'Diff summary generated by AI.'
-
field :review_llm_summaries, ::Types::MergeRequests::ReviewLlmSummaryType.connection_type,
null: true,
method: :merge_request_review_llm_summaries,
diff --git a/ee/app/graphql/types/merge_requests/diff_llm_summary_type.rb b/ee/app/graphql/types/merge_requests/diff_llm_summary_type.rb
deleted file mode 100644
index c2e6063eda081..0000000000000
--- a/ee/app/graphql/types/merge_requests/diff_llm_summary_type.rb
+++ /dev/null
@@ -1,37 +0,0 @@
-# frozen_string_literal: true
-
-module Types
- module MergeRequests
- class DiffLlmSummaryType < ::Types::BaseObject
- graphql_name 'MergeRequestDiffLlmSummary'
- description 'A diff summary generated by AI.'
-
- authorize :read_merge_request
-
- field :merge_request_diff_id,
- GraphQL::Types::ID,
- null: false,
- description: 'ID of the Merge Request diff associated with the diff summary.'
-
- field :user, Types::UserType,
- null: true,
- description: 'User associated with the diff summary.'
-
- field :provider, GraphQL::Types::String,
- null: false,
- description: 'AI provider that generated the summary.'
-
- field :content, GraphQL::Types::String,
- null: false,
- description: 'Content of the diff summary.'
-
- field :created_at, Types::TimeType,
- null: false,
- description: 'Timestamp of when the diff summary was created.'
-
- field :updated_at, Types::TimeType,
- null: false,
- description: 'Timestamp of when the diff summary was updated.'
- end
- end
-end
diff --git a/ee/app/helpers/ee/merge_requests_helper.rb b/ee/app/helpers/ee/merge_requests_helper.rb
index b71f2290dd0cb..5e12e3c3b3e7b 100644
--- a/ee/app/helpers/ee/merge_requests_helper.rb
+++ b/ee/app/helpers/ee/merge_requests_helper.rb
@@ -39,23 +39,11 @@ def mr_compare_form_data(user, merge_request)
super.merge({ target_branch_finder_path: target_branch_finder_path })
end
- def summarize_llm_enabled?(project)
- ::Llm::MergeRequests::SummarizeDiffService.enabled?(group: project.root_ancestor)
- end
-
override :review_bar_data
def review_bar_data(merge_request, user)
super.merge({ can_summarize: Ability.allowed?(user, :summarize_draft_code_review, merge_request).to_s })
end
- def diff_llm_summary(merge_request)
- merge_request.latest_merge_request_diff&.merge_request_diff_llm_summary
- end
-
- def truncated_diff_llm_summary(merge_request)
- diff_llm_summary(merge_request).content.truncate(250)
- end
-
def review_llm_summary_allowed?(merge_request, user)
Ability.allowed?(user, :summarize_submitted_review, merge_request)
end
diff --git a/ee/app/helpers/ee/todos_helper.rb b/ee/app/helpers/ee/todos_helper.rb
index ef1dec50176b5..00bfefe1dc506 100644
--- a/ee/app/helpers/ee/todos_helper.rb
+++ b/ee/app/helpers/ee/todos_helper.rb
@@ -19,16 +19,6 @@ def show_todo_state?(todo)
super || (todo.target.is_a?(Epic) && todo.target.state == 'closed')
end
- override :todo_target_path_anchor
- def todo_target_path_anchor(todo)
- if todo.review_requested? && summarize_llm_enabled?(todo.target.project) &&
- diff_llm_summary(todo.target).present?
- return "diff-summary"
- end
-
- super
- end
-
override :todo_action_name
def todo_action_name(todo)
return s_('Todos|have been added as an approver') if todo.action == ::Todo::ADDED_APPROVER
diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb
index 4c5ba68dcd1d9..a487452968688 100644
--- a/ee/app/models/ee/merge_request.rb
+++ b/ee/app/models/ee/merge_request.rb
@@ -477,13 +477,6 @@ def rebase_commit_is_different?(newrev)
rebase_commit_sha != newrev
end
- def diff_llm_summaries
- ::MergeRequest::DiffLlmSummary
- .includes(:user, merge_request_diff: [:merge_request])
- .where(merge_request_diff_id: merge_request_diffs.recent)
- .order(created_at: :desc)
- end
-
def merge_train
target_project.merge_train_for(target_branch)
end
diff --git a/ee/app/models/ee/merge_request_diff.rb b/ee/app/models/ee/merge_request_diff.rb
index 61e311c2fe391..02913d783b8ce 100644
--- a/ee/app/models/ee/merge_request_diff.rb
+++ b/ee/app/models/ee/merge_request_diff.rb
@@ -16,13 +16,10 @@ module MergeRequestDiff
with_replicator ::Geo::MergeRequestDiffReplicator
has_one :merge_request_diff_detail, autosave: false, inverse_of: :merge_request_diff
- has_one :merge_request_diff_llm_summary, class_name: 'MergeRequest::DiffLlmSummary'
has_many :merge_request_review_llm_summaries, class_name: 'MergeRequest::ReviewLlmSummary'
after_save :save_verification_details
- after_create_commit :prepare_diff_summary, unless: :importing?
-
scope :has_external_diffs, -> { with_files.where(stored_externally: true) }
scope :project_id_in, ->(ids) { where(merge_request_id: ::MergeRequest.where(target_project_id: ids)) }
scope :available_replicables, -> { has_external_diffs }
@@ -34,20 +31,6 @@ module MergeRequestDiff
def verification_state_object
merge_request_diff_detail
end
-
- def prepare_diff_summary
- return unless ::Feature.enabled?(:summarize_diff_automatically, project)
- return if merge_head?
- return if empty?
-
- Llm::SummarizeMergeRequestService
- .new(
- merge_request.author,
- merge_request,
- diff_id: id
- )
- .execute
- end
end
class_methods do
diff --git a/ee/app/models/merge_request/diff_llm_summary.rb b/ee/app/models/merge_request/diff_llm_summary.rb
index 5b3d368d0a1d0..7a1f28b60fafe 100644
--- a/ee/app/models/merge_request/diff_llm_summary.rb
+++ b/ee/app/models/merge_request/diff_llm_summary.rb
@@ -1,4 +1,4 @@
-# rubocop:disable Style/ClassAndModuleChildren
+# rubocop:disable Style/ClassAndModuleChildren -- Will be removed when removing the table
# frozen_string_literal: true
class MergeRequest::DiffLlmSummary < ApplicationRecord
diff --git a/ee/app/policies/ee/merge_request_policy.rb b/ee/app/policies/ee/merge_request_policy.rb
index 15dc79e26125f..e5e1f2e1feb51 100644
--- a/ee/app/policies/ee/merge_request_policy.rb
+++ b/ee/app/policies/ee/merge_request_policy.rb
@@ -66,16 +66,6 @@ module MergeRequestPolicy
subject&.project&.custom_roles_enabled?
end
- with_scope :subject
- condition(:summarize_merge_request_enabled) do
- ::Feature.enabled?(:summarize_diff_automatically, subject.project) &&
- subject.project.licensed_feature_available?(:summarize_mr_changes) &&
- ::Gitlab::Llm::FeatureAuthorizer.new(
- container: subject.project,
- feature_name: :summarize_diff
- ).allowed?
- end
-
def read_only?
@subject.target_project&.namespace&.read_only?
end
@@ -121,18 +111,6 @@ def group_access?(protected_branch)
enable :approve_merge_request
end
- rule { can?(:read_merge_request) }.policy do
- enable :generate_diff_summary
- end
-
- rule { llm_bot }.policy do
- enable :generate_diff_summary
- end
-
- rule do
- summarize_merge_request_enabled & can?(:generate_diff_summary)
- end.enable :summarize_merge_request
-
rule do
summarize_draft_code_review_enabled & can?(:read_merge_request)
end.enable :summarize_draft_code_review
diff --git a/ee/app/policies/merge_request/diff_llm_summary_policy.rb b/ee/app/policies/merge_request/diff_llm_summary_policy.rb
deleted file mode 100644
index fae200fdf8a08..0000000000000
--- a/ee/app/policies/merge_request/diff_llm_summary_policy.rb
+++ /dev/null
@@ -1,7 +0,0 @@
-# frozen_string_literal: true
-
-# rubocop:disable Style/ClassAndModuleChildren
-class MergeRequest::DiffLlmSummaryPolicy < BasePolicy
- delegate { @subject.merge_request_diff.project }
-end
-# rubocop:enable Style/ClassAndModuleChildren
diff --git a/ee/app/services/ee/notification_service.rb b/ee/app/services/ee/notification_service.rb
index 6041e9f25d46c..0bec999a35928 100644
--- a/ee/app/services/ee/notification_service.rb
+++ b/ee/app/services/ee/notification_service.rb
@@ -187,17 +187,5 @@ def new_review_deliver_options(review)
options
end
-
- override :review_request_deliver_options
- def review_request_deliver_options(project)
- summary_diff_enabled = ::Llm::MergeRequests::SummarizeDiffService.enabled?(group: project.root_ancestor)
-
- # NOTE: delay the email delivery to allow some time for the summary to be generated by AI
- if summary_diff_enabled
- super.merge(wait: 2.minutes)
- else
- super
- end
- end
end
end
diff --git a/ee/app/services/llm/merge_requests/summarize_diff_service.rb b/ee/app/services/llm/merge_requests/summarize_diff_service.rb
deleted file mode 100644
index 3c695444d4947..0000000000000
--- a/ee/app/services/llm/merge_requests/summarize_diff_service.rb
+++ /dev/null
@@ -1,90 +0,0 @@
-# frozen_string_literal: true
-
-# TODO: This service is deprecated and will be removed in the future in
-# https://gitlab.com/gitlab-org/gitlab/-/issues/423210.
-module Llm
- module MergeRequests
- class SummarizeDiffService
- TRACKING_CONTEXT = { action: 'summarize_diff' }.freeze
-
- def initialize(title:, user:, diff:)
- @title = title
- @user = user
- @diff = diff
- end
-
- def execute
- return unless self.class.enabled?(group: diff.merge_request.project.root_ancestor) &&
- user.can?(:generate_diff_summary, diff.merge_request)
-
- response_modifier.new(response).response_body.presence
- end
-
- def self.enabled?(group:)
- ::Feature.enabled?(:ai_global_switch, type: :ops) &&
- Gitlab::Llm::StageCheck.available?(group, :summarize_diff) &&
- ::License.feature_available?(:summarize_mr_changes)
- end
-
- private
-
- attr_reader :title, :user, :diff
-
- def prompt
- <<~PROMPT
- You are a code assistant, developed to help summarize code in non-technical terms.
-
- ```
- #{extracted_diff}
- ```
-
- The code above, enclosed by three ticks, is the code diff of a merge request. The merge request's
- title is: '#{title}'
-
- Write a summary of the changes in couple sentences, the way an expert engineer would summarize the
- changes using simple - generally non-technical - terms.
-
- You MUST ensure that it is no longer than 1800 characters. A character is considered anything, not only
- letters.
- PROMPT
- end
-
- def summary_message
- prompt
- end
-
- def diff_output(old_path, new_path, diff)
- <<~DIFF
- --- #{old_path}
- +++ #{new_path}
- #{diff}
- DIFF
- end
-
- def extracted_diff
- # Each diff string starts with information about the lines changed,
- # bracketed by @@. Removing this saves us tokens.
- #
- # Ex: @@ -0,0 +1,58 @@\n+# frozen_string_literal: true\n+\n+module MergeRequests\n+
- #
- diff.raw_diffs.to_a.map do |diff|
- next unless diff.diff.encoding == Encoding::UTF_8
-
- diff_output(diff.old_path, diff.new_path, diff.diff.sub(Gitlab::Regex.git_diff_prefix, ""))
- end.join.truncate_words(750)
- end
-
- def response_modifier
- ::Gitlab::Llm::VertexAi::ResponseModifiers::Predictions
- end
-
- def response
- Gitlab::Llm::VertexAi::Client.new(user, tracking_context: TRACKING_CONTEXT)
- .text(content: summary_message)
- end
- end
- end
-end
-
-# Added for JiHu
-Llm::MergeRequests::SummarizeDiffService.prepend_mod
diff --git a/ee/app/services/llm/summarize_merge_request_service.rb b/ee/app/services/llm/summarize_merge_request_service.rb
deleted file mode 100644
index fb79915729cd4..0000000000000
--- a/ee/app/services/llm/summarize_merge_request_service.rb
+++ /dev/null
@@ -1,21 +0,0 @@
-# frozen_string_literal: true
-
-module Llm
- class SummarizeMergeRequestService < ::Llm::BaseService
- private
-
- def ai_action
- :summarize_merge_request
- end
-
- def perform
- schedule_completion_worker
- end
-
- def valid?
- super &&
- resource.to_ability_name == "merge_request" &&
- Ability.allowed?(user, :summarize_merge_request, resource)
- end
- end
-end
diff --git a/ee/app/views/notify/_diff_summary.html.haml b/ee/app/views/notify/_diff_summary.html.haml
deleted file mode 100644
index ff3a242684e05..0000000000000
--- a/ee/app/views/notify/_diff_summary.html.haml
+++ /dev/null
@@ -1,8 +0,0 @@
-- if summarize_llm_enabled?(@merge_request.project)
- - diff_llm_summary = diff_llm_summary(@merge_request)
- - if diff_llm_summary.present?
- .md{ style: 'border-bottom:1px solid #ededed; padding-bottom: 1em; padding-top: 1em;' }
- %h4= _('Summary')
- = markdown(diff_llm_summary.content, pipeline: :email)
- %p{ style: 'color: #737278;' }
- = _('Summary generated by AI (Experiment)')
diff --git a/ee/app/views/notify/_diff_summary.text.erb b/ee/app/views/notify/_diff_summary.text.erb
deleted file mode 100644
index 2b8e6c5f539e2..0000000000000
--- a/ee/app/views/notify/_diff_summary.text.erb
+++ /dev/null
@@ -1,14 +0,0 @@
-<% if summarize_llm_enabled?(@merge_request.project) -%>
-<% diff_llm_summary = diff_llm_summary(@merge_request) -%>
-<% if diff_llm_summary.present? -%>
-
---
-
-Summary
-
-<%= diff_llm_summary.content %>
-
-<%= _('Summary generated by AI (Experiment)') -%>
-
-<% end -%>
-<% end -%>
diff --git a/ee/config/feature_flags/development/summarize_diff_automatically.yml b/ee/config/feature_flags/development/summarize_diff_automatically.yml
deleted file mode 100644
index 416dfe337bdf1..0000000000000
--- a/ee/config/feature_flags/development/summarize_diff_automatically.yml
+++ /dev/null
@@ -1,8 +0,0 @@
----
-name: summarize_diff_automatically
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/118812
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/409135
-milestone: '16.1'
-type: development
-group: group::code review
-default_enabled: false
diff --git a/ee/lib/gitlab/duo/developments/setup.rb b/ee/lib/gitlab/duo/developments/setup.rb
index bcd8d63e12e47..6de7f08a426fb 100644
--- a/ee/lib/gitlab/duo/developments/setup.rb
+++ b/ee/lib/gitlab/duo/developments/setup.rb
@@ -67,7 +67,6 @@ def ensure_feature_flags
::Feature.enable(:ai_global_switch)
::Feature.enable(:ai_duo_chat_switch)
- ::Feature.enable(:summarize_diff_automatically)
::Feature.enable(:summarize_my_code_review)
::Feature.enable(:automatically_summarize_mr_review)
::Feature.enable(:fill_in_mr_template)
diff --git a/ee/lib/gitlab/llm/completions_factory.rb b/ee/lib/gitlab/llm/completions_factory.rb
index a4b3eb34016ee..496f423ae8c9c 100644
--- a/ee/lib/gitlab/llm/completions_factory.rb
+++ b/ee/lib/gitlab/llm/completions_factory.rb
@@ -59,11 +59,6 @@ class CompletionsFactory
prompt_class: ::Gitlab::Llm::Templates::SummarizeSubmittedReview,
feature_category: :code_review_workflow
},
- summarize_merge_request: {
- service_class: ::Gitlab::Llm::VertexAi::Completions::SummarizeMergeRequest,
- prompt_class: ::Gitlab::Llm::Templates::SummarizeMergeRequest,
- feature_category: :code_review_workflow
- },
summarize_new_merge_request: {
service_class: ::Gitlab::Llm::VertexAi::Completions::SummarizeNewMergeRequest,
prompt_class: ::Gitlab::Llm::Templates::SummarizeNewMergeRequest,
diff --git a/ee/lib/gitlab/llm/stage_check.rb b/ee/lib/gitlab/llm/stage_check.rb
index 486106de6c0f3..f83c028ee9f11 100644
--- a/ee/lib/gitlab/llm/stage_check.rb
+++ b/ee/lib/gitlab/llm/stage_check.rb
@@ -9,7 +9,6 @@ class StageCheck
:summarize_my_mr_code_review,
:explain_code,
:generate_description,
- :summarize_diff,
:explain_vulnerability,
:resolve_vulnerability,
:generate_commit_message,
diff --git a/ee/lib/gitlab/llm/templates/summarize_merge_request.rb b/ee/lib/gitlab/llm/templates/summarize_merge_request.rb
deleted file mode 100644
index 8a3770c14aa6a..0000000000000
--- a/ee/lib/gitlab/llm/templates/summarize_merge_request.rb
+++ /dev/null
@@ -1,62 +0,0 @@
-# frozen_string_literal: true
-
-module Gitlab
- module Llm
- module Templates
- class SummarizeMergeRequest
- include Gitlab::Utils::StrongMemoize
-
- def initialize(merge_request, mr_diff)
- @merge_request = merge_request
- @mr_diff = mr_diff
- end
-
- def to_prompt
- return if extracted_diff.blank?
-
- <<~PROMPT
- You are a code assistant, developed to help summarize code in non-technical terms.
-
- ```
- #{extracted_diff}
- ```
-
- The code above, enclosed by three ticks, is the code diff of a merge request.
-
- Write a summary of the changes in couple sentences, the way an expert engineer would summarize the
- changes using simple - generally non-technical - terms.
-
- You MUST ensure that it is no longer than 1800 characters. A character is considered anything, not only
- letters.
- PROMPT
- end
-
- private
-
- attr_reader :merge_request, :mr_diff
-
- def extracted_diff
- # Each diff string starts with information about the lines changed,
- # bracketed by @@. Removing this saves us tokens.
- #
- # Ex: @@ -0,0 +1,58 @@\n+# frozen_string_literal: true\n+\n+module MergeRequests\n+
- #
- mr_diff.raw_diffs.to_a.map do |diff|
- next if diff.diff.encoding != Encoding::UTF_8 || diff.has_binary_notice?
-
- diff_output(diff.old_path, diff.new_path, diff.diff.sub(Gitlab::Regex.git_diff_prefix, ""))
- end.join.truncate_words(750)
- end
- strong_memoize_attr :extracted_diff
-
- def diff_output(old_path, new_path, diff)
- <<~DIFF
- --- #{old_path}
- +++ #{new_path}
- #{diff}
- DIFF
- end
- end
- end
- end
-end
diff --git a/ee/lib/gitlab/llm/vertex_ai/completions/summarize_merge_request.rb b/ee/lib/gitlab/llm/vertex_ai/completions/summarize_merge_request.rb
deleted file mode 100644
index 3cd29a477b563..0000000000000
--- a/ee/lib/gitlab/llm/vertex_ai/completions/summarize_merge_request.rb
+++ /dev/null
@@ -1,60 +0,0 @@
-# frozen_string_literal: true
-
-module Gitlab
- module Llm
- module VertexAi
- module Completions
- class SummarizeMergeRequest < Gitlab::Llm::Completions::Base
- # rubocop:disable CodeReuse/ActiveRecord
- def execute
- mr_diff = merge_request.merge_request_diffs.find_by(id: options[:diff_id])
-
- return unless mr_diff.present?
-
- prompt = generate_prompt(merge_request, mr_diff)
-
- return unless prompt.present?
-
- response = response_for(user, prompt)
- response_modifier = ::Gitlab::Llm::VertexAi::ResponseModifiers::Predictions.new(response)
-
- store_response(response_modifier, mr_diff)
- end
- # rubocop:enable CodeReuse/ActiveRecord
-
- private
-
- def merge_request
- resource
- end
-
- def generate_prompt(merge_request, mr_diff)
- ai_prompt_class.new(merge_request, mr_diff).to_prompt
- end
-
- def response_for(user, prompt)
- ::Gitlab::Llm::VertexAi::Client
- .new(user, tracking_context: tracking_context)
- .text(
- content: prompt,
- parameters: ::Gitlab::Llm::VertexAi::Configuration.payload_parameters(temperature: 0)
- )
- end
-
- def store_response(response_modifier, mr_diff)
- return if response_modifier.errors.any? || response_modifier.response_body.blank?
-
- summary = MergeRequest::DiffLlmSummary.new(
- merge_request_diff: mr_diff,
- content: response_modifier.response_body,
- provider: MergeRequest::DiffLlmSummary.providers[:vertex_ai]
- )
-
- summary.save! if summary.valid?
- summary
- end
- end
- end
- end
- end
-end
diff --git a/ee/spec/factories/ai_messages.rb b/ee/spec/factories/ai_messages.rb
index 27fbf2c850ba1..68cfde86fce39 100644
--- a/ee/spec/factories/ai_messages.rb
+++ b/ee/spec/factories/ai_messages.rb
@@ -69,10 +69,6 @@
ai_action { :generate_commit_message }
end
- trait :summarize_merge_request do
- ai_action { :summarize_merge_request }
- end
-
trait :summarize_review do
ai_action { :summarize_review }
end
diff --git a/ee/spec/graphql/ee/types/merge_request_type_spec.rb b/ee/spec/graphql/ee/types/merge_request_type_spec.rb
index f8722f64c10dc..9200fbeba1069 100644
--- a/ee/spec/graphql/ee/types/merge_request_type_spec.rb
+++ b/ee/spec/graphql/ee/types/merge_request_type_spec.rb
@@ -11,7 +11,6 @@
it { expect(described_class).to have_graphql_field(:has_security_reports, calls_gitaly?: true) }
it { expect(described_class).to have_graphql_field(:security_reports_up_to_date_on_target_branch, calls_gitaly?: true) }
it { expect(described_class).to have_graphql_field(:suggested_reviewers) }
- it { expect(described_class).to have_graphql_field(:diff_llm_summaries) }
it { expect(described_class).to have_graphql_field(:blocking_merge_requests) }
it { expect(described_class).to have_graphql_field(:merge_request_diffs) }
diff --git a/ee/spec/graphql/types/merge_request_diff_type_spec.rb b/ee/spec/graphql/types/merge_request_diff_type_spec.rb
index 6564cf23ee095..d7f6b979695e3 100644
--- a/ee/spec/graphql/types/merge_request_diff_type_spec.rb
+++ b/ee/spec/graphql/types/merge_request_diff_type_spec.rb
@@ -5,7 +5,6 @@
RSpec.describe GitlabSchema.types['MergeRequestDiff'], feature_category: :code_review_workflow do
let(:fields) do
%i[
- diff_llm_summary
review_llm_summaries
created_at
updated_at
diff --git a/ee/spec/graphql/types/merge_requests/diff_llm_summary_type_spec.rb b/ee/spec/graphql/types/merge_requests/diff_llm_summary_type_spec.rb
deleted file mode 100644
index 27639444e38fe..0000000000000
--- a/ee/spec/graphql/types/merge_requests/diff_llm_summary_type_spec.rb
+++ /dev/null
@@ -1,19 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe GitlabSchema.types['MergeRequestDiffLlmSummary'], feature_category: :code_review_workflow do
- let(:fields) do
- %i[
- user
- merge_request_diff_id
- provider
- content
- created_at
- updated_at
- ]
- end
-
- it { expect(described_class).to have_graphql_fields(fields) }
- it { expect(described_class).to require_graphql_authorizations(:read_merge_request) }
-end
diff --git a/ee/spec/helpers/ee/merge_requests_helper_spec.rb b/ee/spec/helpers/ee/merge_requests_helper_spec.rb
index 66f84840019f0..ffdfeb2cee10a 100644
--- a/ee/spec/helpers/ee/merge_requests_helper_spec.rb
+++ b/ee/spec/helpers/ee/merge_requests_helper_spec.rb
@@ -100,37 +100,6 @@
end
end
- describe '#summarize_llm_enabled?' do
- let_it_be(:user) { build_stubbed(:user) }
- let_it_be(:group) { build_stubbed(:group) }
- let_it_be(:project) { build_stubbed(:project, namespace: group) }
-
- it 'calls Llm::MergeRequests::SummarizeDiffService enabled? method' do
- expect(Llm::MergeRequests::SummarizeDiffService).to receive(:enabled?).with(group: group)
-
- summarize_llm_enabled?(project)
- end
- end
-
- describe '#diff_llm_summary' do
- let(:merge_request) { instance_double('MergeRequest') }
- let(:summary) { instance_double('MergeRequestDiff', merge_request_diff_llm_summary: 'summary') }
-
- before do
- allow(merge_request).to receive(:latest_merge_request_diff).and_return(summary)
- end
-
- context 'when merge request has summary' do
- it { expect(helper.diff_llm_summary(merge_request)).to eq('summary') }
- end
-
- context 'when merge request has does not have summary' do
- let(:summary) { nil }
-
- it { expect(helper.diff_llm_summary(merge_request)).to eq(nil) }
- end
- end
-
describe '#review_llm_summary_allowed?' do
let(:user) { build_stubbed(:user) }
let(:merge_request) { build_stubbed(:merge_request) }
diff --git a/ee/spec/helpers/ee/todos_helper_spec.rb b/ee/spec/helpers/ee/todos_helper_spec.rb
index 7fed2b8c09fab..7f1cdccd11522 100644
--- a/ee/spec/helpers/ee/todos_helper_spec.rb
+++ b/ee/spec/helpers/ee/todos_helper_spec.rb
@@ -165,44 +165,6 @@
it { expect(helper.todo_target_path_anchor(todo)).to eq(nil) }
end
-
- describe 'with a review requested todo' do
- let_it_be(:todo) do
- create(:todo,
- :review_requested,
- user: user,
- project: project,
- target: merge_request,
- author: author)
- end
-
- context 'when the summarize LLM feature is disabled' do
- before do
- allow(::Llm::MergeRequests::SummarizeDiffService).to receive(:enabled?).and_return(false)
- end
-
- it { expect(helper.todo_target_path_anchor(todo)).to eq(nil) }
- end
-
- context 'when the summarize LLM feature is enabled' do
- let(:summary) { instance_double('MergeRequestDiff', merge_request_diff_llm_summary: 'summary') }
-
- before do
- allow(merge_request).to receive(:latest_merge_request_diff).and_return(summary)
- allow(::Llm::MergeRequests::SummarizeDiffService).to receive(:enabled?).and_return(true)
- end
-
- context 'without llm summary' do
- let(:summary) { instance_double('MergeRequestDiff', merge_request_diff_llm_summary: nil) }
-
- it { expect(helper.todo_target_path_anchor(todo)).to eq(nil) }
- end
-
- context 'with llm summary' do
- it { expect(helper.todo_target_path_anchor(todo)).to eq('diff-summary') }
- end
- end
- end
end
describe '#todo_action_name' do
diff --git a/ee/spec/lib/gitlab/llm/templates/summarize_merge_request_spec.rb b/ee/spec/lib/gitlab/llm/templates/summarize_merge_request_spec.rb
deleted file mode 100644
index 5c455dafbc401..0000000000000
--- a/ee/spec/lib/gitlab/llm/templates/summarize_merge_request_spec.rb
+++ /dev/null
@@ -1,69 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::Llm::Templates::SummarizeMergeRequest, feature_category: :code_review_workflow do
- let_it_be(:project) { create(:project, :repository) }
-
- let(:merge_request) do
- create(
- :merge_request,
- source_branch: source_branch,
- target_branch: 'master',
- source_project: project,
- target_project: project
- )
- end
-
- let(:mr_diff) { merge_request.merge_request_diff }
- let(:source_branch) { 'feature' }
-
- subject { described_class.new(merge_request, mr_diff) }
-
- describe '#to_prompt' do
- it 'includes raw diff' do
- expect(subject.to_prompt)
- .to include("+class Feature\n+ def foo\n+ puts 'bar'\n+ end\n+end")
- end
-
- context 'when a diff contains the binary notice' do
- let(:source_branch) { 'signed-commits' }
-
- it 'does not contain the binary diff' do
- mr_diff.raw_diffs.to_a[0].diff = "@@ -0,0 +1 @@hellothere\n+🌚\n"
-
- binary_message = Gitlab::Git::Diff.binary_message('a', 'b')
- binary_notice = binary_message
- mr_diff.raw_diffs.to_a[1].diff = binary_notice
-
- expect(subject.to_prompt).to include("hellothere")
- expect(subject.to_prompt).not_to include(binary_message)
- end
- end
-
- context 'when a diff is not encoded with UTF-8' do
- let(:source_branch) { 'signed-commits' }
-
- it 'does not raise any error and not contain the non-UTF diff' do
- mr_diff.raw_diffs.to_a[0].diff = "@@ -0,0 +1 @@hellothere\n+🌚\n"
-
- non_utf_diff = "@@ -1 +1 @@\n-This should not be in the prompt\n+#{(0..255).map(&:chr).join}\n"
- mr_diff.raw_diffs.to_a[1].diff = non_utf_diff
-
- expect { subject.to_prompt }.not_to raise_error
- expect(subject.to_prompt).to include("hellothere")
- expect(subject.to_prompt).not_to include("This should not be in the prompt")
- end
- end
-
- context 'when extracted diff is blank' do
- before do
- allow(mr_diff).to receive(:raw_diffs).and_return([])
- end
-
- it 'returns nil' do
- expect(subject.to_prompt).to be_nil
- end
- end
- end
-end
diff --git a/ee/spec/lib/gitlab/llm/vertex_ai/completions/summarize_merge_request_spec.rb b/ee/spec/lib/gitlab/llm/vertex_ai/completions/summarize_merge_request_spec.rb
deleted file mode 100644
index 3ea8b02f5b926..0000000000000
--- a/ee/spec/lib/gitlab/llm/vertex_ai/completions/summarize_merge_request_spec.rb
+++ /dev/null
@@ -1,176 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::Llm::VertexAi::Completions::SummarizeMergeRequest, 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(:mr_diff) { merge_request.merge_request_diff }
- let_it_be(:user) { merge_request.author }
-
- let(:prompt_class) { Gitlab::Llm::Templates::SummarizeMergeRequest }
- let(:diff_id) { mr_diff.id }
- let(:tracking_context) { { action: :summarize_merge_request, request_id: 'uuid' } }
- let(:options) do
- {
- diff_id: diff_id,
- action: :summarize_merge_request
- }
- end
-
- let(:prompt_message) do
- build(:ai_message, :summarize_merge_request, user: user, resource: merge_request, request_id: 'uuid')
- end
-
- subject { described_class.new(prompt_message, prompt_class, options) }
-
- describe '#execute' do
- let(:prompt) { "This is a prompt." }
-
- let(:payload_parameters) do
- {
- temperature: 0,
- maxOutputTokens: 1024,
- topK: 40,
- topP: 0.95
- }
- end
-
- before do
- allow_next_instance_of(prompt_class) do |template|
- allow(template).to receive(:to_prompt).and_return(prompt)
- end
-
- allow(::Gitlab::Llm::VertexAi::Configuration)
- .to receive(:payload_parameters)
- .with(temperature: 0)
- .and_return(payload_parameters)
- end
-
- context 'when specific diff_id does not exist' do
- let(:diff_id) { mr_diff.id + 1 }
-
- it 'does not make a request to AI provider' do
- expect(Gitlab::Llm::VertexAi::Client).not_to receive(:new)
-
- subject.execute
- end
- end
-
- context 'when generated prompt is nil' do
- let(:prompt) { nil }
-
- it 'does not make a request to AI provider' do
- expect(Gitlab::Llm::VertexAi::Client).not_to receive(:new)
-
- subject.execute
- end
- end
-
- context 'when the text 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, user, tracking_context: tracking_context) do |client|
- allow(client)
- .to receive(:text)
- .with(content: prompt, parameters: payload_parameters)
- .and_return(example_response.to_json)
- end
- end
-
- it 'stores the content from the AI response' do
- expect { subject.execute }
- .to change { ::MergeRequest::DiffLlmSummary.count }
- .by(1)
-
- diff_llm_summary = mr_diff.merge_request_diff_llm_summary
-
- aggregate_failures do
- expect(diff_llm_summary.merge_request_diff).to eq(mr_diff)
- expect(diff_llm_summary.provider).to eq('vertex_ai')
- expect(diff_llm_summary.content).to eq('Summary generated by AI')
- end
- end
-
- context 'when the AI response is too big' do
- let(:example_answer) { "i" * 3000 }
-
- it 'does not store the content' do
- expect { subject.execute }
- .not_to change { ::MergeRequest::DiffLlmSummary.count }
- end
-
- it 'returns unsaved Active Record object' do
- response = subject.execute
-
- expect(response).to be_a(MergeRequest::DiffLlmSummary)
- expect(response).not_to be_persisted
- end
-
- it 'does not raise an error' do
- expect { subject.execute }
- .not_to raise_error
- end
- end
- end
-
- context 'when the text client returns an unsuccessful response' do
- let(:error) { { error: { message: 'Error' } } }
-
- before do
- allow_next_instance_of(Gitlab::Llm::VertexAi::Client, user, tracking_context: tracking_context) do |client|
- allow(client)
- .to receive(:text)
- .with(content: prompt, parameters: payload_parameters)
- .and_return(error.to_json)
- end
- end
-
- it 'does not store the content' do
- expect { subject.execute }
- .not_to change { ::MergeRequest::DiffLlmSummary.count }
- end
- end
-
- context 'when the AI response is empty' do
- before do
- allow_next_instance_of(Gitlab::Llm::VertexAi::Client, user, tracking_context: tracking_context) do |client|
- allow(client)
- .to receive(:text)
- .with(content: prompt, parameters: payload_parameters)
- .and_return({})
- end
- end
-
- it 'does not store the content' do
- expect { subject.execute }
- .not_to change { ::MergeRequest::DiffLlmSummary.count }
- end
-
- it 'does not raise an error' do
- expect { subject.execute }
- .not_to raise_error
- end
- end
- end
-end
diff --git a/ee/spec/mailers/notify_spec.rb b/ee/spec/mailers/notify_spec.rb
index 2a79caf48fc14..5982751736a73 100644
--- a/ee/spec/mailers/notify_spec.rb
+++ b/ee/spec/mailers/notify_spec.rb
@@ -223,50 +223,6 @@
end
end
end
-
- context 'for merge request with reviewer' do
- describe 'merge request diff summary' do
- let_it_be(:merge_request_diff) { create(:merge_request_diff, merge_request: merge_request) }
- let_it_be(:recipient) { create(:user) }
-
- subject { described_class.request_review_merge_request_email(recipient.id, merge_request.id, merge_request.author_id) }
-
- before do
- allow(::Llm::MergeRequests::SummarizeDiffService).to receive(:enabled?).and_return(summarize_diff_service_enabled)
- end
-
- context 'when merge_request_diff_llm_summary exists' do
- let_it_be(:merge_request_diff_llm_summary) { create(:merge_request_diff_llm_summary, merge_request_diff: merge_request_diff) }
-
- context 'when SummarizeDiffService is disabled' do
- let(:summarize_diff_service_enabled) { false }
-
- it 'does not include diff summary' do
- expect(subject.text_part.body.raw_source).not_to include(merge_request_diff_llm_summary.content)
- end
- end
-
- context 'when SummarizeDiffService is enabled' do
- let(:summarize_diff_service_enabled) { true }
-
- it 'includes diff summary' do
- expect(subject.text_part.body.raw_source).to include(merge_request_diff_llm_summary.content)
- expect(subject.text_part.body.raw_source).to include('Summary generated by AI (Experiment)')
- end
- end
- end
-
- context 'when merge_request_diff_llm_summary is empty' do
- context 'when SummarizeDiffService is enabled' do
- let(:summarize_diff_service_enabled) { true }
-
- it 'does not include diff summary' do
- expect(subject.text_part.body.raw_source).not_to include('Summary generated by AI (Experiment)')
- end
- end
- end
- end
- end
end
context 'for a group' do
diff --git a/ee/spec/models/ee/merge_request_diff_spec.rb b/ee/spec/models/ee/merge_request_diff_spec.rb
index d8549e75a3103..a3bca4dc0942a 100644
--- a/ee/spec/models/ee/merge_request_diff_spec.rb
+++ b/ee/spec/models/ee/merge_request_diff_spec.rb
@@ -70,66 +70,6 @@
end
end
- describe 'prepare_diff_summary' do
- let_it_be(:merge_request) { create(:merge_request, :skip_diff_creation, target_project: project, source_project: project) }
-
- let(:diff_with_commits) do
- create(
- :merge_request_diff,
- merge_request: merge_request,
- start_commit_sha: merge_request.target_branch_sha,
- head_commit_sha: merge_request.source_branch_sha
- )
- end
-
- before do
- stub_licensed_features(summarize_mr_changes: true)
- end
-
- it 'executes Llm::SummarizeMergeRequestService' do
- expect_next_instance_of(
- ::Llm::SummarizeMergeRequestService,
- merge_request.author,
- merge_request,
- diff_id: kind_of(Numeric)
- ) do |svc|
- expect(svc).to receive(:execute)
- end
-
- diff_with_commits
- end
-
- context 'when diff is empty' do
- it 'does not execute Llm::SummarizeMergeRequestService' do
- expect(Llm::SummarizeMergeRequestService).not_to receive(:new)
-
- create(:merge_request_diff, merge_request: merge_request, state: :empty)
- end
- end
-
- context 'when summarize_diff_automatically feature flag is off' do
- before do
- stub_feature_flags(summarize_diff_automatically: false)
- end
-
- it 'does not call Llm::SummarizeMergeRequestService' do
- expect(::Llm::SummarizeMergeRequestService).not_to receive(:new)
-
- diff_with_commits
- end
- end
-
- context 'when the diff is not merge_head' do
- let(:diff_with_commits) { create(:merge_request_diff, :merge_head) }
-
- it 'does not call Llm::SummarizeMergeRequestService' do
- expect(::Llm::SummarizeMergeRequestService).not_to receive(:new)
-
- diff_with_commits
- end
- end
- end
-
describe '.search' do
let_it_be(:merge_request_diff1) { create(:merge_request_diff) }
let_it_be(:merge_request_diff2) { create(:merge_request_diff) }
diff --git a/ee/spec/models/merge_request/diff_llm_summary_spec.rb b/ee/spec/models/merge_request/diff_llm_summary_spec.rb
deleted file mode 100644
index 860457add62b0..0000000000000
--- a/ee/spec/models/merge_request/diff_llm_summary_spec.rb
+++ /dev/null
@@ -1,18 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe ::MergeRequest::DiffLlmSummary, feature_category: :code_review_workflow do
- let_it_be_with_reload(:project) { create(:project, :repository) }
-
- subject(:merge_request_diff_llm_summary) { build(:merge_request_diff_llm_summary) }
-
- describe 'associations' do
- it { is_expected.to belong_to(:merge_request_diff) }
- it { is_expected.to belong_to(:user).optional }
- it { is_expected.to validate_uniqueness_of(:merge_request_diff_id) }
- it { is_expected.to validate_presence_of(:content) }
- it { is_expected.to validate_length_of(:content).is_at_most(2056) }
- it { is_expected.to validate_presence_of(:provider) }
- end
-end
diff --git a/ee/spec/models/merge_request_spec.rb b/ee/spec/models/merge_request_spec.rb
index 72df502e51953..98ec2bd2b14f1 100644
--- a/ee/spec/models/merge_request_spec.rb
+++ b/ee/spec/models/merge_request_spec.rb
@@ -2081,21 +2081,6 @@ def create_mr(metrics_data = {})
end
end
- describe '#diff_llm_summaries' do
- let(:merge_request) { create(:merge_request) }
- let!(:mr_diff_1) { create(:merge_request_diff, merge_request: merge_request) }
- let!(:mr_diff_2) { create(:merge_request_diff, merge_request: merge_request) }
- let!(:mr_diff_3) { create(:merge_request_diff, merge_request: merge_request) }
- let!(:other_mr_diff) { create(:merge_request_diff) }
- let!(:mr_diff_summary_1) { create(:merge_request_diff_llm_summary, merge_request_diff: mr_diff_1) }
- let!(:mr_diff_summary_3) { create(:merge_request_diff_llm_summary, merge_request_diff: mr_diff_3) }
- let!(:other_mr_diff_summary) { create(:merge_request_diff_llm_summary, merge_request_diff: other_mr_diff) }
-
- it 'returns recent MergeRequest::DiffLlmSummary records associated to merge request diffs' do
- expect(merge_request.diff_llm_summaries).to eq([mr_diff_summary_3, mr_diff_summary_1])
- end
- end
-
describe '#merge_train' do
subject { merge_request.merge_train }
diff --git a/ee/spec/policies/merge_request/diff_llm_summary_policy_spec.rb b/ee/spec/policies/merge_request/diff_llm_summary_policy_spec.rb
deleted file mode 100644
index 16258c2c3073f..0000000000000
--- a/ee/spec/policies/merge_request/diff_llm_summary_policy_spec.rb
+++ /dev/null
@@ -1,26 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe MergeRequest::DiffLlmSummaryPolicy, feature_category: :code_review_workflow do
- let_it_be_with_reload(:project) { create(:project) }
- let_it_be(:user) { create(:user) }
- let_it_be(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
- let_it_be(:mr_diff_summary) do
- create(:merge_request_diff_llm_summary, merge_request_diff: merge_request.merge_request_diff)
- end
-
- subject(:policy) { described_class.new(user, mr_diff_summary) }
-
- context 'when user is not permitted to read merge request' do
- it { is_expected.to be_disallowed(:read_merge_request) }
- end
-
- context 'when user is permitted to read merge request' do
- before do
- project.add_developer(user)
- end
-
- it { is_expected.to be_allowed(:read_merge_request) }
- end
-end
diff --git a/ee/spec/policies/merge_request_policy_spec.rb b/ee/spec/policies/merge_request_policy_spec.rb
index 875755f5bda0e..67b1dd37a05ad 100644
--- a/ee/spec/policies/merge_request_policy_spec.rb
+++ b/ee/spec/policies/merge_request_policy_spec.rb
@@ -506,86 +506,4 @@ def policy_for(user)
end
end
end
-
- describe 'when enabling generate diff summary permission' do
- let_it_be(:project) { create(:project) }
- let_it_be(:user) { create(:user) }
-
- context 'when can read_merge_request' do
- before do
- project.add_developer(user)
- end
-
- it 'allows to generate_diff_summary' do
- expect(policy_for(user)).to be_allowed(:generate_diff_summary)
- end
- end
-
- context 'when can not read_merge_request' do
- it 'does not allow to generate_diff_summary' do
- expect(policy_for(user)).not_to be_allowed(:generate_diff_summary)
- end
-
- context 'and when is the LLM bot' do
- let(:user) { create(:user, :llm_bot) }
-
- it 'allows to generate_diff_summary' do
- expect(policy_for(user)).to be_allowed(:generate_diff_summary)
- end
- end
- end
- end
-
- describe 'summarize_merge_request policy' do
- let_it_be(:project) { create(:project) }
- let_it_be(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
- let_it_be(:current_user) { developer }
-
- let(:authorizer) { instance_double(::Gitlab::Llm::FeatureAuthorizer) }
-
- subject { described_class.new(current_user, merge_request) }
-
- before do
- stub_licensed_features(summarize_mr_changes: true)
- allow(::Gitlab::Llm::FeatureAuthorizer).to receive(:new).and_return(authorizer)
- end
-
- context "when feature is authorized" do
- before do
- allow(authorizer).to receive(:allowed?).and_return(true)
- end
-
- it { is_expected.to be_allowed(:summarize_merge_request) }
-
- context 'when summarize_diff_automatically feature flag is disabled' do
- before do
- stub_feature_flags(summarize_diff_automatically: false)
- end
-
- it { is_expected.to be_disallowed(:summarize_merge_request) }
- end
-
- context 'when license is not set' do
- before do
- stub_licensed_features(summarize_mr_changes: false)
- end
-
- it { is_expected.to be_disallowed(:summarize_merge_request) }
- end
-
- context 'when user cannot generate_diff_summary' do
- let(:current_user) { guest }
-
- it { is_expected.to be_disallowed(:summarize_merge_request) }
- end
- end
-
- context "when feature is not authorized" do
- before do
- allow(authorizer).to receive(:allowed?).and_return(false)
- end
-
- it { is_expected.to be_disallowed(:summarize_draft_code_review) }
- end
- end
end
diff --git a/ee/spec/requests/api/graphql/project/merge_request_spec.rb b/ee/spec/requests/api/graphql/project/merge_request_spec.rb
index 13f932afde9e4..bde6d98b7bf62 100644
--- a/ee/spec/requests/api/graphql/project/merge_request_spec.rb
+++ b/ee/spec/requests/api/graphql/project/merge_request_spec.rb
@@ -88,60 +88,11 @@
end
end
- describe 'diffLlmSummaries' do
- let(:mr_fields) { "diffLlmSummaries { nodes { mergeRequestDiffId content } }" }
-
- context 'when there are MergeRequest::DiffLlmSummary records associated to MR' do
- let!(:mr_diff_1) { create(:merge_request_diff, merge_request: merge_request) }
- let!(:mr_diff_2) { create(:merge_request_diff, merge_request: merge_request) }
- let!(:mr_diff_summary_1) { create(:merge_request_diff_llm_summary, merge_request_diff: mr_diff_1) }
- let!(:mr_diff_summary_2) { create(:merge_request_diff_llm_summary, merge_request_diff: mr_diff_2) }
-
- it 'returns the diff summaries' do
- post_graphql(query, current_user: current_user)
-
- expect(merge_request_graphql_data).to eq({
- 'diffLlmSummaries' => {
- 'nodes' => [
- {
- 'mergeRequestDiffId' => mr_diff_2.id.to_s,
- 'content' => mr_diff_summary_2.content
- },
- {
- 'mergeRequestDiffId' => mr_diff_1.id.to_s,
- 'content' => mr_diff_summary_1.content
- }
- ]
- }
- })
- end
- end
-
- context 'when there are no MergeRequest::DiffLlmSummary records associated to MR' do
- it 'returns empty nodes' do
- post_graphql(query, current_user: current_user)
-
- expect(merge_request_graphql_data).to eq({
- 'diffLlmSummaries' => {
- 'nodes' => []
- }
- })
- end
- end
- end
-
describe 'mergeRequestDiffs' do
let(:mr_fields) do
<<-GQL
mergeRequestDiffs {
nodes {
- diffLlmSummary {
- mergeRequestDiffId
- content
- user {
- id
- }
- }
reviewLlmSummaries {
nodes {
mergeRequestDiffId
@@ -163,26 +114,17 @@
let_it_be(:mr_diff_1) { create(:merge_request_diff, merge_request: merge_request) }
let_it_be(:mr_diff_2) { create(:merge_request_diff, merge_request: merge_request) }
- context 'when there are diff and review summaries associated to MR diffs' do
- let_it_be(:mr_diff_summary_1) { create(:merge_request_diff_llm_summary, merge_request_diff: mr_diff_1) }
- let_it_be(:mr_diff_summary_2) { create(:merge_request_diff_llm_summary, merge_request_diff: mr_diff_2) }
+ context 'when there are review summaries associated to MR diffs' do
let_it_be(:mr_review_summary_1) { create(:merge_request_review_llm_summary, merge_request_diff: mr_diff_1) }
let_it_be(:mr_review_summary_2) { create(:merge_request_review_llm_summary, merge_request_diff: mr_diff_2) }
- it 'returns the diff and review summaries' do
+ it 'returns the review summaries' do
post_graphql(query, current_user: current_user)
expect(merge_request_graphql_data).to eq({
'mergeRequestDiffs' => {
'nodes' => [
{
- 'diffLlmSummary' => {
- 'mergeRequestDiffId' => mr_diff_2.id.to_s,
- 'content' => mr_diff_summary_2.content,
- 'user' => {
- 'id' => mr_diff_summary_2.user.to_gid.to_s
- }
- },
'reviewLlmSummaries' => {
'nodes' => [
{
@@ -199,13 +141,6 @@
}
},
{
- 'diffLlmSummary' => {
- 'mergeRequestDiffId' => mr_diff_1.id.to_s,
- 'content' => mr_diff_summary_1.content,
- 'user' => {
- 'id' => mr_diff_summary_1.user.to_gid.to_s
- }
- },
'reviewLlmSummaries' => {
'nodes' => [
{
@@ -236,7 +171,6 @@
expect_graphql_errors_to_be_empty
mr_diff_3 = create(:merge_request_diff, merge_request: merge_request)
- create(:merge_request_diff_llm_summary, merge_request_diff: mr_diff_3)
create(:merge_request_review_llm_summary, merge_request_diff: mr_diff_3)
expect { post_graphql(query, current_user: current_user) }.not_to exceed_all_query_limit(control)
@@ -244,7 +178,7 @@
end
end
- context 'when there are no diff and review summaries associated to MR diffs' do
+ context 'when there are no review summaries associated to MR diffs' do
it 'returns empty nodes' do
post_graphql(query, current_user: current_user)
@@ -252,11 +186,9 @@
'mergeRequestDiffs' => {
'nodes' => [
{
- 'diffLlmSummary' => nil,
'reviewLlmSummaries' => { 'nodes' => [] }
},
{
- 'diffLlmSummary' => nil,
'reviewLlmSummaries' => { 'nodes' => [] }
}
]
diff --git a/ee/spec/services/ee/notification_service_spec.rb b/ee/spec/services/ee/notification_service_spec.rb
index f8f2eab5f1eed..c3dec4b50ecf4 100644
--- a/ee/spec/services/ee/notification_service_spec.rb
+++ b/ee/spec/services/ee/notification_service_spec.rb
@@ -912,40 +912,6 @@ def add_users_with_subscription(project, issuable)
# Make the watcher a subscriber to detect dupes
issuable.subscriptions.create!(user: @watcher_and_subscriber, project: project, subscribed: true)
end
-
- describe '#review_requested_of_merge_request' do
- let_it_be(:reviewer) { create(:user) }
-
- let(:merge_request) { create(:merge_request, source_project: project, reviewers: [reviewer]) }
- let(:current_user) { merge_request.author }
- let(:mailer) { double }
-
- before do
- allow(::Llm::MergeRequests::SummarizeDiffService).to receive(:enabled?).and_return(summarize_diff_service_enabled)
- allow(Notify).to receive(:request_review_merge_request_email)
- .with(Integer, Integer, Integer, String).and_return(mailer)
- end
-
- context 'when SummarizeDiffService is disabled' do
- let(:summarize_diff_service_enabled) { false }
-
- it 'does not call the summarize worker' do
- expect(mailer).to receive(:deliver_later).with({})
-
- notification.review_requested_of_merge_request(merge_request, current_user, reviewer)
- end
- end
-
- context 'when SummarizeDiffService is enabled' do
- let(:summarize_diff_service_enabled) { true }
-
- it 'does not call the summarize worker' do
- expect(mailer).to receive(:deliver_later).with({ wait: 2.minutes })
-
- notification.review_requested_of_merge_request(merge_request, current_user, reviewer)
- end
- end
- end
end
context 'IncidentManagement' do
diff --git a/ee/spec/services/llm/merge_requests/summarize_diff_service_spec.rb b/ee/spec/services/llm/merge_requests/summarize_diff_service_spec.rb
deleted file mode 100644
index 6e73af88fe701..0000000000000
--- a/ee/spec/services/llm/merge_requests/summarize_diff_service_spec.rb
+++ /dev/null
@@ -1,151 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Llm::MergeRequests::SummarizeDiffService, feature_category: :code_review_workflow do
- let_it_be(:user) { create(:user) }
- let_it_be(:project) { create(:project, :with_namespace_settings, :repository, :public) }
- let_it_be(:merge_request) { create(:merge_request, source_project: project) }
-
- let_it_be(:merge_request_2) { create(:merge_request) }
- let_it_be(:project_2) { merge_request_2.project }
-
- let_it_be(:example_answer) { "This merge request includes changes to limit the transfer..." }
- let_it_be(:example_response) do
- {
- "predictions" => [
- {
- "candidates" => [
- {
- "author" => "",
- "content" => example_answer
- }
- ],
- "safetyAttributes" => {
- "categories" => ["Violent"],
- "scores" => [0.4000000059604645],
- "blocked" => false
- }
- }
- ],
- "deployedModelId" => "1",
- "model" => "projects/1/locations/us-central1/models/text-bison",
- "modelDisplayName" => "text-bison",
- "modelVersionId" => "1"
- }
- end
-
- let(:tracking_context) { { action: 'summarize_diff' } }
- let(:response_double) { example_response.to_json }
- let(:errored_response_double) { { error: "true" }.to_json }
-
- subject(:service) do
- described_class.new(title: merge_request.title, user: user, diff:
- merge_request.merge_request_diff)
- end
-
- describe "#execute" do
- before do
- project.add_developer(user)
- stub_licensed_features(summarize_mr_changes: true, ai_features: true)
-
- merge_request.project.namespace.namespace_settings.update_attribute(:experiment_features_enabled, true)
- end
-
- context "when the user does not have read access to the MR" do
- it "returns without attempting to summarize" do
- secondary_service = described_class.new(title: merge_request_2.title, user: user, diff:
- merge_request_2.merge_request_diff)
-
- expect(secondary_service).not_to receive(:llm_client)
- expect(secondary_service.execute).to be_nil
- end
- end
-
- context "when the feature is not enabled" do
- context 'when the ai_global_switch flag is false' do
- before do
- stub_feature_flags(ai_global_switch: false)
- end
-
- it "returns without attempting to summarize" do
- expect(service).not_to receive(:llm_client)
-
- service.execute
- end
- end
-
- context 'when summarize_mr_change feature not avaliable' do
- before do
- stub_licensed_features(summarize_mr_changes: false)
- end
-
- it "returns without attempting to summarize" do
- expect(service).not_to receive(:llm_client)
-
- service.execute
- end
- end
-
- context 'when the project experiment_features_allowed is false' do
- before do
- merge_request.project.namespace.namespace_settings.update_attribute(:experiment_features_enabled, false)
- end
-
- it "returns without attempting to summarize" do
- expect(service).not_to receive(:llm_client)
-
- service.execute
- end
- end
- end
-
- context "when Gitlab::Llm::VertexAi::Client.text returns a typical response" do
- before do
- allow_next_instance_of(Gitlab::Llm::VertexAi::Client, user, tracking_context: tracking_context) do |llm_client|
- allow(llm_client).to receive(:text).and_return(response_double)
- end
- end
-
- it "returns the content field from the VertexAI response" do
- expect(service.execute).to eq(example_answer)
- end
- end
-
- context "when Gitlab::Llm::VertexAi::Client.text returns an unsuccessful response" do
- before do
- allow_next_instance_of(Gitlab::Llm::VertexAi::Client, user, tracking_context: tracking_context) do |llm_client|
- allow(llm_client).to receive(:text).and_return(errored_response_double)
- end
- end
-
- it "returns nil" do
- expect(service.execute).to be_nil
- end
- end
-
- context "when Gitlab::Llm::VertexAi::Client.text returns an nil response" do
- before do
- allow_next_instance_of(Gitlab::Llm::VertexAi::Client, user, tracking_context: tracking_context) do |llm_client|
- allow(llm_client).to receive(:text).and_return(nil)
- end
- end
-
- it "returns nil" do
- expect(service.execute).to be_nil
- end
- end
-
- context "when Gitlab::Llm::VertexAi::Client.text returns a response without parsed_response" do
- before do
- allow_next_instance_of(Gitlab::Llm::VertexAi::Client, user, tracking_context: tracking_context) do |llm_client|
- allow(llm_client).to receive(:text).and_return({ message: "Foo" }.to_json)
- end
- end
-
- it "returns nil" do
- expect(service.execute).to be_nil
- end
- end
- end
-end
diff --git a/ee/spec/services/llm/summarize_merge_request_service_spec.rb b/ee/spec/services/llm/summarize_merge_request_service_spec.rb
deleted file mode 100644
index 9493e1e5d13a1..0000000000000
--- a/ee/spec/services/llm/summarize_merge_request_service_spec.rb
+++ /dev/null
@@ -1,51 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Llm::SummarizeMergeRequestService, :saas, feature_category: :code_review_workflow do
- let_it_be(:user) { create(:user) }
- let_it_be_with_reload(:group) { create(:group_with_plan, plan: :ultimate_plan) }
- 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_merge_request_enabled) { true }
- let(:current_user) { user }
- let(:options) { {} }
-
- describe '#perform' do
- include_context 'with ai features enabled for group'
-
- let(:action_name) { :summarize_merge_request }
- let(:content) { 'Summarize merge request' }
-
- 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_merge_request, resource)
- .and_return(summarize_merge_request_enabled)
- end
-
- subject { described_class.new(current_user, resource, options).execute }
-
- it_behaves_like 'schedules completion worker' do
- subject { described_class.new(current_user, resource, options) }
- 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_merge_request' do
- let(:summarize_merge_request_enabled) { false }
-
- it { is_expected.to be_error.and have_attributes(message: eq(described_class::INVALID_MESSAGE)) }
- end
- end
-end
diff --git a/spec/factories/merge_requests_diff_llm_summary.rb b/spec/factories/merge_requests_diff_llm_summary.rb
deleted file mode 100644
index fc67f8442caf5..0000000000000
--- a/spec/factories/merge_requests_diff_llm_summary.rb
+++ /dev/null
@@ -1,10 +0,0 @@
-# frozen_string_literal: true
-
-FactoryBot.define do
- factory :merge_request_diff_llm_summary, class: 'MergeRequest::DiffLlmSummary' do
- association :user, factory: :user
- association :merge_request_diff, factory: :merge_request_diff
- provider { 0 }
- content { FFaker::Lorem.sentence }
- end
-end
--
GitLab