From 8983b62c82fc63abb902801dd2552bb875e787c4 Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Tue, 1 Aug 2023 10:26:36 +0900 Subject: [PATCH 1/9] Support explain code for blobs If the current user is viewing some code blob, the user should be able to ask the chat to explain the code. We will inject the blob's code into the zeroshot executor's prompt and ask the LLMs to directly explain the code when instructed. To make that possible, we will make use of the Referer header to detect if a user is viewing a Blob. The referer url will be added as an option to be extracted by CompletionWorker. CompletionWorker will then attempt to resolve and authorize the blob pointed to by the referer url. If the blob is found and authorized, it will be available as a context attribute, 'extra_resource'. The zeroshot executor can then use the attribute to include the code blob and additional prompt. The change is guarded with a feature flag. --- .../development/explain_current_blob.yml | 8 ++ ee/app/finders/llm/extra_resource_finder.rb | 79 +++++++++++++++ ee/app/graphql/mutations/ai/action.rb | 2 + ee/app/workers/llm/completion_worker.rb | 2 + .../llm/chain/agents/zero_shot/executor.rb | 15 ++- .../chain/agents/zero_shot/prompts/base.rb | 4 +- ee/lib/gitlab/llm/chain/gitlab_context.rb | 5 +- ee/lib/gitlab/llm/chain/utils/prompt.rb | 11 ++- ee/lib/gitlab/llm/completions/chat.rb | 4 +- .../finders/llm/extra_resource_finder_spec.rb | 96 +++++++++++++++++++ ee/spec/graphql/mutations/ai/action_spec.rb | 23 ++++- .../chain/agents/zero_shot/executor_spec.rb | 89 ++++++++++++++++- .../zero_shot/prompts/anthropic_spec.rb | 3 +- .../zero_shot/prompts/vertex_ai_spec.rb | 3 +- .../lib/gitlab/llm/chain/utils/prompt_spec.rb | 16 ++++ .../lib/gitlab/llm/completions/chat_spec.rb | 17 +++- ee/spec/workers/llm/completion_worker_spec.rb | 21 +++- spec/support/finder_collection_allowlist.yml | 1 + 18 files changed, 375 insertions(+), 24 deletions(-) create mode 100644 config/feature_flags/development/explain_current_blob.yml create mode 100644 ee/app/finders/llm/extra_resource_finder.rb create mode 100644 ee/spec/finders/llm/extra_resource_finder_spec.rb diff --git a/config/feature_flags/development/explain_current_blob.yml b/config/feature_flags/development/explain_current_blob.yml new file mode 100644 index 00000000000000..e296748a3f79fb --- /dev/null +++ b/config/feature_flags/development/explain_current_blob.yml @@ -0,0 +1,8 @@ +--- +name: explain_current_blob +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/128342/ +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/420959 +milestone: '16.3' +type: development +group: group::ai framework +default_enabled: false diff --git a/ee/app/finders/llm/extra_resource_finder.rb b/ee/app/finders/llm/extra_resource_finder.rb new file mode 100644 index 00000000000000..2730b384b66c2e --- /dev/null +++ b/ee/app/finders/llm/extra_resource_finder.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +module Llm + # ExtraResourceFinder attempts to locate a resource based on `referer_url` + # Presently, the finder only deals with a Blob resource. + # Since the finder does not deal with DB resources, it's been added to spec/support/finder_collection_allowlist.yml. + # As more resource types need to be supported (potentially), appropriate abstractions should be designed and added. + class ExtraResourceFinder + # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/422133 + # The module is meant to be used in controllers. + include ::ExtractsRef + + def initialize(current_user, referer_url) + @current_user = current_user + @referer_url = referer_url + @extra_resources = {} + end + + def execute + find_blob_resource + + @extra_resources + end + + private + + def find_blob_resource + return unless Feature.enabled?(:explain_current_blob, @current_user) && @referer_url + + project_fullpath, resource_path = parse_referer(@referer_url) + return unless project_fullpath && resource_path + + @project = find_project(project_fullpath) + return unless @project && @current_user.can?(:read_code, @project) && @project.repository + + ref, path = extract_blob_ref_and_path(resource_path) + return unless ref && path + + blob = find_blob(ref, path) + + @extra_resources[:blob] = blob if blob && blob.readable_text? + end + + def find_project(project_fullpath) + project = Project.find_by_full_path(project_fullpath) + return unless project && @current_user.can?(:read_code, project) && project.repository + + project + end + + def find_blob(ref, path) + commit = @project.repository.commit(ref) + return if commit.nil? + + @project.repository.blob_at(commit.id, path) + end + + def parse_referer(referer_url) + referer_url.split("#{Gitlab.config.gitlab.base_url}/")[1].try(:split, "/-/", 2) + end + + def extract_blob_ref_and_path(resource_path) + return unless resource_path.start_with?("blob/") + + resource_path = resource_path + .sub('blob/', '') # Trim `blob/` + .split(%r{\#|\?}, 2) # Extract up to the first occurence of # or ? (URL anchor/param) + .first.tap { |blob_path| blob_path || "" } + return if resource_path.empty? + + extract_ref(resource_path) + end + + # Required to use the method `extract_ref` from ExtractsRef + def repository_container + @project + end + end +end diff --git a/ee/app/graphql/mutations/ai/action.rb b/ee/app/graphql/mutations/ai/action.rb index 0c03e933aad1fe..25663e9b6419ec 100644 --- a/ee/app/graphql/mutations/ai/action.rb +++ b/ee/app/graphql/mutations/ai/action.rb @@ -36,6 +36,8 @@ def resolve(**attributes) resource_id, method, options = extract_method_params!(attributes) resource = resource_id&.then { |id| authorized_find!(id: id) } + options[:referer_url] = context[:request].headers["Referer"] + response = Llm::ExecuteMethodService.new(current_user, resource, method, options).execute { diff --git a/ee/app/workers/llm/completion_worker.rb b/ee/app/workers/llm/completion_worker.rb index 8e1153dd0f6135..8e274b78d92616 100644 --- a/ee/app/workers/llm/completion_worker.rb +++ b/ee/app/workers/llm/completion_worker.rb @@ -28,6 +28,8 @@ def perform(user_id, resource_id, resource_class, ai_action_name, options = {}) resource = find_resource(resource_id, resource_class) return if resource && !user.can?("read_#{resource.to_ability_name}", resource) + options[:extra_resource] = ::Llm::ExtraResourceFinder.new(user, options.delete(:referer_url)).execute + params = options.extract!(:request_id, :internal_request, :cache_response) logger.debug(message: "Params", params: params) ai_completion = ::Gitlab::Llm::CompletionsFactory.completion(ai_action_name.to_sym, params) diff --git a/ee/lib/gitlab/llm/chain/agents/zero_shot/executor.rb b/ee/lib/gitlab/llm/chain/agents/zero_shot/executor.rb index 3aef4d892f4ebc..a5bc2c240e85f1 100644 --- a/ee/lib/gitlab/llm/chain/agents/zero_shot/executor.rb +++ b/ee/lib/gitlab/llm/chain/agents/zero_shot/executor.rb @@ -90,7 +90,9 @@ def options user_input: user_input, agent_scratchpad: +"", conversation: conversation, - prompt_version: prompt_version + prompt_version: prompt_version, + current_code: current_code, + explain_current_blob: Feature.enabled?(:explain_current_blob, context.current_user) } end @@ -116,6 +118,15 @@ def conversation by_request.values.sort_by { |messages| messages.first.timestamp }.flatten end + def current_code + return "" unless Feature.enabled?(:explain_current_blob, context.current_user) + + blob = @context.extra_resource[:blob] + return "" unless blob + + "The current code file that user sees is #{blob.path} and has the following content\n#{blob.data}\n\n" + end + PROMPT_TEMPLATE = [ Utils::Prompt.as_system( <<~PROMPT @@ -141,7 +152,7 @@ def conversation Final Answer: the final answer to the original input question. When concluding your response, provide the final answer as "Final Answer:" as soon as the answer is recognized. - + %s If no tool is needed, give a final answer with "Action: DirectAnswer" for the Action parameter and skip writing an Observation. Begin! PROMPT diff --git a/ee/lib/gitlab/llm/chain/agents/zero_shot/prompts/base.rb b/ee/lib/gitlab/llm/chain/agents/zero_shot/prompts/base.rb index d36d7d89724018..bbd0d61cccfecc 100644 --- a/ee/lib/gitlab/llm/chain/agents/zero_shot/prompts/base.rb +++ b/ee/lib/gitlab/llm/chain/agents/zero_shot/prompts/base.rb @@ -12,8 +12,10 @@ def self.base_prompt(options) options.fetch(:prompt_version), options ) + explain_current_blob = options[:explain_current_blob] + default_system_prompt = Utils::Prompt.default_system_prompt(explain_current_blob: explain_current_blob) - "#{Utils::Prompt.default_system_prompt}\n\n#{base_prompt}" + "#{default_system_prompt}\n\n#{base_prompt}" end end end diff --git a/ee/lib/gitlab/llm/chain/gitlab_context.rb b/ee/lib/gitlab/llm/chain/gitlab_context.rb index 481e6527ad29e1..dba25a0cae3b6f 100644 --- a/ee/lib/gitlab/llm/chain/gitlab_context.rb +++ b/ee/lib/gitlab/llm/chain/gitlab_context.rb @@ -4,14 +4,15 @@ module Gitlab module Llm module Chain class GitlabContext - attr_accessor :current_user, :container, :resource, :ai_request, :tools_used + attr_accessor :current_user, :container, :resource, :ai_request, :tools_used, :extra_resource - def initialize(current_user:, container:, resource:, ai_request:, tools_used: []) + def initialize(current_user:, container:, resource:, ai_request:, tools_used: [], extra_resource: {}) @current_user = current_user @container = container @resource = resource @ai_request = ai_request @tools_used = tools_used + @extra_resource = extra_resource end end end diff --git a/ee/lib/gitlab/llm/chain/utils/prompt.rb b/ee/lib/gitlab/llm/chain/utils/prompt.rb index 75327d8754dd4b..df0524b0be4360 100644 --- a/ee/lib/gitlab/llm/chain/utils/prompt.rb +++ b/ee/lib/gitlab/llm/chain/utils/prompt.rb @@ -33,7 +33,15 @@ def self.role_conversation(prompt_template, input_variables) end.to_json end - def self.default_system_prompt + def self.default_system_prompt(explain_current_blob: false) + # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/420959 + # Remove the conditional along with the feature flag. + explain_code_prompt = if explain_current_blob + "\nYou can explain code if the user provided a code snippet and answer directly." + else + "" + end + <<~PROMPT You are a DevSecOps Assistant named '#{Gitlab::Llm::Chain::Agents::ZeroShot::Executor::AGENT_NAME}' created by GitLab. @@ -49,6 +57,7 @@ def self.default_system_prompt The generated code should be formatted in markdown. If a question cannot be answered with the tools and information given, answer politely that you don’t know. + #{explain_code_prompt} If the question is to write or generate new code you should always answer directly. When no tool matches you should answer the question directly. diff --git a/ee/lib/gitlab/llm/completions/chat.rb b/ee/lib/gitlab/llm/completions/chat.rb index 777a0fd4d781ff..01dcda699980d4 100644 --- a/ee/lib/gitlab/llm/completions/chat.rb +++ b/ee/lib/gitlab/llm/completions/chat.rb @@ -15,11 +15,13 @@ def execute(user, resource, options) # one we like. At the moment Anthropic is default and some features may not be supported # by other providers. ai_request = ::Gitlab::Llm::Chain::Requests::Anthropic.new(user) + extra_resource_options = options.extract!(:extra_resource) context = ::Gitlab::Llm::Chain::GitlabContext.new( current_user: user, container: resource.try(:resource_parent)&.root_ancestor, resource: resource, - ai_request: ai_request + ai_request: ai_request, + extra_resource: extra_resource_options[:extra_resource] || {} ) response = Gitlab::Llm::Chain::Agents::ZeroShot::Executor.new( diff --git a/ee/spec/finders/llm/extra_resource_finder_spec.rb b/ee/spec/finders/llm/extra_resource_finder_spec.rb new file mode 100644 index 00000000000000..6b43bec9a177aa --- /dev/null +++ b/ee/spec/finders/llm/extra_resource_finder_spec.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Llm::ExtraResourceFinder, feature_category: :ai_abstraction_layer do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:developer) { create(:user).tap { |u| project.add_developer(u) } } + let_it_be(:guest) { create(:user).tap { |u| project.add_guest(u) } } + let_it_be(:issue) { create(:issue, project: project) } + + let(:current_user) { developer } + let(:blob_url) { Gitlab::Routing.url_helpers.project_blob_url(project, project.default_branch) } + + describe '.execute' do + subject(:execute) { described_class.new(current_user, referer_url).execute } + + context 'with an invalid or non-resource referer_url' do + where(:referer_url) do + [ + [nil], + [''], + ['foo'], + [Gitlab.config.gitlab.base_url], + [lazy { "#{blob_url}/?" }] + ] + end + + with_them do + it 'returns an empty hash' do + expect(execute).to be_empty + end + end + end + + context 'when referer_url references a resource other than Blob' do + let(:referer_url) { ::Gitlab::Routing.url_helpers.project_issue_url(project, issue.id) } + + it 'returns an empty hash' do + expect(execute).to be_empty + end + end + + context 'when referer_url references a Blob' do + let(:referer_url) { "#{blob_url}/#{path}" } + + context 'when referer_url references a valid blob' do + let(:path) { 'files/ruby/popen.rb' } + + context 'when the blob is a readable text' do + let(:expected_blob) { project.repository.blob_at(project.default_branch, path) } + + it 'returns the blob' do + expect(expected_blob).not_to eq(nil) + expect(execute[:blob].id).to eq(expected_blob.id) + end + + context 'when the feature flag :explain_current_blob is disabled for user' do + before do + stub_feature_flags(explain_current_blob: guest) + end + + it 'returns an empty hash' do + expect(execute).to be_empty + end + end + + context "when user is not authorized to read code for the blob's project" do + let(:current_user) { guest } + + it 'returns an empty hash' do + expect(execute).to be_empty + end + end + end + + context 'when the blob is not a readable text' do + let(:non_readable_blob) { project.repository.blob_at(project.default_branch, path) } + let(:path) { 'Gemfile.zip' } + + it 'returns an empty hash' do + expect(non_readable_blob).not_to eq(nil) + expect(execute).to be_empty + end + end + end + + context 'when referer_url references a non-existing blob' do + let(:path) { 'foobar.rb' } + + it 'returns an empty hash' do + expect(execute).to be_empty + end + end + end + end +end diff --git a/ee/spec/graphql/mutations/ai/action_spec.rb b/ee/spec/graphql/mutations/ai/action_spec.rb index 29a3bfdda9013c..1c969bc76f619e 100644 --- a/ee/spec/graphql/mutations/ai/action_spec.rb +++ b/ee/spec/graphql/mutations/ai/action_spec.rb @@ -7,8 +7,11 @@ let_it_be(:resource, reload: true) { create(:issue) } let(:resource_id) { resource.to_gid.to_s } let(:request_id) { 'uuid' } + let(:request) { instance_double(ActionDispatch::Request, headers: { "Referer" => "foobar" }) } + let(:context) { { current_user: user, request: request } } + let(:base_options) { { referer_url: "foobar" } } - subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) } + subject(:mutation) { described_class.new(object: nil, context: context, field: nil) } describe '#ready?' do let(:arguments) { { summarize_comments: { resource_id: resource_id }, markup_format: :markdown } } @@ -115,7 +118,7 @@ user, resource, expected_method, - expected_options + base_options.merge(expected_options) ) do |svc| expect(svc) .to receive(:execute) @@ -136,7 +139,7 @@ user, nil, :chat, - expected_options + base_options.merge(expected_options) ) do |svc| expect(svc) .to receive(:execute) @@ -155,7 +158,7 @@ user, resource, expected_method, - expected_options + base_options.merge(expected_options) ) do |svc| expect(svc) .to receive(:execute) @@ -169,6 +172,18 @@ end end + context 'when chat input is set ' do + let_it_be(:project) { create(:project, :repository).tap { |p| p.add_developer(user) } } + let_it_be(:issue) { create(:issue, project: project) } + + let(:input) { { chat: { resource_id: resource_id } } } + + it_behaves_like 'an AI action' do + let(:expected_method) { :chat } + let(:expected_options) { {} } + end + end + context 'when summarize_comments input is set' do let(:input) { { summarize_comments: { resource_id: resource_id } } } let(:expected_method) { :summarize_comments } diff --git a/ee/spec/lib/gitlab/llm/chain/agents/zero_shot/executor_spec.rb b/ee/spec/lib/gitlab/llm/chain/agents/zero_shot/executor_spec.rb index f9016c4391d213..0f7dd8f39a8920 100644 --- a/ee/spec/lib/gitlab/llm/chain/agents/zero_shot/executor_spec.rb +++ b/ee/spec/lib/gitlab/llm/chain/agents/zero_shot/executor_spec.rb @@ -10,12 +10,15 @@ let(:tool_answer) { instance_double(Gitlab::Llm::Chain::Answer, is_final?: false, content: 'Bar', status: :ok) } let(:tool_double) { instance_double(Gitlab::Llm::Chain::Tools::IssueIdentifier::Executor) } let(:tools) { [Gitlab::Llm::Chain::Tools::IssueIdentifier] } + let(:extra_resource) { {} } let(:response_double) { "I know the final answer\nFinal Answer: FooBar" } + let(:resource) { user } let(:context) do Gitlab::Llm::Chain::GitlabContext.new( - current_user: user, container: nil, resource: user, ai_request: ai_request_double, - tools_used: [Gitlab::Llm::Chain::Tools::IssueIdentifier, Gitlab::Llm::Chain::Tools::IssueIdentifier] + current_user: user, container: nil, resource: resource, ai_request: ai_request_double, + tools_used: [Gitlab::Llm::Chain::Tools::IssueIdentifier, Gitlab::Llm::Chain::Tools::IssueIdentifier], + extra_resource: extra_resource ) end @@ -155,14 +158,46 @@ agent.prompt end + + context 'when resource is a blob' do + let(:project) { create(:project, :repository) } + let(:blob) { project.repository.blob_at("master", "README") } + let(:extra_resource) { { blob: blob } } + + let(:injected_prompt) do + "The current code file that user sees is #{blob.path} and has the following content\n#{blob.data}" + end + + before do + stub_feature_flags(explain_current_blob: user) + end + + it 'includes the blob data in the prompt' do + expect(agent.prompt[:prompt]).to include injected_prompt + end + + context 'when the feature flag explain_current_blob is disabled for current user' do + let(:other_user) { create(:user) } + + before do + stub_feature_flags(explain_current_blob: other_user) + end + + it 'omits the blob data in the prompt' do + expect(agent.prompt[:prompt]).to exclude injected_prompt + end + end + end end describe 'real requests', :real_ai_request, :saas do using RSpec::Parameterized::TableSyntax let_it_be_with_reload(:group) { create(:group_with_plan, :public, plan: :ultimate_plan) } - let_it_be(:project) { create(:project, group: group) } + let_it_be(:project) { create(:project, :repository, group: group) } + let(:resource) { user } + let(:extra_resource) { {} } let(:executor) do ai_request = ::Gitlab::Llm::Chain::Requests::Anthropic.new(user) @@ -170,7 +205,8 @@ current_user: user, container: resource.try(:resource_parent)&.root_ancestor, resource: resource, - ai_request: ai_request + ai_request: ai_request, + extra_resource: extra_resource ) described_class.new( @@ -200,6 +236,51 @@ end end + context 'with blob as resource' do + let(:blob) { project.repository.blob_at("master", "files/ruby/popen.rb") } + let(:extra_resource) { { blob: blob } } + + context 'when the feature flag :explain_current_blob is enabled for user' do + where(:input_template, :tools, :answer_match) do + 'Explain the code' | [] | /ruby|popen/i + 'Explain this code' | [] | /ruby|popen/i + 'What is this code doing?' | [] | /ruby|popen/i + 'Can you explain the code ""def hello_world\\nputs(\""Hello, world!\\n\"");\nend""?' | [] | /hello/i + end + + with_them do + let(:input) { input_template } + + before do + stub_feature_flags(explain_current_blob: user) + end + + it_behaves_like 'successful prompt processing' + end + end + + context 'when the feature flag :explain_current_blob is disabled for user' do + let_it_be(:other_user) { create(:user) } + + where(:input_template, :tools, :answer_match) do + 'Explain the code' | [] | /react/i # Hallucinates by making up react code + 'Explain this code' | [] | /react/i # Hallucinates by making up react code + 'What is this code doing?' | [] | /react/i # Hallucinates by making up react code + 'Can you explain the code ""def hello_world\\nputs(\""Hello, world!\\n\"");\nend""?' | [] | /hello/i + end + + with_them do + let(:input) { input_template } + + before do + stub_feature_flags(explain_current_blob: other_user) + end + + it_behaves_like 'successful prompt processing' + end + end + end + context 'with predefined issue', time_travel_to: Time.utc(2023, 8, 11) do let_it_be(:label) { create(:label, project: project, title: 'ai-enablement') } let_it_be(:milestone) { create(:milestone, project: project, title: 'milestone1', due_date: 3.days.from_now) } diff --git a/ee/spec/lib/gitlab/llm/chain/agents/zero_shot/prompts/anthropic_spec.rb b/ee/spec/lib/gitlab/llm/chain/agents/zero_shot/prompts/anthropic_spec.rb index 3631a5c7b9d081..0dd66f65d0f439 100644 --- a/ee/spec/lib/gitlab/llm/chain/agents/zero_shot/prompts/anthropic_spec.rb +++ b/ee/spec/lib/gitlab/llm/chain/agents/zero_shot/prompts/anthropic_spec.rb @@ -25,7 +25,8 @@ 'request_id' => 'uuid1', 'role' => 'assistant', 'content' => 'response 2', 'timestamp' => Time.current.to_s ) ], - prompt_version: prompt_version + prompt_version: prompt_version, + current_code: "" } end diff --git a/ee/spec/lib/gitlab/llm/chain/agents/zero_shot/prompts/vertex_ai_spec.rb b/ee/spec/lib/gitlab/llm/chain/agents/zero_shot/prompts/vertex_ai_spec.rb index a10cea46752ae1..b3f00189ee552f 100644 --- a/ee/spec/lib/gitlab/llm/chain/agents/zero_shot/prompts/vertex_ai_spec.rb +++ b/ee/spec/lib/gitlab/llm/chain/agents/zero_shot/prompts/vertex_ai_spec.rb @@ -10,7 +10,8 @@ tool_names: "tool names", user_input: 'foo?', agent_scratchpad: "some observation", - prompt_version: ::Gitlab::Llm::Chain::Agents::ZeroShot::Executor::PROMPT_TEMPLATE + prompt_version: ::Gitlab::Llm::Chain::Agents::ZeroShot::Executor::PROMPT_TEMPLATE, + current_code: "" } prompt = described_class.prompt(options)[:prompt] prompt_text = "Answer the question as accurate as you can." diff --git a/ee/spec/lib/gitlab/llm/chain/utils/prompt_spec.rb b/ee/spec/lib/gitlab/llm/chain/utils/prompt_spec.rb index 8abed54a260fb0..db257a22f01c7e 100644 --- a/ee/spec/lib/gitlab/llm/chain/utils/prompt_spec.rb +++ b/ee/spec/lib/gitlab/llm/chain/utils/prompt_spec.rb @@ -38,4 +38,20 @@ expect(described_class.role_conversation([prompt], input_vars)).to eq([result].to_json) end end + + describe "#default_system_prompt" do + let(:explain_current_blob) do + "You can explain code if the user provided a code snippet and answer directly." + end + + it 'includes the prompt to explain code directly' do + expect(described_class.default_system_prompt(explain_current_blob: true)).to include explain_current_blob + end + + context 'when explain_current_blob is false by default' do + it 'omits the prompt to explain code directly' do + expect(described_class.default_system_prompt).to exclude explain_current_blob + end + end + end end diff --git a/ee/spec/lib/gitlab/llm/completions/chat_spec.rb b/ee/spec/lib/gitlab/llm/completions/chat_spec.rb index f3081837aa3349..6bf8e12d446c23 100644 --- a/ee/spec/lib/gitlab/llm/completions/chat_spec.rb +++ b/ee/spec/lib/gitlab/llm/completions/chat_spec.rb @@ -3,15 +3,20 @@ require 'spec_helper' RSpec.describe Gitlab::Llm::Completions::Chat, feature_category: :shared do + include FakeBlobHelpers + let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group) } - let_it_be(:project) { create(:project, group: group) } - let_it_be(:resource) { create(:issue, project: project) } + let_it_be(:project) { create(:project, :repository, group: group) } + let_it_be(:issue) { create(:issue, project: project) } + let(:resource) { issue } let(:expected_container) { group } let(:content) { 'Summarize issue' } let(:ai_request) { instance_double(Gitlab::Llm::Chain::Requests::Anthropic) } - let(:options) { { content: content } } + let(:blob) { fake_blob(path: 'file.md') } + let(:extra_resource) { { blob: blob } } + let(:options) { { request_id: 'uuid', content: content, extra_resource: extra_resource } } let(:container) { group } let(:context) do instance_double( @@ -51,7 +56,8 @@ .to receive(:increment) .with(labels: { tool: "IssueIdentifier" }, success: true) expect(::Gitlab::Llm::Chain::GitlabContext).to receive(:new) - .with(current_user: user, container: expected_container, resource: resource, ai_request: ai_request) + .with(current_user: user, container: expected_container, resource: resource, ai_request: ai_request, + extra_resource: extra_resource) .and_return(context) subject @@ -143,7 +149,8 @@ end expect(::Gitlab::Llm::Chain::GitlabContext).to receive(:new) - .with(current_user: user, container: expected_container, resource: resource, ai_request: ai_request) + .with(current_user: user, container: expected_container, resource: resource, ai_request: ai_request, + extra_resource: extra_resource) .and_return(context) subject diff --git a/ee/spec/workers/llm/completion_worker_spec.rb b/ee/spec/workers/llm/completion_worker_spec.rb index f88d3551955884..c198eb3c06721a 100644 --- a/ee/spec/workers/llm/completion_worker_spec.rb +++ b/ee/spec/workers/llm/completion_worker_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' RSpec.describe Llm::CompletionWorker, feature_category: :team_planning do + include FakeBlobHelpers include AfterNextHelpers it_behaves_like 'worker with data consistency', described_class, data_consistency: :delayed @@ -19,13 +20,22 @@ let(:options) { { 'key' => 'value' } } let(:ai_template) { { method: :completions, prompt: 'something', options: { temperature: 0.7 } } } let(:ai_action_name) { :summarize_comments } - let(:params) { options.merge(request_id: 'uuid', internal_request: true, cache_response: false) } + let(:referer_url) { nil } + let(:extra_resource) { {} } + + let(:params) do + options.merge(request_id: 'uuid', internal_request: true, cache_response: false, referer_url: referer_url) + end subject { described_class.new.perform(user_id, resource_id, resource_type, ai_action_name, params) } shared_examples 'performs successfully' do it 'calls Gitlab::Llm::CompletionsFactory' do completion = instance_double(Gitlab::Llm::Completions::SummarizeAllOpenNotes) + extra_resource_finder = instance_double(::Llm::ExtraResourceFinder) + + expect(::Llm::ExtraResourceFinder).to receive(:new).with(user, referer_url).and_return(extra_resource_finder) + expect(extra_resource_finder).to receive(:execute).and_return(extra_resource) expect(Gitlab::Llm::CompletionsFactory) .to receive(:completion) @@ -34,7 +44,7 @@ expect(completion) .to receive(:execute) - .with(user, resource, options.symbolize_keys) + .with(user, resource, options.symbolize_keys.merge(extra_resource: extra_resource)) subject end @@ -45,6 +55,13 @@ group.add_reporter(user) end + context 'when extra resource is found' do + let(:referer_url) { "foobar" } + let(:extra_resource) { { blob: fake_blob(path: 'file.md') } } + + it_behaves_like 'performs successfully' + end + context 'for an issue' do let_it_be(:resource) { create(:issue, project: project) } diff --git a/spec/support/finder_collection_allowlist.yml b/spec/support/finder_collection_allowlist.yml index 5de8e8cdca2f49..e7dd9cea9229f2 100644 --- a/spec/support/finder_collection_allowlist.yml +++ b/spec/support/finder_collection_allowlist.yml @@ -7,6 +7,7 @@ - Namespaces::FreeUserCap::UsersFinder # Reason: There is no need to have anything else besides the count - Groups::EnvironmentScopesFinder # Reason: There is no need to have anything else besides the simple strucutre with the scope name - Security::RelatedPipelinesFinder # Reason: There is no need to have anything else besides the IDs of pipelines +- Llm::ExtraResourceFinder # Reason: The finder does not deal with DB-backend resource for now. # Temporary excludes (aka TODOs) # For example: -- GitLab From 314a0b52c6c264a9e0f98d1d3db59f5503464a8c Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Wed, 16 Aug 2023 13:08:13 +0900 Subject: [PATCH 2/9] Remove verbose checks and add more specs --- ee/app/finders/llm/extra_resource_finder.rb | 2 +- .../finders/llm/extra_resource_finder_spec.rb | 18 +++++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/ee/app/finders/llm/extra_resource_finder.rb b/ee/app/finders/llm/extra_resource_finder.rb index 2730b384b66c2e..c27a3b484205e9 100644 --- a/ee/app/finders/llm/extra_resource_finder.rb +++ b/ee/app/finders/llm/extra_resource_finder.rb @@ -31,7 +31,7 @@ def find_blob_resource return unless project_fullpath && resource_path @project = find_project(project_fullpath) - return unless @project && @current_user.can?(:read_code, @project) && @project.repository + return unless @project ref, path = extract_blob_ref_and_path(resource_path) return unless ref && path diff --git a/ee/spec/finders/llm/extra_resource_finder_spec.rb b/ee/spec/finders/llm/extra_resource_finder_spec.rb index 6b43bec9a177aa..58f68012249868 100644 --- a/ee/spec/finders/llm/extra_resource_finder_spec.rb +++ b/ee/spec/finders/llm/extra_resource_finder_spec.rb @@ -4,7 +4,9 @@ RSpec.describe Llm::ExtraResourceFinder, feature_category: :ai_abstraction_layer do let_it_be(:project) { create(:project, :repository) } + let_it_be(:other_project) { create(:project, :repository) } let_it_be(:developer) { create(:user).tap { |u| project.add_developer(u) } } + let_it_be(:other_developer) { create(:user).tap { |u| other_project.add_developer(u) } } let_it_be(:guest) { create(:user).tap { |u| project.add_guest(u) } } let_it_be(:issue) { create(:issue, project: project) } @@ -65,10 +67,20 @@ end context "when user is not authorized to read code for the blob's project" do - let(:current_user) { guest } + context 'when user is a guest' do + let(:current_user) { guest } - it 'returns an empty hash' do - expect(execute).to be_empty + it 'returns an empty hash' do + expect(execute).to be_empty + end + end + + context 'when user does not have any access' do + let(:current_user) { other_developer } + + it 'returns an empty hash' do + expect(execute).to be_empty + end end end end -- GitLab From 5274275536aa179922fff70c596b1bc07c7705e8 Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Wed, 16 Aug 2023 13:31:22 +0900 Subject: [PATCH 3/9] Use referer_url for chat only and fix specs --- ee/app/graphql/mutations/ai/action.rb | 2 +- ee/spec/graphql/mutations/ai/action_spec.rb | 15 +++++++-------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/ee/app/graphql/mutations/ai/action.rb b/ee/app/graphql/mutations/ai/action.rb index 25663e9b6419ec..9d4853ed724221 100644 --- a/ee/app/graphql/mutations/ai/action.rb +++ b/ee/app/graphql/mutations/ai/action.rb @@ -36,7 +36,7 @@ def resolve(**attributes) resource_id, method, options = extract_method_params!(attributes) resource = resource_id&.then { |id| authorized_find!(id: id) } - options[:referer_url] = context[:request].headers["Referer"] + options[:referer_url] = context[:request].headers["Referer"] if method == :chat response = Llm::ExecuteMethodService.new(current_user, resource, method, options).execute diff --git a/ee/spec/graphql/mutations/ai/action_spec.rb b/ee/spec/graphql/mutations/ai/action_spec.rb index 1c969bc76f619e..032f82bceb8222 100644 --- a/ee/spec/graphql/mutations/ai/action_spec.rb +++ b/ee/spec/graphql/mutations/ai/action_spec.rb @@ -9,7 +9,7 @@ let(:request_id) { 'uuid' } let(:request) { instance_double(ActionDispatch::Request, headers: { "Referer" => "foobar" }) } let(:context) { { current_user: user, request: request } } - let(:base_options) { { referer_url: "foobar" } } + let(:expected_options) { {} } subject(:mutation) { described_class.new(object: nil, context: context, field: nil) } @@ -118,7 +118,7 @@ user, resource, expected_method, - base_options.merge(expected_options) + expected_options ) do |svc| expect(svc) .to receive(:execute) @@ -130,16 +130,15 @@ end context 'when resource is null' do - let(:input) { { chat: { resource_id: nil } } } - let(:expected_options) { {} } + let(:resource_id) { nil } it 'calls Llm::ExecuteMethodService' do expect_next_instance_of( Llm::ExecuteMethodService, user, nil, - :chat, - base_options.merge(expected_options) + expected_method, + expected_options ) do |svc| expect(svc) .to receive(:execute) @@ -158,7 +157,7 @@ user, resource, expected_method, - base_options.merge(expected_options) + expected_options ) do |svc| expect(svc) .to receive(:execute) @@ -180,7 +179,7 @@ it_behaves_like 'an AI action' do let(:expected_method) { :chat } - let(:expected_options) { {} } + let(:expected_options) { { referer_url: "foobar" } } end end -- GitLab From 117cc139db356b6d4afae9301f1e1854a9de2430 Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Wed, 16 Aug 2023 13:40:44 +0900 Subject: [PATCH 4/9] Use delete over extract! --- ee/lib/gitlab/llm/completions/chat.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ee/lib/gitlab/llm/completions/chat.rb b/ee/lib/gitlab/llm/completions/chat.rb index 01dcda699980d4..bb25a23f575d14 100644 --- a/ee/lib/gitlab/llm/completions/chat.rb +++ b/ee/lib/gitlab/llm/completions/chat.rb @@ -15,13 +15,12 @@ def execute(user, resource, options) # one we like. At the moment Anthropic is default and some features may not be supported # by other providers. ai_request = ::Gitlab::Llm::Chain::Requests::Anthropic.new(user) - extra_resource_options = options.extract!(:extra_resource) context = ::Gitlab::Llm::Chain::GitlabContext.new( current_user: user, container: resource.try(:resource_parent)&.root_ancestor, resource: resource, ai_request: ai_request, - extra_resource: extra_resource_options[:extra_resource] || {} + extra_resource: options.delete(:extra_resource) || {} ) response = Gitlab::Llm::Chain::Agents::ZeroShot::Executor.new( -- GitLab From 7656d4bce95e965c53fc85c5f30f8d4c22e3fba6 Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Wed, 16 Aug 2023 14:20:41 +0900 Subject: [PATCH 5/9] Fix chat_spec.rb --- .../requests/api/graphql/mutations/projects/chat_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ee/spec/requests/api/graphql/mutations/projects/chat_spec.rb b/ee/spec/requests/api/graphql/mutations/projects/chat_spec.rb index 36f5d603820eb4..1cd83b6ca64dc1 100644 --- a/ee/spec/requests/api/graphql/mutations/projects/chat_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/projects/chat_spec.rb @@ -34,7 +34,7 @@ expect(Llm::CompletionWorker).to receive(:perform_async).with( current_user.id, nil, nil, :chat, { content: "summarize", markup_format: :raw, request_id: an_instance_of(String), - cache_response: true, emit_user_messages: true + cache_response: true, emit_user_messages: true, referer_url: nil } ) @@ -47,7 +47,7 @@ expect(Llm::CompletionWorker).to receive(:perform_async).with( current_user.id, resource.id, "Issue", :chat, { content: "summarize", markup_format: :raw, request_id: an_instance_of(String), - cache_response: true, emit_user_messages: true + cache_response: true, emit_user_messages: true, referer_url: nil } ) @@ -64,7 +64,7 @@ expect(Llm::CompletionWorker).to receive(:perform_async).with( current_user.id, current_user.id, "User", :chat, { content: "summarize", markup_format: :raw, request_id: an_instance_of(String), - cache_response: true, emit_user_messages: true + cache_response: true, emit_user_messages: true, referer_url: nil } ) -- GitLab From a0e70f38b5ed09227020f744f6b0dbca4791a8e3 Mon Sep 17 00:00:00 2001 From: Alexandru Croitor Date: Wed, 16 Aug 2023 20:22:51 +0300 Subject: [PATCH 6/9] Adjust IssueIdentifier response In cases when an issue is being successfully identified, adjust the prompt to make AI more willing to load the issue data through ResourceReader. That fixes some edge cases discussed in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/128342#note_1498848592 --- .../development/push_ai_to_load_identified_issue_json.yml | 8 ++++++++ .../gitlab/llm/chain/tools/issue_identifier/executor.rb | 5 +++++ 2 files changed, 13 insertions(+) create mode 100644 config/feature_flags/development/push_ai_to_load_identified_issue_json.yml diff --git a/config/feature_flags/development/push_ai_to_load_identified_issue_json.yml b/config/feature_flags/development/push_ai_to_load_identified_issue_json.yml new file mode 100644 index 00000000000000..bc11abf48a5268 --- /dev/null +++ b/config/feature_flags/development/push_ai_to_load_identified_issue_json.yml @@ -0,0 +1,8 @@ +--- +name: push_ai_to_load_identified_issue_json +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/128342 +rollout_issue_url: +milestone: '16.3' +type: development +group: group::ai framework +default_enabled: true diff --git a/ee/lib/gitlab/llm/chain/tools/issue_identifier/executor.rb b/ee/lib/gitlab/llm/chain/tools/issue_identifier/executor.rb index 8eccbc780fcd2f..7a6429dd9cf4fc 100644 --- a/ee/lib/gitlab/llm/chain/tools/issue_identifier/executor.rb +++ b/ee/lib/gitlab/llm/chain/tools/issue_identifier/executor.rb @@ -126,6 +126,7 @@ def perform # now the issue in context is being referenced in user input. context.resource = issue content = "I identified the issue #{json[:ResourceIdentifier]}." + content += " For more information use ResourceReader." if load_json? logger.debug(message: "Answer", class: self.class.to_s, content: content) return Answer.new(status: :ok, context: context, content: content, tool: nil) @@ -147,6 +148,10 @@ def perform private + def load_json? + Feature.enabled?(:push_ai_to_load_identified_issue_json) + end + def authorize Utils::Authorizer.context_authorized?(context: context) end -- GitLab From 0573bb1d771f8d6144bb1d181a377031ac6e611a Mon Sep 17 00:00:00 2001 From: Alexandru Croitor Date: Wed, 16 Aug 2023 21:54:00 +0300 Subject: [PATCH 7/9] Spec fix --- .../tools/issue_identifier/executor_spec.rb | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/ee/spec/lib/gitlab/llm/chain/tools/issue_identifier/executor_spec.rb b/ee/spec/lib/gitlab/llm/chain/tools/issue_identifier/executor_spec.rb index b099045e92ac6d..2943aae7e57284 100644 --- a/ee/spec/lib/gitlab/llm/chain/tools/issue_identifier/executor_spec.rb +++ b/ee/spec/lib/gitlab/llm/chain/tools/issue_identifier/executor_spec.rb @@ -3,13 +3,18 @@ require 'spec_helper' RSpec.describe Gitlab::Llm::Chain::Tools::IssueIdentifier::Executor, feature_category: :shared do - RSpec.shared_examples 'success response' do + RSpec.shared_examples 'success response' do |ff_off| it 'returns success response' do ai_request = double allow(ai_request).to receive(:request).and_return(ai_response) allow(context).to receive(:ai_request).and_return(ai_request) - response = "I identified the issue #{identifier}." + response = if ff_off + "I identified the issue #{identifier}." + else + "I identified the issue #{identifier}. For more information use ResourceReader." + end + expect(tool.execute.content).to eq(response) end end @@ -139,6 +144,21 @@ it_behaves_like 'success response' end + context 'when push_ai_to_load_identified_issue_json FF is disabled' do + before do + stub_feature_flags(push_ai_to_load_identified_issue_json: false) + end + + context 'when is issue identified with reference' do + let(:identifier) { issue2.to_reference(full: true) } + let(:ai_response) do + "reference\", \"ResourceIdentifier\": \"#{identifier}\"}" + end + + it_behaves_like 'success response', true + end + end + # Skipped pending https://gitlab.com/gitlab-org/gitlab/-/issues/413509 xcontext 'when is issue identified with url' do let(:identifier) { Gitlab::Saas.com_url + Gitlab::Routing.url_helpers.project_issue_path(project, issue2) } -- GitLab From 6a9d12f97d34eedf1137abb8650d8d19613b0e82 Mon Sep 17 00:00:00 2001 From: Alexandru Croitor Date: Thu, 17 Aug 2023 10:35:16 +0300 Subject: [PATCH 8/9] Change FF default enabled value --- .../development/push_ai_to_load_identified_issue_json.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/feature_flags/development/push_ai_to_load_identified_issue_json.yml b/config/feature_flags/development/push_ai_to_load_identified_issue_json.yml index bc11abf48a5268..265b561c7607a5 100644 --- a/config/feature_flags/development/push_ai_to_load_identified_issue_json.yml +++ b/config/feature_flags/development/push_ai_to_load_identified_issue_json.yml @@ -5,4 +5,4 @@ rollout_issue_url: milestone: '16.3' type: development group: group::ai framework -default_enabled: true +default_enabled: false -- GitLab From 596d0d3532e841940d7b27a823d6e1cd50b266d8 Mon Sep 17 00:00:00 2001 From: Alexandru Croitor Date: Thu, 17 Aug 2023 18:15:22 +0300 Subject: [PATCH 9/9] Add minot spec fix --- .../lib/gitlab/llm/chain/agents/zero_shot/executor_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ee/spec/lib/gitlab/llm/chain/agents/zero_shot/executor_spec.rb b/ee/spec/lib/gitlab/llm/chain/agents/zero_shot/executor_spec.rb index 0f7dd8f39a8920..c9f127bb59bb79 100644 --- a/ee/spec/lib/gitlab/llm/chain/agents/zero_shot/executor_spec.rb +++ b/ee/spec/lib/gitlab/llm/chain/agents/zero_shot/executor_spec.rb @@ -160,10 +160,10 @@ end context 'when resource is a blob' do - let(:project) { create(:project, :repository) } - let(:blob) { project.repository.blob_at("master", "README") } - let(:extra_resource) { { blob: blob } } + let_it_be(:project) { create(:project, :repository) } + let_it_be(:blob) { project.repository.blob_at("master", "README") } + let(:extra_resource) { { blob: blob } } let(:injected_prompt) do "The current code file that user sees is #{blob.path} and has the following content\n#{blob.data}" end -- GitLab