Track find_no_issues Duo Code Review event once per review
What does this MR do and why?
When an agentic Duo Code Review finds nothing to comment on, find_no_issues_duo_code_review_after_review was tracked twice for the same review:
Ai::DuoWorkflows::CodeReview::ProcessCommentsService#executetracked it whendraft_noteswas empty.Ai::DuoWorkflows::CodeReview::CreateCommentsService#executecallsProcessCommentsService, then tracked it again when the returneddraft_noteswas empty.
The two emissions land in ClickHouse ai_usage_events at different microseconds, so the ReplacingMergeTree does not collapse them. This inflates "Without comments" and "Reviews published" on the DAP Impact dashboard (dap_impact.yaml), and any other consumer of the event such as Analytics::AiAnalytics::UsageEventCountService.
This MR keeps the tracking in CreateCommentsService, which owns the review outcome (progress note update and publish decision), and removes it from ProcessCommentsService. CreateCommentsService is its only caller.
The duplicate went unnoticed because create_comments_service_spec.rb stubs ProcessCommentsService with an instance_double, so each spec only ever saw one emission. A new spec runs the real ProcessCommentsService through CreateCommentsService and asserts the event fires exactly once. It fails on master with expected: 1 time … received: 2 times.
Historical events are not corrected.
References
- Closes https://gitlab.com/gitlab-org/gitlab/-/issues/631748
- Related: https://gitlab.com/gitlab-org/gitlab/-/issues/631549 (publish event overcounting; also needed for With + Without = Published)
Screenshots or screen recordings
Local GDK run of the real CreateCommentsService → ProcessCommentsService flow for a review with nothing to comment on. The ClickHouse write buffer is flushed and rows are counted with FINAL.
Before (master) |
After (this MR) | |
|---|---|---|
| Times the event is tracked | 2 (process_comments_service.rb:48, create_comments_service.rb:47) |
1 (create_comments_service.rb:47) |
New rows in ClickHouse ai_usage_events (FINAL) |
2 | 1 |
How to set up and validate locally
- Make sure ClickHouse is enabled in your GDK (
clickhouse.enabled: true) and Gitaly is running. - Save the script below as
proof_631748.rband runbin/rails runner proof_631748.rb, first onmaster, then on this branch.
proof_631748.rb
EVENT = 'find_no_issues_duo_code_review_after_review'
TABLE = Ai::UsageEvent.clickhouse_table_name
def flush_clickhouse_buffer
ClickHouse::DumpWriteBufferWorker.new.perform(TABLE)
end
def clickhouse_count(user, since)
ClickHouse::Client.select(<<~SQL, :main).first['c']
SELECT count() AS c FROM #{TABLE} FINAL
WHERE event = #{Ai::UsageEvent.events[EVENT]}
AND user_id = #{user.id}
AND timestamp >= toDateTime64('#{since.utc.strftime('%Y-%m-%d %H:%M:%S.%6N')}', 6, 'UTC')
SQL
end
tracked = []
Gitlab::InternalEvents.singleton_class.prepend(Module.new do
define_method(:track_event) do |name, *args, **kwargs|
if name == EVENT
loc = caller_locations.find { |l| l.path.include?('code_review/') && l.path.end_with?('_service.rb') }
tracked << "#{File.basename(loc.path)}:#{loc.lineno}" if loc
end
super(name, *args, **kwargs)
end
end)
user = User.find_by!(username: 'root')
merge_request = MergeRequest.first!
flush_clickhouse_buffer
started_at = Time.current
sleep 0.01
result = Ai::DuoWorkflows::CodeReview::CreateCommentsService.new(
user: user,
merge_request: merge_request,
review_output: '', # the review found nothing to comment on
progress_note: nil
).execute
flush_clickhouse_buffer
puts "Service result: #{result.status}"
puts "#{EVENT} tracked #{tracked.size} time(s):"
tracked.each { |loc| puts " - from #{loc}" }
puts "New rows in ClickHouse #{TABLE} (FINAL): #{clickhouse_count(user, started_at)}"- Expected:
tracked 2 time(s)/2rows onmaster, andtracked 1 time(s)/1row on this branch.
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.
