Skip to content

.agent_loop: orchestration layer driving review-pr and validate-kerne… - #5455

Draft
demonsan wants to merge 1 commit into
mainfrom
blyu/review_agent_loop
Draft

demonsan wants to merge 1 commit into
mainfrom
blyu/review_agent_loop

Conversation

@demonsan

Copy link
Copy Markdown
Contributor

…l-pr as one gated loop

Adds .agent_loop/, a workflow-level orchestration package that runs the two review skills as a single loop: prefetch, one human break, then analysis fan-out, independent cross-model refutation, the seven review-pr gates, on-GPU validation, refutation of the executor's own findings, refold and cleanup.

It is not a third skill, which is why it sits beside .claude rather than inside it: it calls the skills' CLI surface and adds the guarantees a single pass cannot give. Every card finding is attacked by an agent that never saw the analysis, that agent writes the ledger line itself, and the ledger's provenance is checked before a card can be certified.

Six fail-closed guards, all covered by a zero-token dry-run harness (tools/dryrun.mjs, 10/10): dead analysis agent, collection short of the FIRE count, ledger provenance mismatch, gates never green, ledger rewritten behind the gate, and gates failing after the validation refold.

docs/upstream-findings.md records four reproducible defects this loop found in validate-kernel-pr itself, each of which reads as a defect in the PR under review if the tool is trusted.

Validated on #4961 across three heads on gfx950 and gfx1250; see VALIDATED-AGAINST.md for the pinned CLI surface and the measurements.

…l-pr as one gated loop

Adds .agent_loop/, a workflow-level orchestration package that runs the two
review skills as a single loop: prefetch, one human break, then analysis
fan-out, independent cross-model refutation, the seven review-pr gates,
on-GPU validation, refutation of the executor's own findings, refold and
cleanup.

It is not a third skill, which is why it sits beside .claude rather than
inside it: it calls the skills' CLI surface and adds the guarantees a single
pass cannot give. Every card finding is attacked by an agent that never saw
the analysis, that agent writes the ledger line itself, and the ledger's
provenance is checked before a card can be certified.

Six fail-closed guards, all covered by a zero-token dry-run harness
(tools/dryrun.mjs, 10/10): dead analysis agent, collection short of the FIRE
count, ledger provenance mismatch, gates never green, ledger rewritten behind
the gate, and gates failing after the validation refold.

docs/upstream-findings.md records four reproducible defects this loop found in
validate-kernel-pr itself, each of which reads as a defect in the PR under
review if the tool is trusted.

Validated on #4961 across three heads on gfx950 and gfx1250; see
VALIDATED-AGAINST.md for the pinned CLI surface and the measurements.
@github-actions

Copy link
Copy Markdown
Contributor

🏷️ CI Guide

Runs automatically on every PR:

  • ✅ Pre-checks (submodule verification, code formatting)
  • ✅ Aiter op tests (gfx942 + gfx950)
  • ✅ Triton tests on MI35X (only when aiter/ops/triton/** or related paths are changed)

Extended tests (opt-in via labels):

Label Tests
ci:gfx1250-ffm-triton Run the five-shard gfx1250 FFM Triton test suite
ci:triton-300x Run an additional Triton test job on MI300X in PRs; main branch always runs both MI35X and MI300X
multigpu Aiter multi-GPU tests on the 8-GPU runner
ci:sglang SGLang integration tests: DeepSeek-R1-MXFP4 accuracy, Qwen 3.5 accuracy
ci:atom ATOM benchmark: DeepSeek-R1-0528, GPT-OSS-120B
ci:atom_full ATOM accuracy suite for PR and main models from ATOM models_accuracy.json
ci:vllm vLLM benchmark: GPT-OSS-120B, DeepSeek-R1-0528, Kimi-K2.5
ci:all All standard extended tests (excludes ci:atom_full)

Only add ci:atom_full for FlyDSL or Triton upgrades.
Add labels via the sidebar or gh pr edit 5455 --add-label <label>

PR title tags & labels:
Component tags ([Triton/Gluon], [HIP], [CK], [ASM], ...) are added to the PR title and as PR labels automatically from the changed files and re-synced on every push — change-type tags like [fix]/[Perf], op tags like [MLA], and human labels (ci:*) are left untouched. Add the no-auto-title label to opt this PR out.

Comment on lines +26 to +27
except Exception:
continue

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [ruff] <S112> reported by reviewdog 🐶
try-except-continue detected, consider logging the exception

for line in raw.splitlines():
try:
yield json.loads(line)
except Exception:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [ruff] <BLE001> reported by reviewdog 🐶
Do not catch blind exception: Exception

Comment on lines +27 to +28
except Exception:
continue

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [ruff] <S112> reported by reviewdog 🐶
try-except-continue detected, consider logging the exception

for line in raw.splitlines():
try:
yield json.loads(line)
except Exception:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [ruff] <BLE001> reported by reviewdog 🐶
Do not catch blind exception: Exception

@@ -0,0 +1,155 @@
#!/usr/bin/env python

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [ruff] <EXE001> reported by reviewdog 🐶
Shebang is present but file is not executable


runs = {}
rows = []
for line in zstd.decompress(open(p, "rb").read()).decode("utf-8", errors="replace").splitlines():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [ruff] <SIM115> reported by reviewdog 🐶
Use a context manager for opening files

Comment on lines +19 to +20
except Exception:
continue

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [ruff] <S112> reported by reviewdog 🐶
try-except-continue detected, consider logging the exception

for line in zstd.decompress(open(p, "rb").read()).decode("utf-8", errors="replace").splitlines():
try:
o = json.loads(line)
except Exception:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [ruff] <BLE001> reported by reviewdog 🐶
Do not catch blind exception: Exception

rows.append((d.get("seq"), d.get("phase"), d.get("label"), d.get("childId")))

for seq, ph, label, cid in rows:
print(f"seq {seq:>3} {str(ph):<26} {str(label):<44} {cid}")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [ruff] <RUF010> reported by reviewdog 🐶
Use explicit conversion flag

Suggested change
print(f"seq {seq:>3} {str(ph):<26} {str(label):<44} {cid}")
print(f"seq {seq:>3} {ph!s:<26} {str(label):<44} {cid}")

rows.append((d.get("seq"), d.get("phase"), d.get("label"), d.get("childId")))

for seq, ph, label, cid in rows:
print(f"seq {seq:>3} {str(ph):<26} {str(label):<44} {cid}")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [ruff] <RUF010> reported by reviewdog 🐶
Use explicit conversion flag

Suggested change
print(f"seq {seq:>3} {str(ph):<26} {str(label):<44} {cid}")
print(f"seq {seq:>3} {str(ph):<26} {label!s:<44} {cid}")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants