Skip to content

Commit b0bac05

Browse files
authored
[autorevert] Surface test identity at signal level for advisor JSON (#8031)
## Summary Surfaces authoritative TEST signal identity (`test_file` / `test_classname` / `test_name`) once at the top of the advisor `signal_pattern` payload, so the AI advisor has structured ground truth for which test failed without re-deriving it from logs. A TEST signal corresponds to a single test by construction (one signal per `(workflow, job_base, test_id)`), so the identity is emitted once at the Signal level, not per event. ## Classname uniqueness `test_id` is `file::name` — classname is dropped from the id, so one `file::name` can span multiple test classes. `test_classname` is therefore surfaced **only when a single distinct classname is observed** for the `test_id`; when ambiguous it is omitted (`None`) rather than guessed. (A first-seen capture with no ordering would be nondeterministic and could name a different class than the one that failed.) `test_file` / `test_name` are the stable components of the signal key. ## Changes - `signal.py`: `Signal` gains optional `test_file` / `test_classname` / `test_name`, threaded through `Signal.replace` so they survive the dedup/filter passes. - `signal_extraction.py`: `_build_test_signals` collects the distinct classnames per `test_id` and emits one only when unambiguous. - `signal_actions.py::_build_signal_pattern_json`: emits the three fields for TEST signals only, when populated; JOB signals omit them. - Tests: classname surfaced when unique, omitted when ambiguous; identity preserved through dedup. ## Payload shape ```json { "signal_key": "test/foo.py::test_bar", "signal_source": "test", "test_file": "test/foo.py", "test_classname": "TestFooBar", "test_name": "test_bar", "workflow_name": "trunk", "job_base_name": "linux-jammy / test", "suspect_commit": "abc...", "commits": [ ... ] } ``` ## Companion change pytorch/pytorch#182176 — the advisor prompt rewrite that consumes this structured identity. ## Test plan - [x] CI runs the existing + new unit tests under `pytorch_auto_revert/tests/` - [x] Manual: observe the three keys at the top of TEST advisor `signal_pattern` JSONs after merge
1 parent b9b2bab commit b0bac05

7 files changed

Lines changed: 397 additions & 4 deletions

File tree

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,9 @@ def __init__(
383383
job_base_name: Optional[str] = None,
384384
test_module: Optional[str] = None,
385385
source: SignalSource = SignalSource.TEST,
386+
test_file: Optional[str] = None,
387+
test_classname: Optional[str] = None,
388+
test_name: Optional[str] = None,
386389
):
387390
self.key = key
388391
self.workflow_name = workflow_name
@@ -393,6 +396,16 @@ def __init__(
393396
self.test_module = test_module
394397
# Track the origin of the signal (test-track or job-track).
395398
self.source = source
399+
# For TEST signals: structured identity of the failing test, sourced
400+
# from tests.all_test_runs and surfaced at the top of the advisor
401+
# signal_pattern JSON. test_file/test_name are authoritative — they
402+
# are the components of the signal key ("file::name"). test_classname
403+
# is only populated when a single classname was observed for this
404+
# test_id; it is None when the same file::name spans multiple classes
405+
# (ambiguous — better omitted than guessed).
406+
self.test_file = test_file
407+
self.test_classname = test_classname
408+
self.test_name = test_name
396409

397410
def replace(self, **changes) -> "Signal":
398411
"""Return a copy with selected fields replaced (`dataclasses.replace`-style).
@@ -407,6 +420,9 @@ def replace(self, **changes) -> "Signal":
407420
"job_base_name": changes.pop("job_base_name", self.job_base_name),
408421
"test_module": changes.pop("test_module", self.test_module),
409422
"source": changes.pop("source", self.source),
423+
"test_file": changes.pop("test_file", self.test_file),
424+
"test_classname": changes.pop("test_classname", self.test_classname),
425+
"test_name": changes.pop("test_name", self.test_name),
410426
}
411427
if changes:
412428
raise TypeError(

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

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from dataclasses import dataclass
77
from datetime import datetime, timedelta
88
from enum import Enum
9-
from typing import Dict, FrozenSet, Iterable, List, Optional, Tuple, Union
9+
from typing import Any, Dict, FrozenSet, Iterable, List, Optional, Tuple, Union
1010

1111
import github
1212

@@ -18,6 +18,7 @@
1818
Ineligible,
1919
RestartCommits,
2020
Signal,
21+
SignalSource,
2122
)
2223
from .signal_extraction_types import RunContext
2324
from .utils import (
@@ -863,15 +864,27 @@ def _partition_label(sha: str) -> str:
863864
}
864865
)
865866

866-
payload = {
867+
payload: Dict[str, Any] = {
867868
"signal_key": signal.key,
868869
"signal_source": signal.source.value if signal.source else "unknown",
869870
"workflow_name": signal.workflow_name,
870871
"job_base_name": signal.job_base_name,
871872
"commit_order": "newest_first",
872873
"suspect_commit": dispatch_advisor.suspect_commit,
873-
"commits": commits_json,
874874
}
875+
# For TEST signals, surface authoritative test identity once at the
876+
# top of the payload (file/classname/name from tests.all_test_runs).
877+
# Every FAILURE event in this signal IS this specific test failing —
878+
# the AI advisor does not need to re-derive from logs. Only emitted
879+
# when populated to keep the payload backward-compatible.
880+
if signal.source == SignalSource.TEST:
881+
if signal.test_file:
882+
payload["test_file"] = signal.test_file
883+
if signal.test_classname:
884+
payload["test_classname"] = signal.test_classname
885+
if signal.test_name:
886+
payload["test_name"] = signal.test_name
887+
payload["commits"] = commits_json
875888
if dispatch_advisor.is_born_red:
876889
payload["pattern_context"] = _BORN_RED_PATTERN_CONTEXT
877890
return json.dumps(payload)

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,13 @@ def _build_test_signals(
351351
failing_tests_by_job_base_name: Set[
352352
Tuple[WorkflowName, JobBaseName, TestId]
353353
] = set()
354+
# Capture structured test identity per test_id so we can attach it
355+
# once at the Signal level. The TestRow `test_id` property collapses
356+
# to "file::name" and drops classname, so one test_id may span
357+
# multiple classes; collect all distinct non-empty classnames and
358+
# only surface one later if it is unambiguous.
359+
test_file_name_by_test_id: Dict[TestId, Tuple[str, str]] = {}
360+
test_classnames_by_test_id: Dict[TestId, Set[str]] = {}
354361
for tr in test_rows:
355362
job = jobs_by_id.get(tr.job_id)
356363
job_base_name = job.base_name
@@ -377,6 +384,11 @@ def _build_test_signals(
377384
outcome = existing
378385

379386
tests_by_group_attempt[key] = outcome
387+
test_file_name_by_test_id.setdefault(tr.test_id, (tr.file, tr.name))
388+
if tr.classname:
389+
test_classnames_by_test_id.setdefault(tr.test_id, set()).add(
390+
tr.classname
391+
)
380392

381393
# Track keys that have at least one persistent failure (no retry success)
382394
if outcome.failure_runs > 0 and outcome.success_runs == 0:
@@ -490,6 +502,13 @@ def _build_test_signals(
490502
test_module = test_id.split("::")[0].replace(".py", "")
491503
else:
492504
test_module = None
505+
test_file, test_name = test_file_name_by_test_id.get(test_id, ("", ""))
506+
# Classname is only trustworthy when a single distinct value
507+
# was seen for this test_id; otherwise omit rather than guess.
508+
classnames = test_classnames_by_test_id.get(test_id, set())
509+
test_classname = (
510+
next(iter(classnames)) if len(classnames) == 1 else None
511+
)
493512

494513
signals.append(
495514
Signal(
@@ -499,6 +518,9 @@ def _build_test_signals(
499518
job_base_name=str(job_base_name),
500519
test_module=test_module,
501520
source=SignalSource.TEST,
521+
test_file=test_file or None,
522+
test_classname=test_classname or None,
523+
test_name=test_name or None,
502524
)
503525
)
504526

aws/lambda/pytorch-auto-revert/pytorch_auto_revert/tests/test_signal.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1129,6 +1129,9 @@ def setUp(self) -> None:
11291129
"job_base_name": "linux-jammy / test",
11301130
"test_module": "test_foo",
11311131
"source": SignalSource.JOB,
1132+
"test_file": "test/foo.py",
1133+
"test_classname": "TestFooBar",
1134+
"test_name": "test_bar",
11321135
}
11331136

11341137
def _init_params(self):

aws/lambda/pytorch-auto-revert/pytorch_auto_revert/tests/test_signal_actions.py

Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,6 +1100,130 @@ def test_signal_pattern_sanity_check(self):
11001100
reparsed = json.loads(json.dumps(result))
11011101
self.assertEqual(reparsed, result)
11021102

1103+
def test_test_identity_surfaced_for_test_signals(self):
1104+
"""TEST signals surface authoritative test identity (file/classname/name)
1105+
once at the top of the payload. Every FAILURE event in the signal IS
1106+
this specific test failing — no duplication per event."""
1107+
import json
1108+
1109+
from pytorch_auto_revert.signal import (
1110+
DispatchAdvisor,
1111+
Signal,
1112+
SignalCommit,
1113+
SignalEvent,
1114+
SignalSource,
1115+
SignalStatus,
1116+
)
1117+
1118+
t0 = datetime(2025, 8, 19, 12, 0, 0)
1119+
t1 = datetime(2025, 8, 19, 11, 0, 0)
1120+
1121+
c_fail = SignalCommit(
1122+
head_sha="sha_fail",
1123+
timestamp=t0,
1124+
events=[
1125+
SignalEvent(
1126+
"test", SignalStatus.FAILURE, t0, wf_run_id=100, job_id=200
1127+
),
1128+
],
1129+
)
1130+
c_base = SignalCommit(
1131+
head_sha="sha_base",
1132+
timestamp=t1,
1133+
events=[
1134+
SignalEvent("test", SignalStatus.SUCCESS, t1, wf_run_id=99, job_id=199),
1135+
],
1136+
)
1137+
signal = Signal(
1138+
key="test/inductor/test_comm_analysis.py::test_nccl_estimate_device_resolution_gpu",
1139+
workflow_name="trunk",
1140+
commits=[c_fail, c_base],
1141+
source=SignalSource.TEST,
1142+
job_base_name="linux-jammy / test",
1143+
test_file="test/inductor/test_comm_analysis.py",
1144+
test_classname="TestNcclEstimateDeviceResolution",
1145+
test_name="test_nccl_estimate_device_resolution_gpu",
1146+
)
1147+
advisor = DispatchAdvisor(
1148+
suspect_commit="sha_fail",
1149+
failed_commits=("sha_fail",),
1150+
successful_commits=("sha_base",),
1151+
)
1152+
1153+
result = json.loads(
1154+
SignalActionProcessor._build_signal_pattern_json(
1155+
signal=signal,
1156+
dispatch_advisor=advisor,
1157+
repo_full_name="pytorch/pytorch",
1158+
)
1159+
)
1160+
# Top-level test identity is present, populated, and authoritative
1161+
self.assertEqual(result["test_file"], "test/inductor/test_comm_analysis.py")
1162+
self.assertEqual(result["test_classname"], "TestNcclEstimateDeviceResolution")
1163+
self.assertEqual(
1164+
result["test_name"], "test_nccl_estimate_device_resolution_gpu"
1165+
)
1166+
1167+
# No per-event test_failures emission (would be redundant with signal_key)
1168+
for c in result["commits"]:
1169+
for ev in c["events"]:
1170+
self.assertNotIn("test_failures", ev)
1171+
self.assertNotIn("test_file", ev)
1172+
self.assertNotIn("test_classname", ev)
1173+
self.assertNotIn("test_name", ev)
1174+
1175+
def test_test_identity_omitted_for_job_signals(self):
1176+
"""JOB signals (and TEST signals without identity populated) must not
1177+
emit the test_* top-level keys."""
1178+
import json
1179+
1180+
from pytorch_auto_revert.signal import (
1181+
DispatchAdvisor,
1182+
Signal,
1183+
SignalCommit,
1184+
SignalEvent,
1185+
SignalSource,
1186+
SignalStatus,
1187+
)
1188+
1189+
t0 = datetime(2025, 8, 19, 12, 0, 0)
1190+
1191+
c_fail = SignalCommit(
1192+
head_sha="sha_fail",
1193+
timestamp=t0,
1194+
events=[
1195+
SignalEvent("j", SignalStatus.FAILURE, t0, wf_run_id=1, job_id=1),
1196+
],
1197+
)
1198+
c_base = SignalCommit(
1199+
head_sha="sha_base",
1200+
timestamp=t0,
1201+
events=[
1202+
SignalEvent("j", SignalStatus.SUCCESS, t0, wf_run_id=2, job_id=2),
1203+
],
1204+
)
1205+
signal = Signal(
1206+
key="lint",
1207+
workflow_name="trunk",
1208+
commits=[c_fail, c_base],
1209+
source=SignalSource.JOB,
1210+
)
1211+
advisor = DispatchAdvisor(
1212+
suspect_commit="sha_fail",
1213+
failed_commits=("sha_fail",),
1214+
successful_commits=("sha_base",),
1215+
)
1216+
1217+
result = json.loads(
1218+
SignalActionProcessor._build_signal_pattern_json(
1219+
signal=signal,
1220+
dispatch_advisor=advisor,
1221+
repo_full_name="pytorch/pytorch",
1222+
)
1223+
)
1224+
for k in ("test_file", "test_classname", "test_name"):
1225+
self.assertNotIn(k, result)
1226+
11031227

11041228
class TestDispatchAdvisorsMethod(unittest.TestCase):
11051229
"""Tests for SignalActionProcessor.dispatch_advisors."""
@@ -1404,6 +1528,108 @@ def test_invalid_verdict_string_defaults_to_unsure(self):
14041528
result[0].commits[0].advisor_result.verdict, AdvisorVerdict.UNSURE
14051529
)
14061530

1531+
def test_preserves_test_identity_on_attach(self):
1532+
# Signal-level test_file/test_classname/test_name must survive the
1533+
# Signal reconstruction inside _attach_advisor_verdicts.
1534+
from pytorch_auto_revert.signal import (
1535+
Signal,
1536+
SignalCommit,
1537+
SignalEvent,
1538+
SignalSource,
1539+
SignalStatus,
1540+
)
1541+
from pytorch_auto_revert.signal_extraction import SignalExtractor
1542+
from pytorch_auto_revert.signal_extraction_types import Sha
1543+
1544+
t0 = datetime(2025, 8, 19, 12, 0, 0)
1545+
c1 = SignalCommit(
1546+
"sha_aaa",
1547+
t0,
1548+
[SignalEvent("j", SignalStatus.FAILURE, t0, wf_run_id=1, job_id=10)],
1549+
)
1550+
signal = Signal(
1551+
key="test/foo.py::test_bar",
1552+
workflow_name="trunk",
1553+
commits=[c1],
1554+
source=SignalSource.TEST,
1555+
test_file="test/foo.py",
1556+
test_classname="TestFooBar",
1557+
test_name="test_bar",
1558+
)
1559+
extractor = SignalExtractor(workflows=["trunk"], lookback_hours=16)
1560+
extractor._datasource = Mock()
1561+
extractor._datasource.fetch_advisor_verdicts.return_value = {
1562+
("sha_aaa", "test/foo.py::test_bar"): ("revert", 0.95, t0),
1563+
}
1564+
out = extractor._attach_advisor_verdicts([signal], [(Sha("sha_aaa"), t0)])
1565+
self.assertEqual(out[0].test_file, "test/foo.py")
1566+
self.assertEqual(out[0].test_classname, "TestFooBar")
1567+
self.assertEqual(out[0].test_name, "test_bar")
1568+
1569+
1570+
class TestInjectPendingWorkflowEvents(unittest.TestCase):
1571+
"""Tests for SignalExtractor._inject_pending_workflow_events."""
1572+
1573+
def test_preserves_test_identity_on_inject(self):
1574+
# Signal-level test_file/test_classname/test_name must survive the
1575+
# Signal reconstruction inside _inject_pending_workflow_events.
1576+
from pytorch_auto_revert.signal import (
1577+
Signal,
1578+
SignalCommit,
1579+
SignalEvent,
1580+
SignalSource,
1581+
SignalStatus,
1582+
)
1583+
from pytorch_auto_revert.signal_extraction import SignalExtractor
1584+
from pytorch_auto_revert.signal_extraction_types import (
1585+
JobBaseName,
1586+
JobId,
1587+
JobName,
1588+
JobRow,
1589+
RunAttempt,
1590+
Sha,
1591+
WfRunId,
1592+
WorkflowName,
1593+
)
1594+
1595+
t0 = datetime(2025, 8, 19, 12, 0, 0)
1596+
c1 = SignalCommit(
1597+
"sha_aaa",
1598+
t0,
1599+
[SignalEvent("j", SignalStatus.FAILURE, t0, wf_run_id=1, job_id=10)],
1600+
)
1601+
signal = Signal(
1602+
key="test/foo.py::test_bar",
1603+
workflow_name="trunk",
1604+
commits=[c1],
1605+
source=SignalSource.TEST,
1606+
test_file="test/foo.py",
1607+
test_classname="TestFooBar",
1608+
test_name="test_bar",
1609+
)
1610+
# One pending JobRow on a different wf_run_id triggers synthesis on c1.
1611+
pending_job = JobRow(
1612+
head_sha=Sha("sha_aaa"),
1613+
workflow_name=WorkflowName("trunk"),
1614+
wf_run_id=WfRunId(2),
1615+
job_id=JobId(20),
1616+
run_attempt=RunAttempt(1),
1617+
name=JobName("j"),
1618+
status="in_progress",
1619+
conclusion="",
1620+
started_at=t0,
1621+
created_at=t0,
1622+
rule="",
1623+
)
1624+
extractor = SignalExtractor(workflows=["trunk"], lookback_hours=16)
1625+
out = extractor._inject_pending_workflow_events([signal], [pending_job])
1626+
self.assertEqual(out[0].test_file, "test/foo.py")
1627+
self.assertEqual(out[0].test_classname, "TestFooBar")
1628+
self.assertEqual(out[0].test_name, "test_bar")
1629+
# Synthesis actually fired (otherwise the test wouldn't exercise
1630+
# the reconstruction path)
1631+
self.assertGreater(len(out[0].commits[0].events), 1)
1632+
14071633

14081634
class TestGroupActionsTestsToInclude(unittest.TestCase):
14091635
"""Coalescing rules around `tests_to_include` in `group_actions`.

0 commit comments

Comments
 (0)