[None][chore] Update AD onboard skill and reviewer agent (#241)

* [None][chore] Update AD onboard skill and reviewer agent

- SKILL.md: Add GPU memory sanity check (Phase 0 Step 0) before onboarding
- SKILL.md: Clarify HF reference strategy — import from HF cache via importlib
  instead of copying class definitions into test files
- SKILL.md: Tighten Phase 9/10/11 to consolidate --use-registry guidance and
  remove duplicate mandatory-warning blocks; simplify summary report format
- SKILL.md: Phase 4 note that AD factory already calls AutoConfig.from_pretrained
- ad-onboard-reviewer.md: Add BB section for vision/multi-modal support checks
- ad-onboard-reviewer.md: Clarify B2 custom config justification criteria
- ad-onboard-reviewer.md: F4 — no standalone HF-like class definitions in tests;
  must import from actual HF source

Signed-off-by: gramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>

* [None][chore] Address PR review feedback on skill/agent updates

- SKILL.md line 110: Relax "always use HF classes" to "use if they exist"
- SKILL.md line 112: Restore fallback to copying from HF source when HF cache
  unavailable; keep strong preference for HF cache import first
- SKILL.md Phase 9: Restore ⚠️ MANDATORY --use-registry block
- SKILL.md Phase 9 failure steps: Restore bold registry config yaml, items 4+5
- SKILL.md Phase 10: Restore ⚠️ MANDATORY block and item 9 (raw prompts)
- SKILL.md Phase 11: Restore MUST list with raw prompts requirement
- SKILL.md Key Gotchas: Restore bold MUST/NEVER RoPE cos/sin guidance
- reviewer F4: Relax to flag standalone class defs only when viable HF alt exists

Signed-off-by: gramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>

* [None][chore] Address second round of PR review feedback

- reviewer BB3 → BB2 (nit rename)
- SKILL.md line 205: remove "first sentence" about --use-registry flag
- SKILL.md line 112: drop overly specific importlib/sys.path sentence

Signed-off-by: gramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>

* [None][chore] Require pytest results to be included in PR

Phase 11: agent must run the pytest command on the latest commit
and include results in the PR, not just provide the command.

Signed-off-by: gramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>

* [None][chore] Add E2E sanity check note for custom config class in Phase 4

Signed-off-by: gramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>

* [None][chore] Reviewer B2: replace Eagle example with E2E heuristic for custom config

Signed-off-by: gramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>

* [None][chore] Reviewer B2: remove E2E heuristic, keep AutoConfig.from_pretrained check

Signed-off-by: gramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>

* [None][chore] Revert HF cache approach; restore copy-into-test-file for missing HF modules

HF cache paths are not dependable in CI. When HF modules are not in
the installed transformers, copy minimal faithful class definitions from
the HF source into the test file instead of loading from HF cache.
Update reviewer F4 accordingly.

Signed-off-by: gramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>

* [None][chore] Extend HF reference strategy to cover trust_remote_code config classes

Clarify that the "copy into test file" pattern applies to config classes
too: when a model's config uses trust_remote_code (not in transformers),
copy a minimal faithful version into the test file rather than loading
from the HF cache. The modeling file itself should not import the config.

Signed-off-by: gramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>

* [None][chore] Address PR review comments on skill config/test guidance

- Remove overly specific examples from config import note (line 96)
- Simplify trust_remote_code config guidance to avoid multi-modal specifics (line 112)

Signed-off-by: gramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>

---------

Signed-off-by: gramnarayan <105831528+govind-ramnarayan@users.noreply.github.com>

Author: govind-ramnarayan

.claude/agents/ad-onboard-reviewer.md

@@ -44,6 +44,15 @@ Read the actual source code for each check. Cite `file:line_number` for every PA
 
 Note: BB1–BB2 only apply if the HF source indicates the model is multi-modal (has image/audio inputs). Mark N/A with justification for pure language models.
 
+### BB. Vision / Multi-Modal Support
+
+| # | Check | How to verify |
+|---|-------|---------------|
+| BB1 | If the model has a vision tower (multi-modal), the full `nn.Module` hierarchy for the vision component is present in the modeling file — it is NOT omitted, stubbed out, or replaced with a `pass` body | Grep for vision-related class names (e.g., `VisionTower`, `ViT`, `CLIPVision`, `SiglipVision`) from the HF source. If the model is multi-modal and none appear, flag as FAIL. |
+| BB2 | The test file asserts that vision-related weight keys are present in the model's `state_dict` after `load_state_dict` | Grep the test file for assertions on vision weight key names (or a check that vision-prefixed keys are in the loaded state_dict). Absence of any such assertion is a FAIL for multi-modal models. |
+
+Note: BB1–BB2 only apply if the HF source indicates the model is multi-modal (has image/audio inputs). Mark N/A with justification for pure language models.
+
 ### C. Ops & Compatibility (STRICT — canonical ops are the backbone of AD)
 
 | # | Check | How to verify |