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:

  1. Ai::DuoWorkflows::CodeReview::ProcessCommentsService#execute tracked it when draft_notes was empty.
  2. Ai::DuoWorkflows::CodeReview::CreateCommentsService#execute calls ProcessCommentsService, then tracked it again when the returned draft_notes was 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

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.

proof_631748

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

  1. Make sure ClickHouse is enabled in your GDK (clickhouse.enabled: true) and Gitaly is running.
  2. Save the script below as proof_631748.rb and run bin/rails runner proof_631748.rb, first on master, 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)}"
  1. Expected: tracked 2 time(s) / 2 rows on master, and tracked 1 time(s) / 1 row 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.

🤖 Generated with Claude Code

Edited by Jatin Parihar

Merge request reports

Loading
Loading