mirror of
https://github.com/ggml-org/llama.cpp.git
synced 2026-08-03 16:40:45 -05:00
agents: clarify comment style and jinja knowledge (#26405)
* agents: clarify comment style and jinja knowledge * improve Security review a bit
This commit is contained in:
29
AGENTS.md
29
AGENTS.md
@@ -71,11 +71,20 @@ For first-time contributors, confirm they have reviewed [CONTRIBUTING.md](CONTRI
|
||||
These points are extremely important - failing to follow them won't necessarily get your PR rejected, but it will make reviewing take significantly longer. Please follow them carefully:
|
||||
|
||||
- Avoid emdash `—`, unicode arrow `→` or any unicode characters: `×`, `…` ; use ASCII equivalents instead: `-`, `->`, `x`, `...`
|
||||
- Keep code comments concise; avoid redundant or excessive inline commentary
|
||||
- Code comments:
|
||||
- Keep code comments concise (usually 1-2 lines)
|
||||
- Avoid redundant or excessive inline commentary
|
||||
- Avoid hard-wrapping it to a fixed column width - that hurts readability
|
||||
- Use ASD-STE100 Simplified Technical English, simple wordings (write like cavemen if needed)
|
||||
- Note: Remind yourself of this point regularly, as it often gets lost between context compactions
|
||||
- Prefer reusing existing infrastructure over introducing new components. Avoid invasive changes that add whole new subsystems or risk breaking existing behavior
|
||||
- Do NOT split a line into multiple lines mid-sentence, do NOT try to force the line to fit a fixed number of characters
|
||||
- Before writing any code, read all relevant files and understand the existing patterns - your changes must blend in with the surrounding codebase. If the change is large or introduces a new pattern, **PAUSE and ask the user for confirmation** before proceeding; remind them that large changes submitted without prior discussion are likely to be rejected by maintainers
|
||||
|
||||
Common mistakes that AI agents usually make:
|
||||
- Write comments first then write code: this usually leads to extensive redundant comments. Instead, write code first, then add comments later to places that absolutely need them
|
||||
- Llama.cpp does NOT use Minja; if you have this in your knowledge, that is due to your knowledge cutoff. Llama.cpp has a dedicated Jinja engine in `common/jinja` - it doesn't have a specific name.
|
||||
|
||||
### Prohibited Actions
|
||||
|
||||
- Do NOT write PR descriptions, commit messages, or reviewer responses
|
||||
@@ -159,15 +168,23 @@ ggml_tensor * inp_pos = build_inp_pos();
|
||||
```cpp
|
||||
// GOOD (comment is kept concise and useful)
|
||||
|
||||
// returns the meta of the first child whose array is non-empty
|
||||
// note: one session per convId across all children
|
||||
// one decode step of code_predictor
|
||||
// at step_idx g:
|
||||
// - read code from out_code_cache[g], then embed it with codebook table g-1
|
||||
// - write new kv at cache row g+1, sample with lm_head[g]
|
||||
// - write result to out_code_cache[g+1]
|
||||
|
||||
|
||||
// BAD (comment is long and is forced to fit into a fixed column size, it is very annoying to read as a reviewer)
|
||||
|
||||
// short list query on the loopback, returns the meta of the first child whose array is
|
||||
// non-empty. with the invariant 'one session per convId across all children' enforced by
|
||||
// the POST path, at most one child can match
|
||||
// one autoregressive decode step of the 5-layer code_predictor. See the
|
||||
// comment in models.h for the cache/tensor conventions this relies on.
|
||||
//
|
||||
// index mapping (derived from the reference pipeline-tts.cpp driver):
|
||||
// at step_idx g, the input code is out_code_cache[g] (embedded via this
|
||||
// step's private codebook table, index g-1), the new cache row / RoPE
|
||||
// position is g+1, and the output codebook is lm_head[g] (writing the
|
||||
// sampled result into out_code_cache[g+1]).
|
||||
```
|
||||
|
||||
Commit message:
|
||||
|
||||
@@ -47,6 +47,7 @@ Mandatory on every review; any finding here is **blocking**. Rule of thumb: GGUF
|
||||
- **Sizes/counts from tensor dims:** validate before allocating. Products like `ne[i]*nb[i]`/nbytes can overflow on crafted dims into an undersized alloc then heap overflow. Overflow checks must run BEFORE the arithmetic they guard - padding/alignment macros wrap to 0 near `SIZE_MAX`, so a guard after the pad passes.
|
||||
- **GGUF strings/arrays:** cap declared lengths and element counts before using them to size a loop or buffer; validate element type and length before casting an array to a pointer or reading fixed indices (`[i+1]`, `[0..2]`).
|
||||
- **File-supplied counts indexing fixed arrays:** bound any count (e.g. layer/block count into a `LLAMA_MAX_*` array) before indexing; watch checks that only fire when an optional key is present.
|
||||
- **Declared vs actual array length:** check the declared length of a GGUF array against the count actually read, not just against a buffer size.
|
||||
- **Bounds comparisons:** flag narrowing casts (`size_t`->`int32_t`) and signed/unsigned mixing that can bypass a length check and copy past a buffer.
|
||||
- **Parsed/derived indices:** range-check `stoi`/`atoi` results and catch parse throws; never use a default or derived token id (EOS/BOS/...) as an index without a bounds check.
|
||||
- **Reused/reserved buffers:** recheck bounds after a buffer is shrunk or reused; watch `reserve()` then index-by-assumed-size, and header fields read before their length is checked.
|
||||
@@ -131,6 +132,8 @@ Enforce the `AGENTS.md` / `CONTRIBUTING.md` coding and naming guidelines on ever
|
||||
- Reuse existing infrastructure over introducing new components; no new third-party dependencies, extra headers, or files unless clearly justified.
|
||||
- Keep it simple: a simpler change doing 90% is often preferable to a complex one doing 100%. Flag unnecessary templates/fancy STL; basic `for` loops are fine here.
|
||||
- Every added line should be something the contributor can explain and defend to a reviewer without AI help - flag anything that looks copied-in without understanding.
|
||||
- `Co-authored-by:` must be reserved for human co-authors; AI contributions (claude, cursor, codex, etc.) must use `Assisted-by:`; if this point is violated, it's a blocking finding.
|
||||
- Any mentions of Minja must be treated as blocking; see `AGENTS.md` for why.
|
||||
|
||||
## Reporting
|
||||
|
||||
|
||||
Reference in New Issue
Block a user