[Skills] Add independent review gate to xpu-alignment

[laifenxiawucha]

Summary

Keep the existing xpu-alignment scan workflow and add an independent review gate before issue filing or implementation.

• preserve the original collect, filter, reproduce, classify, and audit flow and ledger schema
• treat confirmed and related-failure as provisional scan results
• require a fresh reviewer subagent that is verifiably stronger than the main model when available and never weaker; exact parent-model inheritance is the fallback
• generate review conclusions, an engineering dashboard, and local comment drafts
• allow only reviewed needs-xpu-fix cases to proceed, with separate authorization for GitHub writes and handler execution
• track every independent XPU fix in intel/torch-xpu-ops, including implementations that belong in pytorch/pytorch
• keep tracking repository and implementation repository as separate review decisions
• clarify that issue-handler implements and verifies fixes but does not create PRs

Validation

• python3 ~/.codex/skills/.system/skill-creator/scripts/quick_validate.py .claude/skills/xpu-alignment
• git diff --check origin/main...HEAD
• verified every Markdown reference linked from SKILL.md exists
• verified the environment reference is unchanged from origin/main

Documentation/skill-only change; no runtime code was modified.

[ZhaoqiongZ]

@copilot , please help review this pr in depth

[laifenxiawucha]

please help review this pr in depth @copilot

[laifenxiawucha]

@copilot please help review this pr in depth

[Stonepia]

@torchxpubot review

Help to review this PR, and verify the following comments to see if it is true. If this is, then leave the comments there.

The PR HEAD is still **`fe44cdfbe9f78cd391f3c18b045c4702ecace1fe`**, so these line numbers match the current version.

## 1. Already-fixed cases become `non-issue`

**Comment anchor:**

```text
.claude/skills/xpu-alignment/references/xpu-alignment-buckets-and-routing.md:138
```

**Anchor text:**

```md
| Runtime gate fails | `non-issue` | `no-issue` |
```

**Relevant lines:**

* Runtime maps `not-reproduced` to gate failure: **67**
* Ordered verdict table starts: **134**
* Problematic rule: **138**
* Fixed/confirmed paths: **142–144**

**Exact comment:**

> `[P1] Preserve already-fixed bugs instead of classifying them as non-issues`
>
> The environment contract requires testing a current build and refreshing it when the linked fix is newer. Once that build contains the fix, the reproducer will normally produce `not-reproduced`; line 67 consequently fails the runtime gate, and this row immediately derives `non-issue`.
>
> This makes the review contract’s `fixed / verified-in-tested-build` outcome unreachable for a case that was a real XPU bug but has already been fixed. Please distinguish “the behavior was never an issue” from “the issue no longer reproduces because the tested build contains the fix,” retaining historical/pre-fix bug evidence and deriving an explicit fixed outcome.

The build-refresh requirement is at `xpu-alignment-environment-setup.md:15–18`.

The supposedly supported fixed outcome is at `xpu-alignment-review.md:183–190`.

---

## 2. Segfaults and timeouts cannot satisfy evidence validation

**Comment anchor:**

```text
.claude/skills/xpu-alignment/references/xpu-alignment-evidence-contract.md:82
```

**Anchor text:**

```md
`XPU_ALIGNMENT_RESULT=<one-line-JSON>`.
```

**Relevant lines:**

* Every log must end with the terminal record: **81–84**
* Crashes/timeouts are explicitly anticipated: **114–115**
* Evidence validity requires terminal JSON: **117–122**

**Exact comment:**

> `[P1] Define an externally recorded result for abnormal process termination`
>
> This requires every attempt log to end with a parseable `XPU_ALIGNMENT_RESULT`, and lines 119–122 prevent `evidence_status=valid` unless that terminal JSON exists and matches `runtime_result`.
>
> A reproducer that segfaults, aborts, is OOM-killed, or is terminated on timeout cannot emit that record, even though the abnormal termination may itself be the decisive bug signature. Please define a trusted parent runner that records exit code, signal, timeout, and the derived runtime result separately from the raw child log, or add an explicit markerless abnormal-termination evidence path.

The sandbox also explicitly requires process-group termination on timeout at `xpu-alignment-environment-setup.md:71–79`.

---

## 3. PR creation is assigned to the wrong skill

**Comment anchor:**

```text
.claude/skills/xpu-alignment/SKILL.md:11
```

**Anchor text:**

```md
auditable outcome. `issue-handler` exclusively owns implementation and PR
creation.
```

The complete sentence is on **lines 10–12**.

**Exact comment:**

> `[P1] Align handoff ownership with the existing issue-handler contract`
>
> `issue-handler` does not exclusively own PR creation. Its existing contract delegates PR preparation to `xpu-ops-pr-creation`, and that skill explicitly states that agents do not open PRs. In addition, `xpu-ops-pr-creation` only covers `intel/torch-xpu-ops`, while this workflow permits `implementation_repository: pytorch/pytorch`.
>
> Please separate implementation, PR preparation, and actual PR creation into their existing owners and define the corresponding repository-specific handoff, rather than assigning all three responsibilities to `issue-handler`.

Supporting existing contracts:

* `issue-handler/SKILL.md:129–133` delegates PR work.
* `issue-handler/issue-fix/SKILL.md:130–132` delegates again.
* `xpu-ops-pr-creation/SKILL.md:4–8` is specifically for `intel/torch-xpu-ops`.
* `xpu-ops-pr-creation/SKILL.md:116–119` says agents do not open PRs.

[Stonepia]

This part seems re-creating a set of state machine, I am not sure whether this is good or not.

In theory, we only care whether this needs XPU fix, so maintaining a state machine seems not reasonable.

[Stonepia]

record json template 1.

[Stonepia]

