[autorevert] dedupe shared recovery-detection pipeline across the two metrics queries (#8180)

## Summary

`autorevert_significant_reverts` and `autorevert_weekly_metrics` both
implement the same red/green-streak recovery-detection +
causal-attribution pipeline — ~140 identical lines (`commits` → … →
`recovery_with_attribution`). They've drifted once already: the causal
red-streak filter from #8176 had to be applied to each file separately,
and fixing only the summary query left the weekly chart stale until both
were patched.

This PR makes that shared pipeline a single, drift-proof definition.

## Approach

torchci has no SQL include/templating mechanism (each `query.sql` is
sent to ClickHouse verbatim), and ClickHouse view DDL is deployed
manually / out-of-band. A shared **parameterized view** would be the DRY
end-state, but it can't be created or verified from the read-only query
path, and a merged query referencing a not-yet-deployed view would 500
the HUD metrics page until the DDL is applied by hand. So this keeps
both queries self-contained and runnable, but:

1. Factors the **entire** shared pipeline — `commits` through
`recovery_with_attribution`, plus a new `causally_attributed_recoveries`
CTE that **centralizes the #8176 causal filter** (`reverted_commit_sha =
'' OR has(red_shas, reverted_commit_sha)`) — into a block delimited by
`-- @autorevert-shared-recovery-pipeline:begin` / `:end` markers. The
block is **byte-identical** in both files. Only each query's final
aggregation (per-revert-commit vs per-week) differs, below the `:end`
marker.

2. Adds a **jest unit test**
(`torchci/test/autorevertSharedPipeline.test.ts`) that fails CI if the
two marked blocks aren't byte-identical, if a marker is
missing/duplicated, or if the two `params.json` declare different
`params`. The extractor itself is unit-tested (missing / duplicated /
reversed markers) so the guard can't silently no-op. A future fix to the
recovery pipeline therefore *cannot* land in one query but not the
other.

Moving the causal filter into the shared
`causally_attributed_recoveries` CTE is the key win: previously each
query re-stated it in its own tail (`reverts_only` /
`unique_recoveries`), which is exactly what drifted in #8176. Now it
lives once, inside the byte-identical block.

> **Note:** an earlier revision of this PR enforced the byte-identity
with a `lintrunner` adapter (`AUTOREVERT_SHARED_CTE`); per review it's
now a torchci jest test instead, so the guard runs in the normal test
suite rather than extending the linter stack.

## Validation

Confirmed against ClickHouse that both refactored queries return results
**identical** to the pre-refactor versions:

| query | window | result |
|---|---|---|
| `autorevert_significant_reverts` | 2026-06-08 .. 06-15 | identical (3
rows; PR #186928 / `889f6eb` correctly stays excluded) |
| `autorevert_weekly_metrics` | 2026-06-08 .. 06-15 | identical
(`human_revert_recoveries = 0`) |
| `autorevert_weekly_metrics` | 2026-05-01 .. 06-15 | identical across
all 7 weeks (incl. human-revert and non-revert paths) |

The transformation is algebraically equivalence-preserving:
`significant`'s `reverts_only` predicate `is_revert = 1 AND <causal>`
becomes `<causal>` (applied upstream) then `is_revert = 1`; `weekly`'s
`unique_recoveries` reads the same pre-filtered set it did before. The
extra (superset) columns now carried through `weekly`'s shared CTEs are
unused by its final aggregation and don't change row cardinality.

`prettier --check`, `tsc`, and `eslint` run clean on the new test
locally (the jest run itself needs Node ≥18; the test reads the two real
query files and asserts byte-identity + equal params, validated
independently here).

## A parameterized view as a possible future migration

If the team prefers the fully DRY form, this shared block is a clean
candidate for a parameterized ClickHouse view that both queries `SELECT`
from — but that needs a coordinated manual DDL deploy (the view must
exist before the deployed app references it) plus a one-time
confirmation that the read user can `SELECT` from a parameterized view
taking an `Array(String)` argument. I'm happy to follow up with that as
a separate, deploy-coordinated PR. This change is the deployment-free
first step that closes the drift hole now.

---------

Co-authored-by: Ivan <izaitsevfb@meta.com>

Author: izaitsevfb

torchci/clickhouse_queries/autorevert_significant_reverts/query.sql

@@ -2,6 +2,14 @@
 -- Finds recovery events that are reverts and attributes them to autorevert vs human
 -- Used for autorevert metrics precision/recall calculations
 
+-- The block between the @autorevert-shared-recovery-pipeline markers below is kept
+-- BYTE-IDENTICAL with autorevert_weekly_metrics/query.sql. The
+-- autorevertSharedPipeline test (torchci/test/autorevertSharedPipeline.test.ts)
+-- fails CI if the two copies drift, so any change to the recovery-detection or
+-- causal-attribution pipeline (e.g. the #8176 causal red-streak filter) MUST be
+-- applied identically to BOTH files. Only each query's final aggregation, below the
+-- :end marker, is allowed to differ.
+-- @autorevert-shared-recovery-pipeline:begin
 WITH commits AS (
     SELECT
         push.head_commit.'timestamp' AS time,
@@ -264,21 +272,32 @@ recovery_with_attribution AS (
     LEFT JOIN autorevert_events a ON r.reverted_commit_sha = a.reverted_sha
 ),
 
--- Filter to only actual reverts before aggregating, and require the reverted
--- commit to actually belong to the red streak it supposedly recovered. This drops
--- spurious recoveries where an unrelated/flaky signal merely went red->green at the
--- revert commit while the reverted commit was never part of that red streak (e.g.
--- out-of-plane reverts -- ghfirst/nosignal -- credited with a coincidental flake
--- clear). When the reverted commit SHA can't be parsed from the message (e.g.
--- Reapply / Back out shapes), the row is kept unchanged.
-reverts_only AS (
+-- Step 10: Apply the causal-attribution filter centrally, so BOTH downstream
+-- queries (autorevert_significant_reverts and autorevert_weekly_metrics) share it
+-- and cannot drift. A revert only "fixes" a signal when the reverted commit is
+-- actually part of the red streak that recovered; spurious recoveries -- an
+-- unrelated/flaky signal that merely went red->green at the revert commit while the
+-- reverted commit was never in that red streak (e.g. out-of-plane ghfirst/nosignal
+-- reverts credited with a coincidental flake clear) -- are dropped. When the
+-- reverted commit SHA can't be parsed from the message (Reapply / Back out shapes),
+-- the row is kept unchanged.
+causally_attributed_recoveries AS (
     SELECT * FROM recovery_with_attribution
     WHERE
-        is_revert = 1
-        AND (
-            reverted_commit_sha = ''
-            OR has(red_shas, reverted_commit_sha)
-        )
+        reverted_commit_sha = ''
+        OR has(red_shas, reverted_commit_sha)
+),
+-- @autorevert-shared-recovery-pipeline:end
+
+-- ===========================================================================
+-- Query-specific tail: per-revert-commit detail for the precision/recall table.
+-- ===========================================================================
+
+-- Filter to only actual reverts (the causal-attribution filter is already applied
+-- upstream in causally_attributed_recoveries).
+reverts_only AS (
+    SELECT * FROM causally_attributed_recoveries
+    WHERE is_revert = 1
 ),
 
 -- Aggregate by recovery_sha (one row per unique revert commit)

torchci/clickhouse_queries/autorevert_weekly_metrics/query.sql

@@ -2,6 +2,14 @@
 -- Aggregates signal recovery and revert data by week for trend analysis
 -- Computes key metrics: total recoveries, autorevert rate, human revert rate
 
+-- The block between the @autorevert-shared-recovery-pipeline markers below is kept
+-- BYTE-IDENTICAL with autorevert_significant_reverts/query.sql. The
+-- autorevertSharedPipeline test (torchci/test/autorevertSharedPipeline.test.ts)
+-- fails CI if the two copies drift, so any change to the recovery-detection or
+-- causal-attribution pipeline (e.g. the #8176 causal red-streak filter) MUST be
+-- applied identically to BOTH files. Only each query's final aggregation, below the
+-- :end marker, is allowed to differ.
+-- @autorevert-shared-recovery-pipeline:begin
 WITH commits AS (
     SELECT
         push.head_commit.'timestamp' AS time,
@@ -42,6 +50,7 @@ all_jobs AS (
         all_runs.workflow_name AS workflow_name,
         job.run_attempt AS run_attempt,
         job.conclusion AS raw_conclusion,
+        -- Normalize job name to group shards together (same as auto-revert logic)
         trim(
             replaceRegexpAll(
                 replaceRegexpAll(
@@ -64,6 +73,7 @@ all_jobs AS (
         )
 ),
 
+-- Step 1: For each (sha, base_name, run_attempt), determine attempt status
 attempt_status AS (
     SELECT
         time,
@@ -79,6 +89,7 @@ attempt_status AS (
     GROUP BY time, sha, message, base_name, workflow_name, run_attempt
 ),
 
+-- Step 2: For each (sha, base_name), aggregate across all attempts
 signal_status AS (
     SELECT
         time,
@@ -96,6 +107,7 @@ signal_status AS (
     GROUP BY time, sha, message, base_name
 ),
 
+-- Step 3: Assign streak IDs using cumulative status changes
 signal_with_streaks AS (
     SELECT
         base_name,
@@ -104,25 +116,30 @@ signal_with_streaks AS (
         time,
         message,
         status,
+        -- Change marker: 1 when status differs from previous
         if(status != lagInFrame(status, 1, status) OVER w, 1, 0) AS is_change
     FROM signal_status
-    WHERE status IN ('red', 'green')
+    WHERE status IN ('red', 'green')  -- Focus on definitive states
     WINDOW w AS (
         PARTITION BY base_name
         ORDER BY time ASC
     )
 ),
 
+-- Step 4: Compute streak ID (cumulative sum of changes)
 signal_with_streak_ids AS (
     SELECT
         *,
-        sum(is_change) OVER (
-            PARTITION BY base_name
-            ORDER BY time ASC ROWS UNBOUNDED PRECEDING
-        ) AS streak_id
+        sum(is_change)
+            OVER (
+                PARTITION BY base_name
+                ORDER BY time ASC ROWS UNBOUNDED PRECEDING
+            )
+            AS streak_id
     FROM signal_with_streaks
 ),
 
+-- Step 5: Count streak lengths and find boundaries
 streak_lengths AS (
     SELECT
         base_name,
@@ -138,9 +155,11 @@ streak_lengths AS (
     GROUP BY base_name, streak_id, status
 ),
 
--- SHAs comprising each red streak, for causal attribution: a revert only
--- genuinely "fixes" a signal if the reverted commit is part of the red streak
--- that recovered (mirrors autorevert_significant_reverts).
+-- Step 5b: Collect the SHAs that make up each red streak (for causal attribution).
+-- A revert only genuinely "fixes" a signal if the reverted commit is actually part
+-- of the red streak that recovered. Otherwise the red->green transition at the
+-- revert commit is coincidental -- e.g. a flaky signal that merely happened to go
+-- green at the revert -- and crediting the revert with it inflates the FN count.
 red_streak_members AS (
     SELECT
         base_name,
@@ -151,6 +170,7 @@ red_streak_members AS (
     GROUP BY base_name, streak_id
 ),
 
+-- Step 6: Find recovery events: green streak that follows a red streak
 recovery_events AS (
     SELECT
         green.base_name AS signal_key,
@@ -161,7 +181,9 @@ recovery_events AS (
         green.streak_start AS recovery_time,
         green.first_message AS recovery_message,
         red.last_sha AS last_red_sha,
-        red.streak_end AS last_red_time
+        red.streak_end AS last_red_time,
+        red.first_sha AS first_red_sha,
+        red.streak_start AS first_red_time
     FROM streak_lengths green
     JOIN streak_lengths red
         ON
@@ -173,7 +195,7 @@ recovery_events AS (
         AND green.streak_length >= {minGreenCommits: UInt8}
 ),
 
--- Get autorevert events for attribution
+-- Step 7: Get autorevert events for attribution
 autorevert_events AS (
     SELECT
         toString(commit_sha) AS reverted_sha,
@@ -185,24 +207,37 @@ autorevert_events AS (
         AND action = 'revert'
         AND dry_run = 0
         AND failed = 0
+        -- Convert DateTime64 params to DateTime for comparison
         AND ts >= toDateTime({startTime: DateTime64(3)}) - INTERVAL 1 DAY
         AND ts < toDateTime({stopTime: DateTime64(3)}) + INTERVAL 1 DAY
 ),
 
--- Extract reverted commit SHA from recovery message
+-- Step 8: Extract reverted commit SHA from recovery message
 recovery_with_reverted_sha AS (
     SELECT
         r.*,
+        -- Check if recovery commit is a revert
         (
             r.recovery_message LIKE 'Revert %'
             OR r.recovery_message LIKE 'Reapply %'
             OR r.recovery_message LIKE 'Back out%'
         ) AS is_revert,
+        -- Extract reverted PR number if it's a revert
+        extractAll(
+            r.recovery_message,
+            'Reverted https://github.com/pytorch/pytorch/pull/(\\d+)'
+        ) AS reverted_pr_numbers,
+        -- Extract PR number from merge commit message
+        extractAll(
+            r.recovery_message,
+            'Pull Request resolved: https://github.com/pytorch/pytorch/pull/(\\d+)'
+        ) AS merge_pr_numbers,
         -- Extract the actual reverted commit SHA from message (e.g., "This reverts commit abc123...")
         -- The regex captures the full 40-char SHA since commit messages include full SHAs
         arrayElement(
             extractAll(r.recovery_message, 'reverts commit ([a-f0-9]+)'), 1
         ) AS reverted_commit_sha,
+        -- SHAs comprising the red streak this recovery resolves (causal filter input)
         rm.red_shas AS red_shas
     FROM recovery_events r
     LEFT JOIN red_streak_members rm
@@ -211,7 +246,7 @@ recovery_with_reverted_sha AS (
             AND rm.streak_id = r.red_streak_id
 ),
 
--- Join with autorevert events on full SHA match
+-- Step 9: Join with autorevert events on full SHA match
 recovery_with_attribution AS (
     SELECT
         r.signal_key,
@@ -222,32 +257,52 @@ recovery_with_attribution AS (
         r.recovery_message,
         r.last_red_sha,
         r.last_red_time,
+        r.first_red_sha,
+        r.first_red_time,
         r.is_revert,
+        r.reverted_pr_numbers,
+        r.merge_pr_numbers,
         r.reverted_commit_sha,
         r.red_shas,
         -- Check for autorevert attribution by matching the reverted commit SHA
-        a.reverted_sha IS NOT NULL AND a.reverted_sha != '' AS is_autorevert
+        a.reverted_sha IS NOT NULL AND a.reverted_sha != '' AS is_autorevert,
+        a.autorevert_time,
+        a.source_signal_keys AS autorevert_signal_keys
     FROM recovery_with_reverted_sha r
     LEFT JOIN autorevert_events a ON r.reverted_commit_sha = a.reverted_sha
 ),
 
--- Deduplicate by recovery_sha to count unique revert commits
--- A single revert can fix multiple signals, but we count it as one revert event
+-- Step 10: Apply the causal-attribution filter centrally, so BOTH downstream
+-- queries (autorevert_significant_reverts and autorevert_weekly_metrics) share it
+-- and cannot drift. A revert only "fixes" a signal when the reverted commit is
+-- actually part of the red streak that recovered; spurious recoveries -- an
+-- unrelated/flaky signal that merely went red->green at the revert commit while the
+-- reverted commit was never in that red streak (e.g. out-of-plane ghfirst/nosignal
+-- reverts credited with a coincidental flake clear) -- are dropped. When the
+-- reverted commit SHA can't be parsed from the message (Reapply / Back out shapes),
+-- the row is kept unchanged.
+causally_attributed_recoveries AS (
+    SELECT * FROM recovery_with_attribution
+    WHERE
+        reverted_commit_sha = ''
+        OR has(red_shas, reverted_commit_sha)
+),
+-- @autorevert-shared-recovery-pipeline:end
+
+-- ===========================================================================
+-- Query-specific tail: weekly aggregation of recovery counts and rates.
+-- ===========================================================================
+
+-- Deduplicate by recovery_sha to count unique revert commits. A single revert can
+-- fix multiple signals, but we count it as one revert event. The causal-attribution
+-- filter is already applied upstream in causally_attributed_recoveries.
 unique_recoveries AS (
     SELECT
         recovery_sha,
         any(recovery_time) AS recovery_time,
         max(is_revert) AS is_revert,
         max(is_autorevert) AS is_autorevert
-    FROM recovery_with_attribution
-    -- Causal filter (mirrors autorevert_significant_reverts): a revert only counts
-    -- as fixing a signal if the reverted commit is part of that signal's red streak.
-    -- Drops coincidental flake-clears at the revert commit so the weekly chart's
-    -- human_revert_recoveries / recall stay consistent with the summary numbers.
-    -- Non-revert recoveries have an empty reverted_commit_sha and are kept.
-    WHERE
-        reverted_commit_sha = ''
-        OR has(red_shas, reverted_commit_sha)
+    FROM causally_attributed_recoveries
     GROUP BY recovery_sha
 )