Skip to content

Update DCR prompt to include custom instructions

What does this MR do and why?

Testing (LangSmith)

Test Case 1 (simple; one-file test)

  • Description: API controller endpoint without authentication - tests if custom instructions for API security are properly applied
  • LangSmith link: https://smith.langchain.com/public/932bdb6c-f6ed-47b9-988e-b75ab7d426ab/r
  • Results:
    • Custom instruction successfully applied - LLM identified missing authentication for API endpoint
    • Appropriate priority assignment - authentication/authorization issues marked as priority 3, rate limiting as priority 2
    • Context-aware analysis - LLM used full file content to notice existing authenticate! at class level and find_user helper
    • Both standard and custom review criteria applied - standard security checks (authorization, error handling) plus custom instructions (authentication, rate limiting)

Test Case 2 (multi-file test)

  • Description: API controller and service object - tests if custom instructions are only applied to files matching the glob pattern and not to others
  • LangSmith link: https://smith.langchain.com/public/87196265-be3e-4794-b0bb-a7a312bfb113/r
  • Results:
    • Selective application worked perfectly - Custom instructions were ONLY applied to the API controller, NOT to the service object
    • API Controller (app/controllers/api/v4/search_controller.rb):
      • Received custom instruction comments about missing authentication: This API endpoint is missing authentication and authorization checks. According to the custom review instructions, all API endpoints should have proper authentication using the 'authenticate!' method.
      • Received custom instruction comments about rate limiting
      • Both marked as priority 3 as per custom instructions
    • Service Object (app/services/user_search_service.rb):
      • Did NOT receive any custom instruction comments
      • Only received standard code review comments (SQL injection vulnerability, pagination)
      • SQL injection correctly identified as priority 3 (standard security issue)
    • Glob pattern matching confirmed working - The pattern app/controllers/api/**/*.rb correctly excluded the service file at app/services/user_search_service.rb
    • Standard review still applied to all files - Both files received appropriate standard security and performance feedback

How to set up and validate locally

