Skip to content

Commit 83e7f8a

Browse files
committed
feat: read AI advisor verdicts from CH and use in autorevert decisions
When advisor verdicts are available in misc.autorevert_advisor_verdicts, autorevert now uses them to make faster decisions: - revert → produce AutorevertPattern immediately (skip restarts) - not_related → early exit (Ineligible) - garbage → suppress signal for 2 hours since verdict timestamp - unsure → continue normal restart-to-confirm flow Key changes: - AIAdvisorResult dataclass + AdvisorVerdict enum in signal.py - SignalCommit gets optional advisor_result field - fetch_advisor_verdicts() query in signal_extraction_datasource.py - _attach_advisor_verdicts() in signal_extraction.py wires CH data to SignalCommits - Flakiness check moved AFTER advisor check/dispatch so advisor can still evaluate flaky-looking signals - 8 new tests covering all verdict types, expiry, signal key matching
1 parent 9c0a519 commit 83e7f8a

6 files changed

Lines changed: 686 additions & 31 deletions

File tree

aws/lambda/pytorch-auto-revert/pytorch_auto_revert/signal.py

Lines changed: 128 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,29 @@
66
from .bisection_planner import GapBisectionPlanner
77

88

9+
class AdvisorVerdict(Enum):
10+
"""Verdict from the AI advisor workflow."""
11+
12+
REVERT = "revert"
13+
UNSURE = "unsure"
14+
NOT_RELATED = "not_related"
15+
GARBAGE = "garbage"
16+
17+
18+
@dataclass(frozen=True)
19+
class AIAdvisorResult:
20+
"""Result from a previously-dispatched AI advisor run, read from ClickHouse.
21+
22+
Attached to SignalCommit so that process_valid_autorevert_pattern() can
23+
use the advisor's analysis to make faster revert/skip decisions.
24+
"""
25+
26+
verdict: AdvisorVerdict
27+
confidence: float
28+
timestamp: datetime # when the verdict was produced
29+
signal_key: str # the signal key this verdict applies to
30+
31+
932
class SignalStatus(Enum):
1033
"""signal status enum"""
1134

@@ -76,6 +99,8 @@ class IneligibleReason(Enum):
7699
"insufficient_successes" # not enough successes to make call
77100
)
78101
PENDING_GAP = "pending_gap" # unknown/pending commits present
102+
ADVISOR_NOT_RELATED = "advisor_not_related" # AI advisor says not related
103+
ADVISOR_GARBAGE = "advisor_garbage" # AI advisor says signal is garbage
79104

80105

81106
@dataclass
@@ -128,7 +153,13 @@ def is_failure(self) -> bool:
128153
class SignalCommit:
129154
"""All events for a single commit, ordered oldest → newest by start time."""
130155

