diff --git a/skills/code-review/SKILL.md b/skills/code-review/SKILL.md index b282a9164..4c8e4bbcc 100644 --- a/skills/code-review/SKILL.md +++ b/skills/code-review/SKILL.md @@ -81,6 +81,7 @@ See the `add-new-model` skill and `docs/development/HOWTO-add-model.md` for the - New tensor names go through `tensor_mapping.py`, not ad-hoc name matching. - For QKV, split the *activation* with `ggml_view`, not the *weight* tensor; rely on ggml broadcasting instead of manually duplicating tensors. - New graph inputs are declared at the top of the graph-build function, not inline where first used. +- Don't end a text model graph with `ggml_build_forward_expand(gf, cur)` on `t_logits`/`t_embd` - it's redundant, since `llama_model::build_graph()` expands them via `build_logits()` and `build_pooling()` (and skips the LM head for pooled embeddings). - Hparams that the model can't run correctly without must be mandatory (hard-error if missing), not read with a silent default fallback. Only genuinely-optional-across-configs values get a fallback accessor. - New/optional weight tensors (scales, etc.) must route through `build_lora_mm` and the existing helpers, matching convention - don't leave raw matmuls copied from another arch. - Don't hack RoPE with a custom sin/cos implementation. If `ggml_rope_ext` genuinely can't express it, that's an issue for discussion, not a PR.