Prerequisites:

  • Make sure the AI features are enabled locally using this guide
  • Enable duo_code_review_custom_instructions feature flag for the user
  • Create a new file, .gitlab/duo/mr-review-instructions.yaml, with some custom instructions (for reference, take a look at the description of #545339 (closed) for the format)
  • Open a MR with some files where custom instructions are only applicable to few files in the diff
  • Assign GitLabDuo as a reviewer either by commenting /assign_reviewer @GitLabDuo or select GitLabDuo from the reviewers dropdown.

Validate:

  • Make sure GitLabDuo works as expected without duo_code_review_custom_instructions being enabled.
  • Enable duo_code_review_custom_instructions and make sure GitLabDuo calls out issues as per the custom instructions
  • Make sure there are no unwanted comments from GitLabDuo.

Local Testing

  • After enabling the feature flag, I logged the inputs we're sending from monolith side:
{"severity":"INFO","time":"2025-06-03T17:25:20.678Z","correlation_id":"01JWVD5KP7P4P64KC5546KS05Q","meta.caller_id":"Llm::CompletionWorker","meta.feature_category":"ai_abstraction_layer","meta.organization_id":1,"meta.remote_ip":"127.0.0.1","meta.http_router_rule_action":"classify","meta.http_router_rule_type":"SESSION_PREFIX","meta.user":"root","meta.user_id":1,"meta.client_id":"user/1","meta.root_caller_id":"GraphqlController#execute","message":"KINSHUK TESTING: prompt_inputs = {:mr_title=\u003e\"Add user controller and service\", :mr_description=\u003e\"- This MR adds users controller and service\", :diff_lines=\u003e\"\u003cfile_diff filename=\\\"app/controllers/api/v4/services/user_search_service.rb\\\"\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"1\\\"\u003eclass UserSearchService\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"2\\\"\u003e  def initialize(query)\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"3\\\"\u003e    @query = query\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"4\\\"\u003e  end\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"5\\\"\u003e  \u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"6\\\"\u003e  def execute\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"7\\\"\u003e    # Note: Potentially vulnerable to SQL injection\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"8\\\"\u003e    User.where(\\\"name LIKE '%\\#{@query}%' OR email LIKE '%\\#{@query}%'\\\")\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"9\\\"\u003e  end\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"10\\\"\u003e  \u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"11\\\"\u003e  def find_active_users\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"12\\\"\u003e    execute.where(active: true)\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"13\\\"\u003e  end\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"14\\\"\u003e  \u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"15\\\"\u003e  def find_admins\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"16\\\"\u003e    execute.where(admin: true)\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"17\\\"\u003e  end\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"18\\\"\u003eend\u003c/line\u003e\\n\u003c/file_diff\u003e\\n\\n\u003cfile_diff filename=\\\"app/controllers/api/v4/users_controller.rb\\\"\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"1\\\"\u003emodule API\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"2\\\"\u003e  module V4\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"3\\\"\u003e    class UsersController \u003c ApplicationController\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"4\\\"\u003e      def index\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"5\\\"\u003e        @users = User.all\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"6\\\"\u003e        render json: @users\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"7\\\"\u003e      end\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"8\\\"\u003e      \u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"9\\\"\u003e      def show\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"10\\\"\u003e        @user = User.find(params[:id])\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"11\\\"\u003e        render json: @user\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"12\\\"\u003e      end\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"13\\\"\u003e      \u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"14\\\"\u003e      def create\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"15\\\"\u003e        @user = User.new(user_params)\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"16\\\"\u003e        \u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"17\\\"\u003e        if @user.save\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"18\\\"\u003e          render json: @user, status: :created\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"19\\\"\u003e        else\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"20\\\"\u003e          render json: @user.errors, status: :unprocessable_entity\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"21\\\"\u003e        end\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"22\\\"\u003e      end\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"23\\\"\u003e      \u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"24\\\"\u003e      private\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"25\\\"\u003e      \u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"26\\\"\u003e      def user_params\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"27\\\"\u003e        params.require(:user).permit(:name, :email, :password)\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"28\\\"\u003e      end\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"29\\\"\u003e    end\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"30\\\"\u003e  end\u003c/line\u003e\\n\u003cline type=\\\"added\\\" old_line=\\\"\\\" new_line=\\\"31\\\"\u003eend\u003c/line\u003e\\n\u003c/file_diff\u003e\", :full_file_intro=\u003e\"\", :full_content_section=\u003e\"\", :custom_instructions_section=\u003e\"Custom Review Instructions:\\n\u003ccustom_instructions\u003e\\nYou must also apply the following custom review instructions. Each instruction specifies which files it applies to:\\n\\nFor files matching \\\"app/controllers/api/**/*.rb\\\" (API Controllers):\\n1. Ensure all API endpoints have proper authentication checks using 'authenticate!' method\\n2. Verify appropriate authorization is in place\\n3. Check that rate limiting is implemented for public endpoints\\n\\nIMPORTANT: Only apply each custom instruction to files that match its specified pattern. If a file doesn't match any custom instruction pattern, only apply the standard review criteria.\\n\u003c/custom_instructions\u003e\\n\"}"}
  • Custom instructions were passed successfully (and only relevant instructions were passed, ignoring the patters that don't match any of the diff files)
Custom Review Instructions:
<custom_instructions>
You must also apply the following custom review instructions. Each instruction specifies which files it applies to:

For files matching "app/controllers/api/**/*.rb" (API Controllers):
1. Ensure all API endpoints have proper authentication checks using 'authenticate!' method
2. Verify appropriate authorization is in place
3. Check that rate limiting is implemented for public endpoints

IMPORTANT: Only apply each custom instruction to files that match its specified pattern. If a file doesn't match any custom instruction pattern, only apply the standard review criteria.
</custom_instructions>

Sample .gitlab/duo/mr-review-instructions.yaml file

---
# Custom instructions for Duo Code Review

# This file defines custom review criteria that will be applied to specific files
# during merge request reviews. Instructions are grouped by name and can target
# multiple file patterns using glob syntax.

# Format:
# - name: A descriptive name for this group of instructions
# - fileFilters: List of glob patterns to match files
# - instructions: The review criteria to apply (can be multi-line)

# The instructions will only be applied to files matching the specified patterns.
# All files still receive the standard code review regardless of custom instructions.

instructions:
  - name: API Controllers
    fileFilters:
      - app/controllers/api/**/*.rb
    instructions: |
      1. Ensure all API endpoints have proper authentication checks using 'authenticate!' method
      2. Verify appropriate authorization is in place
      3. Check that rate limiting is implemented for public endpoints

  - name: Ruby Models
    fileFilters:
      - app/models/**/*.rb
    instructions: |
      1. Check for N+1 queries and ensure proper eager loading with includes(), preload(), or eager_load()
      2. Verify that database indexes exist for foreign keys and frequently queried columns
      3. Ensure validations are present for required fields

  - name: RSpec Tests
    fileFilters:
      - spec/**/*_spec.rb
    instructions: |
      1. Ensure tests cover edge cases, error scenarios, and not just happy paths
      2. Verify that shared examples are used where appropriate to reduce duplication
      3. Check that database state is properly cleaned between tests

  - name: JavaScript/TypeScript
    fileFilters:
      - "**/*.js"
      - "**/*.ts"
      - "**/*.vue"
    instructions: |
      1. Prefer TypeScript over JavaScript for new code
      2. Ensure proper error handling with try/catch blocks
      3. Check for potential XSS vulnerabilities in user input handling

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #545136 (closed), #545339 (closed)

Edited by Kinshuk Singh

Merge request reports

Loading