131-
def __init__(self, head_sha: str, timestamp: datetime, events: List[SignalEvent]):
156+
def __init__(
157+
self,
158+
head_sha: str,
159+
timestamp: datetime,
160+
events: List[SignalEvent],
161+
advisor_result: Optional[AIAdvisorResult] = None,
162+
):
132163
self.head_sha = head_sha
133164
self.timestamp = timestamp
134165
# enforce events ordered by time, then by wf_run_id (oldest first)
@@ -139,6 +170,8 @@ def __init__(self, head_sha: str, timestamp: datetime, events: List[SignalEvent]
139170
self.statuses = {}
140171
for e in self.events:
141172
self.statuses[e.status] = self.statuses.get(e.status, 0) + 1
173+
# Optional AI advisor result for this (commit, signal) pair
174+
self.advisor_result = advisor_result
142175

143176
@property
144177
def has_pending(self) -> bool:
@@ -384,8 +417,10 @@ def partition_by_autorevert_pattern(self) -> Optional[PartitionedCommits]:
384417
picking_failed = True # simple state machine
385418

386419
# first broadly partition into failed and successful
420+
# A commit with both success and failure (flaky) stays in the failed
421+
# partition — only pure successes trigger the transition.
387422
for c in self.commits:
388-
if c.has_success:
423+
if c.has_success and not c.has_failure:
389424
picking_failed = False
390425
elif c.has_failure and not picking_failed:
391426
# encountered a failure after the streak of successes
@@ -409,24 +444,93 @@ def partition_by_autorevert_pattern(self) -> Optional[PartitionedCommits]:
409444

410445
return PartitionedCommits(failed=failed, unknown=unknown, successful=successful)
411446

447+
# Minimum confidence threshold for acting on advisor verdicts
448+
ADVISOR_CONFIDENCE_THRESHOLD = 0.9
449+
450+
def _build_autorevert_pattern(
451+
self, partition: "PartitionedCommits"
452+
) -> AutorevertPattern:
453+
"""Build an AutorevertPattern from a validated partition."""
454+
suspected = partition.failed[-1]
455+
newer_failures = [c.head_sha for c in partition.failed[:-1]]
456+
failure_event = next(
457+
(e for e in suspected.events if e.is_failure and e.job_id is not None),
458+
None,
459+
)
460+
return AutorevertPattern(
461+
workflow_name=self.workflow_name,
462+
newer_failing_commits=newer_failures,
463+
suspected_commit=suspected.head_sha,
464+
older_successful_commit=partition.successful[0].head_sha,
465+
wf_run_id=failure_event.wf_run_id if failure_event else None,
466+
job_id=failure_event.job_id if failure_event else None,
467+
)
468+
469+
def _check_advisor_verdict(
470+
self, partition: "PartitionedCommits"
471+
) -> Optional[Union[AutorevertPattern, Ineligible]]:
472+
"""Check if an AI advisor verdict is available for the suspect commit.
473+
474+
Returns:
475+
- AutorevertPattern if advisor says "revert" with sufficient confidence
476+
- Ineligible if advisor says "not_related" or "garbage" (within 2h window)
477+
- None if no verdict, verdict is "unsure", or confidence below threshold
478+
"""
479+
suspected = partition.failed[-1]
480+
result = suspected.advisor_result
481+
if result is None:
482+
return None
483+
484+
# Only act on the verdict if it matches this signal
485+
if result.signal_key != self.key:
486+
return None
487+
488+
# Ignore low-confidence verdicts
489+
if result.confidence < self.ADVISOR_CONFIDENCE_THRESHOLD:
490+
return None
491+
492+
if result.verdict == AdvisorVerdict.REVERT:
493+
return self._build_autorevert_pattern(partition)
494+
495+
if result.verdict == AdvisorVerdict.NOT_RELATED:
496+
return Ineligible(
497+
IneligibleReason.ADVISOR_NOT_RELATED,
498+
f"AI advisor says not related (confidence={result.confidence:.2f})",
499+
)
500+
501+
if result.verdict == AdvisorVerdict.GARBAGE:
502+
# Garbage verdict blocks the signal for 2 hours since the verdict timestamp
503+
from datetime import timezone
504+
505+
now = datetime.now(timezone.utc)
506+
verdict_age = now - result.timestamp.replace(tzinfo=timezone.utc)
507+
if verdict_age.total_seconds() < 2 * 3600:
508+
return Ineligible(
509+
IneligibleReason.ADVISOR_GARBAGE,
510+
f"AI advisor says garbage signal, suppressed for 2h "
511+
f"(confidence={result.confidence:.2f}, "
512+
f"age={int(verdict_age.total_seconds() / 60)}min)",
513+
)
514+
# Garbage verdict expired — fall through to normal processing
515+
516+
# "unsure" or expired garbage → continue normally
517+
return None
518+
412519
def process_valid_autorevert_pattern(
413520
self, *, bisection_limit: Optional[int] = None
414521
) -> Union[AutorevertPattern, RestartCommits, Ineligible]:
415522
"""
416523
Detect valid autorevert pattern in the Signal.
417524
418-
Validates all invariants before checking for the pattern.
525+
Validates invariants, checks AI advisor verdicts, and determines
526+
appropriate action.
419527
420528
Returns one of:
421529
- AutorevertPattern: a confirmed pattern ready for action
422530
- RestartCommits: a suggested set of commits to restart to reduce uncertainty
423531
- Ineligible: reason + optional message when no pattern is actionable yet
424532
"""
425-
if self.detect_flaky():
426-
return Ineligible(
427-
IneligibleReason.FLAKY,
428-
"signal is flaky (mixed outcomes on same commit)",
429-
)
533+
# Early exits that prevent partition computation
430534
if self.detect_fixed():
431535
return Ineligible(
432536
IneligibleReason.FIXED, "signal appears recovered at head"
@@ -443,7 +547,12 @@ def process_valid_autorevert_pattern(
443547
"insufficient history to form failed/unknown/successful partitions",
444548
)
445549

446-
# Advisor eligibility: partition exists with sufficient data and
550+
# --- AI advisor verdict check (before flakiness and other heuristics) ---
551+
advisor_decision = self._check_advisor_verdict(partition)
552+
if advisor_decision is not None:
553+
return advisor_decision
554+
555+
# Advisor dispatch eligibility: partition exists with sufficient data and
447556
# no unknown commits between failed and successful partitions.
448557
# Unknown commits mean the partition boundary is uncertain —
449558
# wait for restarts to resolve before dispatching advisor.
@@ -459,6 +568,15 @@ def process_valid_autorevert_pattern(
459568
successful_commits=tuple(c.head_sha for c in partition.successful),
460569
)
461570

571+
# Flakiness check — placed after advisor check/dispatch so that
572+
# advisor can still evaluate flaky-looking signals
573+
if self.detect_flaky():
574+
return Ineligible(
575+
IneligibleReason.FLAKY,
576+
"signal is flaky (mixed outcomes on same commit)",
577+
advisor=advisor,
578+
)
579+
462580
restart_commits = set()
463581

464582
# Cover the unknown gap (between failed and successful partitions)
@@ -554,21 +672,4 @@ def process_valid_autorevert_pattern(
554672
)
555673

556674
# all invariants validated, confirmed not infra, pattern exists
557-
# failed is newest -> older; the last element is the suspected commit
558-
suspected = partition.failed[-1]
559-
newer_failures = [c.head_sha for c in partition.failed[:-1]]
560-
561-
# Extract job_id and wf_run_id from a failing event on the suspected commit
562-
failure_event = next(
563-
(e for e in suspected.events if e.is_failure and e.job_id is not None),
564-
None,
565-
)
566-
567-
return AutorevertPattern(
568-
workflow_name=self.workflow_name,
569-
newer_failing_commits=newer_failures,
570-
suspected_commit=suspected.head_sha,
571-
older_successful_commit=partition.successful[0].head_sha,
572-
wf_run_id=failure_event.wf_run_id if failure_event else None,
573-
job_id=failure_event.job_id if failure_event else None,
574-
)
675+
return self._build_autorevert_pattern(partition)

aws/lambda/pytorch-auto-revert/pytorch_auto_revert/signal_extraction.py

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,15 @@
1212
from typing import Dict, Iterable, List, Optional, Set, Tuple
1313

1414
from .job_agg_index import JobAggIndex, JobMeta, SignalStatus as AggStatus
15-
from .signal import Signal, SignalCommit, SignalEvent, SignalSource, SignalStatus
15+
from .signal import (
16+
AdvisorVerdict,
17+
AIAdvisorResult,
18+
Signal,
19+
SignalCommit,
20+
SignalEvent,
21+
SignalSource,
22+
SignalStatus,
23+
)
1624
from .signal_extraction_datasource import SignalExtractionDatasource
1725
from .signal_extraction_types import (
1826
JobBaseName,
@@ -107,7 +115,10 @@ def extract(self) -> List[Signal]:
107115

108116
# Inject synthetic PENDING events for workflow runs that are known to be
109117
# pending but have no events in a given signal (e.g. multi-stage workflows).
110-
return self._inject_pending_workflow_events(signals, jobs)
118+
signals = self._inject_pending_workflow_events(signals, jobs)
119+
120+
# Attach AI advisor verdicts to SignalCommit objects
121+
return self._attach_advisor_verdicts(signals, commits)
111122

112123
# -----------------------------
113124
# Deduplication (GitHub-specific)
@@ -230,6 +241,71 @@ def _inject_pending_workflow_events(
230241
)
231242
return out
232243

244+
# -----------------------------
245+
# Advisor verdict attachment
246+
# -----------------------------
247+
def _attach_advisor_verdicts(
248+
self,
249+
signals: List[Signal],
250+
commits: List[Tuple[Sha, datetime]],
251+
) -> List[Signal]:
252+
"""Fetch advisor verdicts from CH and attach to SignalCommit objects.
253+
254+
For each (commit_sha, signal_key) pair that has a verdict in
255+
misc.autorevert_advisor_verdicts, sets the advisor_result field
256+
on the corresponding SignalCommit.
257+
"""
258+
head_shas = [sha for sha, _ in commits]
259+
signal_keys = list({s.key for s in signals})
260+
verdicts = self._datasource.fetch_advisor_verdicts(
261+
repo_full_name=self.repo_full_name,
262+
head_shas=head_shas,
263+
signal_keys=signal_keys,
264+
lookback_hours=self.lookback_hours,
265+
)
266+
if not verdicts:
267+
return signals
268+
269+
out: List[Signal] = []
270+
for s in signals:
271+
new_commits: List[SignalCommit] = []
272+
for c in s.commits:
273+
key = (c.head_sha.strip(), s.key)
274+
v = verdicts.get(key)
275+
if v is not None:
276+
verdict_str, confidence, ts = v
277+
try:
278+
advisor_verdict = AdvisorVerdict(verdict_str)
279+
except ValueError:
280+
advisor_verdict = AdvisorVerdict.UNSURE
281+
advisor_result = AIAdvisorResult(
282+
verdict=advisor_verdict,
283+
confidence=confidence,
284+
timestamp=ts,
285+
signal_key=s.key,
286+
)
287+
new_commits.append(
288+
SignalCommit(
289+
head_sha=c.head_sha,
290+
timestamp=c.timestamp,
291+
events=c.events,
292+
advisor_result=advisor_result,
293+
)
294+
)
295+
else:
296+
new_commits.append(c)
297+
out.append(
298+
Signal(
299+
key=s.key,
300+
workflow_name=s.workflow_name,
301+
commits=new_commits,
302+
job_base_name=s.job_base_name,
303+
test_module=s.test_module,
304+
source=s.source,
305+
)
306+
)
307+
return out
308+
233309
# -----------------------------
234310
# Phase B — Tests (tests.all_test_runs only)
235311
# -----------------------------

aws/lambda/pytorch-auto-revert/pytorch_auto_revert/signal_extraction_datasource.py

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,64 @@ def fetch_tests_for_job_ids(
313313
)
314314
return rows
315315

316+
def fetch_advisor_verdicts(
317+
self,
318+
*,
319+
repo_full_name: str,
320+
head_shas: List[Sha],
321+
signal_keys: List[str],
322+
lookback_hours: int,
323+
) -> Dict[tuple[str, str], tuple[str, float, datetime]]:
324+
"""Fetch AI advisor verdicts from misc.autorevert_advisor_verdicts.
325+
326+
Queries by both commit SHAs AND signal keys to minimize data transferred.
327+
328+
Returns a dict keyed by (commit_sha, signal_key) → (verdict, confidence, timestamp).
329+
When multiple verdicts exist for the same (commit, signal), the most recent is used.
330+
"""
331+
if not head_shas or not signal_keys:
332+
return {}
333+
334+
log = logging.getLogger(__name__)
335+
t0 = time.perf_counter()
336+
query = """
337+
SELECT
338+
toString(suspect_commit) AS suspect_commit,
339+
signal_key,
340+
verdict,
341+
confidence,
342+
timestamp
343+
FROM misc.autorevert_advisor_verdicts
344+
WHERE repo = {repo:String}
345+
AND suspect_commit IN {shas:Array(String)}
346+
AND signal_key IN {keys:Array(String)}
347+
AND timestamp > now() - INTERVAL {hours:UInt32} HOUR
348+
ORDER BY suspect_commit, signal_key, timestamp DESC
349+
"""
350+
params = {
351+
"repo": repo_full_name,
352+
"shas": [str(s) for s in head_shas],
353+
"keys": signal_keys,
354+
"hours": lookback_hours,
355+
}
356+
results: Dict[tuple[str, str], tuple[str, float, datetime]] = {}
357+
for attempt in RetryWithBackoff():
358+
with attempt:
359+
res = CHCliFactory().client.query(query, parameters=params)
360+
for r in res.result_rows:
361+
key = (str(r[0]).strip(), str(r[1]))
362+
if key not in results: # keep most recent (ORDER BY ... DESC)
363+
results[key] = (str(r[2]), float(r[3]), r[4])
364+
365+
dt = time.perf_counter() - t0
366+
log.info(
367+
"[extract] Advisor verdicts fetched: %d for %d commits in %.2fs",
368+
len(results),
369+
len(head_shas),
370+
dt,
371+
)
372+
return results
373+
316374
def fetch_autorevert_state_rows(
317375
self,
318376
*,

0 commit comments

Comments
 (0)