record json 2

[Stonepia]

I believe we already have one global env setting file? So maybe this env setting is not needed. But for this PR, I think keeping it would be ok. But we anyway need to align in the future

[Stonepia]

record json 3

[Stonepia]

Overall, I think there is one big issue:

There are too many state machine to be maintained in this single use case. For every skill, it has one json file, one state to keep, then output. But in fact, all these skills serve for the one purpose. So to keep the maintainance simple, you need to just keep one json file, and communicate across agents/skills.

In addition, there are also some other changes needed posted in previous comment, the bot is waiting for a runner, please wait after all result is printed (https://github.com/intel/torch-xpu-ops/actions/runs/29804787930/job/88553038905)

[torchxpubot]

Replying to this comment by @Stonepia

I now have a complete picture. Let me write the review.

PR Review: #4392

Summary

Refactors the xpu-alignment skill from a clear, procedural reproduction-driven scanner (217-line SKILL.md + ~200 lines of references) into an extensive "proof-ladder triage" system with 928 new lines of interlocking formal contracts, JSON schemas, state machines, and cross-artifact audit rules. The change is documentation/skill-only — no runtime code is modified.

Overall verdict: The refactored skill is significantly over-engineered for what a Claude Code skill should be, and introduces requirements that are unrealistic in the execution environment.

Correctness

1. Self-blocking sandbox requirements (xpu-alignment-environment-setup.md): The new environment reference requires "an isolated unprivileged UID/user namespace and PID namespace or equivalent; deny signaling, tracing, or inspecting unrelated processes; CPU, memory, process-count, file-size, and wall-time limits; terminate the complete process group/cgroup on timeout; a restricted syscall profile where the platform supports it." Claude Code executes Bash commands — it cannot create user namespaces, configure seccomp profiles, or set up cgroup limits. The fallback clause ("If the available runtime cannot enforce these boundaries... record a security-sandbox incident and do not execute the repro") makes the skill self-blocking in any standard Claude Code environment, which defeats its purpose of actually running reproducers.

2. SHA-256 hash validation is impractical (xpu-alignment-evidence-contract.md): The evidence contract requires repro_sha256, environment_sha256, and log_sha256 fields, and the audit checks that "file hashes match." An LLM cannot reliably compute SHA-256 hashes during execution — it must shell out, and then round-trip the values into JSON without error. The previous version achieved auditability without this fragile mechanism.

Testing

3. Validation is inadequate for complexity introduced. The PR's test plan is a syntax checker (quick_validate.py) and git diff --check. For a skill with 7+ interacting state machines, 20-field JSON schemas, derivation tables with ordering constraints, and cross-artifact audit rules, there is no evidence of end-to-end execution. Was the refactored skill invoked and did Claude successfully produce valid artifacts under the new contracts?

XPU-Specific Risks

No XPU kernel or runtime code is changed — not applicable.

Dispatch & Registration

Not applicable — documentation-only change.

Backward Compatibility

4. Ledger schema is incompatible with existing runs. The candidate_ledger.jsonl schema changes completely (old fields like title_status, deep_status, local_status, local_bucket are replaced with selection_status, case_key, details_path, etc.). Any in-progress or completed alignment run under the old schema cannot be resumed or re-audited with this skill version. The PR does not acknowledge this break.

Performance

5. The two-attempt requirement inflates cost significantly. The evidence contract requires "two independent process/cache attempts" for every potentially actionable defect, doubling the runtime execution budget compared to the previous single-attempt approach. This is not inherently wrong, but is never justified with data showing single-attempt results were unreliable.

Design Issues (not covered by standard sections)

6. Violates single-purpose principle. The refactored skill covers: environment/sandbox setup, GitHub collection/sharding, case selection/ledger management, repro construction, runtime execution, evidence capture with hashing, semantic assessment with 4-gate proof ladders, independent subagent review with model-capability verification, issue filing/deduplication, handler handoff with eligibility gates, incident management with retry lifecycles, coverage/dashboard/report generation. The original was already at the upper bound of scope; the refactored version covers at least 5 distinct concerns that should be separate skills with clear interfaces.

7. Specification density exceeds LLM execution fidelity. The skill now reads like a distributed systems RFC rather than instructions for an AI agent. It defines: ordered derivation tables where "the first matching row" determines outcome, compatibility matrices between verdict/fix-state pairs, forward-only state machines with 6+ states, cross-artifact consistency rules spanning 6+ files, and nested JSON schemas with 20+ fields that must all use allowed enum values. Claude will frequently produce subtle violations (wrong enum combination, missing hash, derivation order error) that then fail the self-audit — creating a loop of self-correction attempts that burn tokens without converging.

8. Loss of actionable guidance. The original skill had clear procedural steps: "Search -> Filter -> Fetch -> Write repro -> Run -> Classify -> Route -> Report -> Ask before filing." The new version replaces this with abstract "proof gates" and "completion criteria" that defer all procedural detail to cross-referenced documents. An agent following these instructions must load 7 reference files to understand what to actually do — far exceeding reasonable progressive disclosure.

Recommendation

Request Changes

The core idea (requiring stronger evidence before filing issues) is sound, but the implementation is unsuitable for a Claude Code skill:

• The sandbox requirements make the skill self-blocking in standard environments.
• The formal specification density exceeds what an LLM can reliably execute.
• The scope should be decomposed into multiple focused skills (scan, evidence, review, filing).
• The SHA-256 audit mechanism is fragile without tooling support.
• No evidence of end-to-end execution validates the new contracts work in practice.

The previous version was functional and auditable. If stronger evidence gates are desired, they should be added incrementally to the existing procedural structure rather than replacing it with a formal specification document.

[laifenxiawucha]

@torchxpubot review