From af3881b7bd30a8c415098ba468b3fb5b405deb7e Mon Sep 17 00:00:00 2001
From: Lasmar Khalifa <lasmar.khalifa@shopify.com>
Date: Fri, 10 Jul 2026 09:19:42 -0400
Subject: [PATCH] Add doc-comment audit workflow

---
 .../prompts/verify_comment.md.erb             |  51 ++
 .../maintenance/comment_audit/workflow.rb     | 439 ++++++++++++++++++
 2 files changed, 490 insertions(+)
 create mode 100644 internal/workflows/maintenance/comment_audit/prompts/verify_comment.md.erb
 create mode 100644 internal/workflows/maintenance/comment_audit/workflow.rb

diff --git a/internal/workflows/maintenance/comment_audit/prompts/verify_comment.md.erb b/internal/workflows/maintenance/comment_audit/prompts/verify_comment.md.erb
new file mode 100644
index 00000000..87ad3e6e
--- /dev/null
+++ b/internal/workflows/maintenance/comment_audit/prompts/verify_comment.md.erb
@@ -0,0 +1,51 @@
+You are auditing a SINGLE comment in the Roast codebase for factual correctness
+against the real implementation. This is the only comment you are judging right now.
+
+## The comment under review
+
+- File: `<%= unit[:file] %>`
+<% if unit[:kind] == "inline" %>- An inline comment that documents no definition (it describes nearby code, not a method/class/constant)
+<% else %>- Documents: `<%= unit[:symbol] %>` (<%= unit[:kind] %>)
+<% end %>- Source lines <%= unit[:start_line] %>-<%= unit[:end_line] %>
+<% if unit[:signature] %>- Declared RBS signature, type-checked and authoritative (treat as ground truth; correct the prose to match it, never the reverse): `<%= unit[:signature] %>`
+<% end %>
+Current comment text, verbatim (every line is a comment, shown with its original indentation):
+
+<%= unit[:comment_text] %>
+
+## What to do
+
+<% if unit[:kind] == "inline" %>1. Open `<%= unit[:file] %>` and read the code immediately surrounding lines
+   <%= unit[:start_line] %>-<%= unit[:end_line] %> (the statements this comment describes), plus any
+   closely related code needed to judge the claims. Use only the real code in this repository as ground truth.
+<% else %>1. Open `<%= unit[:file] %>` and read the actual implementation of `<%= unit[:symbol] %>`, plus any
+   closely related code (callers, the methods it delegates to, constants it names) needed to judge
+   the claims. Use only the real code in this repository as ground truth.
+<% end %>2. Check every factual claim the comment makes: described behavior, parameters and return values,
+   defaults, side effects, cross-references, and any named entity (method, class, flag, env var).
+3. A claim is an issue ONLY if it is actually WRONG or misleading about the current code. Do not
+   flag wording, style, formatting, or things the comment simply leaves out.
+4. If a declared RBS signature is shown above, it is authoritative. A comment that contradicts its
+   types, arity, or nilability is an issue: fix the prose, and never restate or edit the signature
+   in your suggested fix.
+
+## Output
+
+Return ONLY a JSON object, with no surrounding prose and no code fences. Exactly these keys:
+
+{
+  "factual": true or false,
+  "issues": ["the specific wrong claim and why it is wrong", ...],
+  "confidence": 0.0 to 1.0,
+  "suggested_fix": "the full corrected comment block",
+  "evidence": "file:line references you relied on"
+}
+
+Rules for the fields:
+- If every claim is correct: "factual": true, "issues": [], and "suggested_fix" equal to the
+  original comment text unchanged.
+- If there is at least one issue: "factual": false, list each issue, and provide "suggested_fix"
+  as the COMPLETE corrected comment block. Don't be overly verbose. Each line must start with `# ` (a plain comment line),
+  with NO leading indentation, and do NOT include any `#:` RBS signature line. Preserve the parts of
+  the comment that were already correct; change only what is wrong.
+- "confidence" is how sure you are of this verdict and, when fixing, of the fix's correctness.
diff --git a/internal/workflows/maintenance/comment_audit/workflow.rb b/internal/workflows/maintenance/comment_audit/workflow.rb
new file mode 100644
index 00000000..e9b77eda
--- /dev/null
+++ b/internal/workflows/maintenance/comment_audit/workflow.rb
@@ -0,0 +1,439 @@
+# typed: false
+# frozen_string_literal: true
+
+#: self as Roast::Workflow
+
+# Guards against comment drift by treating a single contiguous `#` prose block as the atomic unit of
+# audit: both a definition's doc comment and an inline comment that documents no definition. It enumerates the
+# in-scope comment blocks (see Targets below), asks an agent to verify each one in isolation against
+# the real implementation, groups the stale ones into a single PR, and opens it only when the `apply`
+# flag is passed. Without `apply` it is read-only: it prints the findings and opens nothing.
+#
+# Parameters:
+#   apply   open PRs instead of only reporting (default: off / dry-run).
+#
+#   Targets: pass one or more files or directories to audit only those (a directory is expanded to
+#            its `.rb`/`.rbi` files); with no target it audits the full default scope, `lib/roast/**/*.rb`
+#            plus the user-facing rbi shims.
+#
+# Read-only (dry-run) runs from any branch and audits the working tree as-is. `apply` opens the PR
+# on top of the branch it is run from: from `main` the fix stacks cleanly on `main`; from a feature
+# branch it stacks on that branch. `apply` requires a clean working tree and the current branch to be
+# pushed and even with its origin counterpart, so the PR diff is only the doc fix.
+#
+# Examples:
+#   # dry-run the full default scope (prints findings, opens nothing, writes nothing):
+#   bin/roast execute internal/workflows/maintenance/comment_audit/workflow.rb
+#
+#   # dry-run only a directory or a single file (shell globs are expanded too):
+#   bin/roast execute internal/workflows/maintenance/comment_audit/workflow.rb lib/roast/cog
+#   bin/roast execute internal/workflows/maintenance/comment_audit/workflow.rb lib/roast/cog/output.rb
+#
+#   # open the PR of fixes on top of the current branch:
+#   bin/roast execute internal/workflows/maintenance/comment_audit/workflow.rb -- apply
+#
+#   # scope the audit and apply in one run:
+#   bin/roast execute internal/workflows/maintenance/comment_audit/workflow.rb lib/roast/cog -- apply
+
+config do
+  agent(:judge) do
+    provider :claude
+    model "sonnet"
+    quiet!
+  end
+
+  # Each unit verification is independent, we can run them in parallel.
+  map(:verify_units) do
+    parallel(8)
+  end
+end
+
+execute do
+  # When `apply` is set, verify up front that the PR can be opened on top of the current branch, before
+  # spending the heavy enumerate/verify work. Dry-run skips this and audits the working tree from any branch.
+  ruby(:preflight) do
+    skip! unless arg?(:apply)
+
+    current = %x(git rev-parse --abbrev-ref HEAD).strip
+    # Apply opens the PR against the current branch, so a detached HEAD has no branch name to target.
+    fail!("run `-- apply` from a branch, not detached HEAD") if current == "HEAD"
+    # Apply mutates the working tree (checkout/commit/checkout back), so it needs a clean working tree.
+    fail!("working tree is dirty; commit or stash before running with `-- apply`") unless %x(git status --porcelain).strip.empty?
+    # The PR stacks on the current branch, so its origin counterpart must exist and equal local HEAD,
+    # otherwise the diff would carry unpushed commits (or `gh pr create` would have no base to target).
+    system("git", "fetch", "origin", current, out: File::NULL, err: File::NULL)
+    local = %x(git rev-parse HEAD).strip
+    remote = IO.popen(["git", "rev-parse", "--verify", "--quiet", "origin/#{current}"], err: File::NULL, &:read).strip
+    fail!("branch `#{current}` must be pushed and even with `origin/#{current}` before `-- apply`") unless !remote.empty? && local == remote
+  end
+
+  # Phase 1: enumerate atomic comment units using prism: a doc-comment block above each definition,
+  # plus any inline comment block that lives inside a body.
+  ruby(:enumerate) do
+    require "prism"
+
+    # The scope: any files/directories passed as targets (a directory is expanded to its Ruby
+    # sources), or the full default scope: every .rb file under lib/roast plus our rbi shims.
+    scope_files =
+      if targets.any?
+        targets.flat_map { |t| File.directory?(t) ? Dir.glob(File.join(t, "**/*.{rb,rbi}")) : [t] }.uniq.sort
+      else
+        (Dir.glob("lib/roast/**/*.rb") + Dir.glob("sorbet/rbi/shims/**/*.rbi")).uniq.sort
+      end
+
+    # Comment patterns that are never human prose and must not be counted as part of a comment block.
+    magic = /\A#\s*(?:typed|frozen_string_literal|encoding|warn_indent|shareable_constant_value):/
+    sig = /\A#[:|]/ # RBS inline signature: the `#:` opener plus its `#|` continuation lines.
+    rubocop = /\A#\s*rubocop:/
+    directive = /\A#\s*@\w+:/ # RBS annotation directives, e.g. `# @requires_ancestor:`.
+
+    # Accumulates one audit unit per comment block (a definition's doc comment or an inline comment),
+    # across every file in scope.
+    units = []
+
+    scope_files.each do |file|
+      # Read and parse the file using prism.
+      source = File.read(file)
+      lines = source.lines.map(&:chomp)
+      result = Prism.parse(source)
+
+      # Mark every line Prism considers part of a comment.
+      comment_line = {}
+      result.comments.each do |c|
+        (c.location.start_line..c.location.end_line).each { |ln| comment_line[ln] = true }
+      end
+
+      # Helper lambdas to analyze the lines of the file. All line numbers are 1-based.
+      # Returns the line text, left-stripped.
+      body = ->(ln) { (lines[ln - 1] || "").lstrip }
+
+      # Checks if the line is a real comment line, excluding inline trailing comments.
+      doc = ->(ln) { comment_line[ln] && body.call(ln).start_with?("#") }
+
+      # Checks if the line is a bare `#`.
+      blank = ->(ln) { body.call(ln).match?(/\A#\s*\z/) }
+
+      # Checks if the line is a magic/directive line.
+      hard = ->(ln) { body.call(ln).match?(magic) || body.call(ln).match?(directive) }
+
+      # Record every definition's anchor line (its `def`/`class`/`module`/constant/attr line) with its
+      # fully-qualified symbol and kind. The walk calls `emit` for each definition it finds; the prose
+      # scan below reads this map to tell a doc comment (a block sitting directly above an anchor) from
+      # an inline comment.
+      anchors = {}
+      emit = ->(anchor, symbol, kind) { anchors[anchor] = { symbol: symbol, kind: kind } }
+
+      # Recursively walk the AST to record each definition's anchor via `emit`. Each branch handles a
+      # form a doc comment can sit above; method bodies are not descended into, so comments inside them
+      # are treated as inline by the scan below.
+      walk = nil
+      walk = lambda do |node, ns, singleton|
+        return if node.nil?
+
+        case node
+          # When Prism sees a program, it has a list of statements; recurse into them.
+        when Prism::ProgramNode
+          walk.call(node.statements, ns, singleton)
+          # When Prism sees a list of statements, recurse into each one.
+        when Prism::StatementsNode
+          node.body.each { |child| walk.call(child, ns, singleton) }
+          # When Prism sees a module/class body with a module-level `rescue` wrapped in a begin; recurse into it.
+        when Prism::BeginNode
+          walk.call(node.statements, ns, singleton)
+          # When Prism sees a module or class, emit it and recurse into its body.
+        when Prism::ModuleNode, Prism::ClassNode
+          name = node.constant_path.slice
+          full = ns.empty? ? name : "#{ns}::#{name}"
+          kind = node.is_a?(Prism::ModuleNode) ? "module" : "class"
+          emit.call(node.location.start_line, full, kind)
+          walk.call(node.body, full, false)
+          # When Prism sees a singleton class, recurse into its body.
+        when Prism::SingletonClassNode
+          walk.call(node.body, ns, true)
+        when Prism::DefNode
+          # When Prism sees a method definition, emit it.
+          sep = singleton || !node.receiver.nil? ? "." : "#"
+          emit.call(node.location.start_line, "#{ns}#{sep}#{node.name}", "method")
+          # When Prism sees a constant assignment, emit it.
+        when Prism::ConstantWriteNode
+          prefix = ns.empty? ? "" : "#{ns}::"
+          emit.call(node.location.start_line, "#{prefix}#{node.name}", "constant")
+          # When Prism sees an attribute assignment, emit it.
+        when Prism::CallNode
+          if node.receiver.nil? && [:attr_reader, :attr_writer, :attr_accessor].include?(node.name)
+            names = (node.arguments&.arguments || [])
+              .select { |a| a.is_a?(Prism::SymbolNode) }.map(&:unescaped)
+            emit.call(node.location.start_line, names.map { |n| "#{ns}##{n}" }.join(", "), "attr") unless names.empty?
+          end
+        end
+      end
+
+      # Start the walk at the file root; the recursion fills in `anchors`.
+      walk.call(result.value, "", false)
+
+      # A prose line is a real comment line carrying human text: not a magic/directive, RBS signature
+      # (`#:`/`#|`), or rubocop line. Bare `#` lines join a block (they are trimmed off each end below).
+      prose = ->(ln) { doc.call(ln) && !hard.call(ln) && !body.call(ln).match?(sig) && !body.call(ln).match?(rubocop) }
+
+      # Scan the file into maximal contiguous prose blocks; each block is one audit unit. It is a
+      # definition's doc comment when the code directly below it is a recorded anchor (skipping the
+      # sig/rubocop/blank zone that may sit between a comment and its def); otherwise it is inline.
+      ln = 1
+      while ln <= lines.length
+        unless prose.call(ln)
+          ln += 1
+          next
+        end
+        run_start = ln
+        ln += 1 while ln <= lines.length && prose.call(ln)
+        run_end = ln - 1
+        run_start += 1 while run_start <= run_end && blank.call(run_start)
+        run_end -= 1 while run_end >= run_start && blank.call(run_end)
+        next if run_start > run_end
+
+        # Look at the code line below the block; `anchor` is set when it begins a definition.
+        below = run_end + 1
+        below += 1 while below <= lines.length && doc.call(below) &&
+            (body.call(below).match?(sig) || body.call(below).match?(rubocop) || blank.call(below))
+        anchor = below <= lines.length ? anchors[below] : nil
+
+        if anchor
+          # Doc comment: pull the definition's RBS signature lines (if any) from the skipped zone.
+          sig_lines = ((run_end + 1)...below).select { |n| body.call(n).match?(sig) }.map { |n| body.call(n) }
+          units << {
+            file: file,
+            symbol: anchor[:symbol],
+            kind: anchor[:kind],
+            start_line: run_start,
+            end_line: run_end,
+            signature: sig_lines.empty? ? nil : sig_lines.join("\n"),
+            comment_text: lines[(run_start - 1)..(run_end - 1)].join("\n"),
+          }
+        else
+          # Inline comment: a prose block that documents no definition.
+          units << {
+            file: file,
+            symbol: run_start == run_end ? "inline @ L#{run_start}" : "inline @ L#{run_start}-#{run_end}",
+            kind: "inline",
+            start_line: run_start,
+            end_line: run_end,
+            signature: nil,
+            comment_text: lines[(run_start - 1)..(run_end - 1)].join("\n"),
+          }
+        end
+      end
+    end
+
+    # Return every enumerated unit.
+    units
+  end
+
+  # Phase 2: verify each unit in isolation (map + agent, runs in parallel).
+  # Goes over the enumerated units, running the :verify_one scope once per unit.
+  # The verdicts are read back in Phase 3.
+  map(:verify_units, run: :verify_one) do
+    ruby!(:enumerate).value
+  end
+
+  # Phase 3: classify each verdict into fixable, low-confidence, or factual.
+  ruby(:plan) do
+    # Collect one verdict per unit.
+    verdicts = collect(map!(:verify_units)) { |verdict, _unit, _idx| verdict }.compact
+
+    # A verdict is auto-fixed only if the agent is confident it is wrong and its fix is safe to apply.
+    # A fix is considered usable (safe to apply) if it is non-empty and every line starts with `#` (after stripping whitespace).
+    usable = lambda do |v|
+      fix = v[:suggested_fix].to_s.strip
+      !fix.empty? && fix.lines.all? { |l| l.strip.start_with?("#") }
+    end
+    failing = []
+    low_confidence = []
+    ok = 0
+    # Checks each verdict: if factual, count it as ok; if flagged, confident and usable, count it as failing; otherwise count it as low-confidence.
+    verdicts.each do |v|
+      if v[:factual]
+        ok += 1
+      elsif v[:confidence].to_f >= 0.8 && usable.call(v) # 0.8 = min confidence to auto-fix; below it is report-only
+        failing << v
+      else
+        low_confidence << v
+      end
+    end
+
+    counts = { checked: verdicts.size, ok: ok, failing: failing.size, low_confidence: low_confidence.size }
+
+    # Returns the fixable findings, the counts, and the low-confidence findings.
+    {
+      failing: failing,
+      counts: counts,
+      low_confidence: low_confidence,
+    }
+  end
+
+  # Phase 4: apply + open PRs (only runs when `-- apply` is passed).
+  ruby(:apply) do
+    skip! unless arg?(:apply)
+    require "shellwords"
+
+    plan = ruby!(:plan).value
+    failing = plan[:failing]
+    base = %x(git rev-parse HEAD).strip
+    base_short = base[0, 10]
+    original = %x(git rev-parse --abbrev-ref HEAD).strip
+    opened = nil
+
+    # Index the failing verdicts by file to shape the PR body (one section per file) and, below, batches
+    # each file's fixes for splicing.
+    by_file = failing.group_by { |v| v[:unit][:file] }
+    files = by_file.keys.sort
+    # Builds the PR body: one section per file, one bullet per stale comment (doc or inline), with its confidence and issues.
+    body = +"Automated comment audit on top of `#{original}` (base `#{base_short}`).\n\n"
+    files.each do |file|
+      body << "### `#{file}`\n\n"
+      by_file[file].each do |v|
+        body << "- **#{v[:unit][:symbol]}** (confidence #{v[:confidence]})\n"
+        Array(v[:issues]).each { |issue| body << "  - #{issue}\n" }
+        body << "  - evidence: #{v[:evidence]}\n" if v[:evidence]
+      end
+      body << "\n"
+    end
+    # Builds the PR plan. The PR is opened on top of the current branch (`original`); the doc-audit
+    # branch is named per source branch so re-running from it updates the same PR.
+    pr_plan =
+      if failing.empty?
+        []
+      else
+        [{
+          id: base_short,
+          base: original,
+          branch: "comment-audit/#{original.tr("/", "-")}",
+          title: "Fix #{failing.size} stale comment(s) across #{files.size} file(s)",
+          body: body,
+          files: files,
+          verdicts: failing,
+        }]
+      end
+
+    # Build the corrected file contents by splicing each fix in, bottom-to-top so earlier line
+    # numbers stay valid.
+    changed = {}
+    by_file.each do |file, verds|
+      lines = File.read(file).lines
+      verds.sort_by { |v| -v[:unit][:start_line] }.each do |v|
+        u = v[:unit]
+        # Re-indent the suggested fix to match the original block's indentation, then splice it in.
+        indent = (lines[u[:start_line] - 1] || "")[/\A\s*/]
+        replacement = v[:suggested_fix].to_s.split("\n").map { |l| "#{indent}#{l.sub(/\A\s+/, "")}\n" }
+        lines[(u[:start_line] - 1)...u[:end_line]] = replacement
+      end
+      changed[file] = lines.join
+    end
+
+    # Restore the original branch on exit, whether the loop completes or raises.
+    begin
+      pr_plan.each do |pr|
+        files = pr[:files]
+
+        # Deterministic branch name; check whether a PR for it already exists.
+        branch = pr[:branch]
+        existing = %x(gh pr list --head #{Shellwords.escape(branch)} --json url --jq ".[0].url" 2>/dev/null).strip
+
+        # Cut the branch from the pinned base, write the corrected files, commit, and push. Abort on
+        # any git failure so a partial or wrong result never looks like success.
+        unless system("git", "checkout", "-B", branch, base, out: File::NULL, err: File::NULL)
+          fail!("could not create branch `#{branch}` from #{base_short}")
+        end
+        files.each { |p| File.write(p, changed[p]) }
+        system("git", "add", *files)
+        # `git commit` exits non-zero when nothing is staged (a fix that re-indents to the original);
+        # treat that as a no-op and skip the push/PR instead of opening an empty PR.
+        next unless system("git", "commit", "-m", pr[:title], out: File::NULL, err: File::NULL)
+
+        unless system("git", "push", "-u", "origin", branch, "--force-with-lease", out: File::NULL, err: File::NULL)
+          fail!("could not push `#{branch}` to origin")
+        end
+
+        # Open a fresh PR, or note that we refreshed the branch of an existing one.
+        if existing.empty?
+          body_file = tmpdir.join("pr_body_#{pr[:id]}.md")
+          File.write(body_file, pr[:body])
+          create = ["gh", "pr", "create", "--draft", "--base", pr[:base], "--head", branch, "--title", pr[:title], "--body-file", body_file.to_s]
+          opened = %x(#{Shellwords.join(create)} 2>&1).strip
+        else
+          opened = "#{existing} (branch updated)"
+        end
+      end
+    ensure
+      system("git", "checkout", original, out: File::NULL, err: File::NULL)
+    end
+
+    opened
+  end
+
+  # Phase 5: user-facing summary
+  ruby(:summary) do
+    plan = ruby!(:plan).value
+    counts = plan[:counts]
+
+    puts "\n=== Comment audit ==="
+    puts "checked: #{counts[:checked]}\n" \
+      "factual: #{counts[:ok]}  \n" \
+      "stale (auto-fixable): #{counts[:failing]}\n" \
+      "low-confidence (not auto-fixable): #{counts[:low_confidence]}"
+    # Stale comments to fix, grouped by file, with the specific issues found.
+    unless plan[:failing].empty?
+      puts "stale comments to fix (#{counts[:failing]}):"
+      plan[:failing].group_by { |v| v[:unit][:file] }.sort.each do |file, verds|
+        puts "  #{file}"
+        verds.each do |v|
+          puts "    · #{v[:unit][:symbol]}  (confidence #{v[:confidence]})"
+          Array(v[:issues]).each { |issue| puts "        - #{issue}" }
+        end
+      end
+    end
+
+    # Low-confidence findings are surfaced for a human but never auto-fixed.
+    unless plan[:low_confidence].empty?
+      puts "low-confidence findings (reported only, not auto-fixed):"
+      plan[:low_confidence].each do |v|
+        puts "  · #{v[:unit][:symbol]} in #{v[:unit][:file]}  (confidence #{v[:confidence]})"
+        Array(v[:issues]).each { |issue| puts "      - #{issue}" }
+      end
+    end
+
+    # In apply mode, print the single PR URL, or state that there was nothing to open.
+    if arg?(:apply)
+      url = ruby!(:apply).value
+      if url
+        puts "opened / updated PR: #{url}"
+      else
+        puts "no stale comments; nothing to open."
+      end
+    else
+      puts "(dry-run; no PRs opened, pass `-- apply` to open them)"
+    end
+  end
+end
+
+# Named scope: verifies one atomic comment unit (a doc comment or an inline comment)
+execute(:verify_one) do
+  # Evaluates one unit at a time, returning a JSON.
+  agent(:judge) do |_, unit|
+    [
+      template("verify_comment", { unit: unit }),
+      "Now output ONLY the JSON verdict described above with no surrounding text.",
+    ]
+  end
+
+  # The scope's return value. Parse the JSON verdict. If parsing fails, record a non-factual,
+  # zero-confidence verdict so the unit is surfaced as report-only.
+  outputs! do |unit, _idx|
+    verdict =
+      begin
+        agent!(:judge).json!
+      rescue => e
+        { factual: false, issues: ["verdict could not be parsed: #{e.message}"], confidence: 0.0, suggested_fix: unit[:comment_text], parse_error: e.message }
+      end
+    verdict.merge(unit: unit)
+  end
+end