Update DCR prompt to include custom instructions
What does this MR do and why?
- This MR updates the DCR prompt to include custom instructions for the diffs
- Custom instructions are stored in
.gitlab/duo/mr-review-instructions.yaml
- Changes are gated behind
duo_code_review_custom_instructions
feature flag - AIGW counterpart: feat: Update DCR prompt to include custom instr... (gitlab-org/modelops/applied-ml/code-suggestions/ai-assist!2624 - merged)
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 existingauthenticate!
at class level andfind_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
- Received custom instruction comments about missing authentication:
-
✅ 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 patternapp/controllers/api/**/*.rb
correctly excluded the service file atapp/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 selectGitLabDuo
from the reviewers dropdown.
Validate:
-
Make sure GitLabDuo
works as expected withoutduo_code_review_custom_instructions
being enabled. -
Enable duo_code_review_custom_instructions
and make sureGitLabDuo
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>
.gitlab/duo/mr-review-instructions.yaml
file
Sample ---
# 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