Repository navigation
docs(core): annotate the three tracked shared functions in utils.rs - #1121
Merged
Merged
Conversation
`docs/code-guidelines.md` opens with the shared-function `Used by:` rule and names `create_causal_mask`, `softcap` and `repeat_kv` in `utils.rs` as the components to track, but `repeat_kv` and `softcap` carried no annotation and `create_causal_mask` carried a stale one. Its list named Llama, Mixtral, Cohere, Phi, GLM4 and StarCoder2, none of which call it any more: those families moved to the fused-SDPA implicit-causal path, so the annotation pointed a contributor at the wrong blast radius. Every list here was derived by grep at this commit, not from memory. `repeat_kv` has 14 callers (12 under `src/models`, plus the DeepSeek-OCR Qwen2 vision encoder and the Qwen3-Omni MoE speech layers) and `softcap` has exactly one production caller, RecurrentGemma, since Gemma 2 and Gemma 3 route through the fused `compiled_softcap` kernels instead. Both get a literal list. `create_causal_mask` has 44 non-test callers under `src/models`. Enumerating 44 names reproduces the staleness this change is fixing, so it gets a rule instead: why a caller is on the list, representatives for each of the four groups (hybrid stacks, sliding-window families, VLM decoders, MLA and custom-attention decoders), the mainstream dense decoders that are deliberately absent, and the grep one-liner that regenerates the exact set. `docs/code-guidelines.md` records that policy under a new "When the caller list is too long to enumerate" section so the next 40-caller helper does not re-litigate it, and notes that public items use `///` rather than the `//` in the older format example. Comments only, no executable change. Verified with `cargo fmt --check` and `scripts/ci/check_cross_repo_refs.py`. Closes #1110
3 tasks done
inureyes
added a commit
that referenced
this pull request
Aug 13, 2026
## Summary Four PRs merged on 2026-08-13. Two of them, #1120 and #1122, got their bilingual technical reports from the chain workflow. #1118, #1119 and #1121 merged without one. This PR closes that gap after the fact so the batch is uniformly documented. Six files, `.en.md` and `.ko.md` per PR, following the naming and section shape of the two reports already in `TECHNICAL_REPORTS/` from this same batch. No source, documentation, or build file is touched. ## What is here | PR | Issue | Squash commit | Report | |---|---|---|---| | #1119 | #1104 | `cf4e22cd` | `1119-paged-decode-v2-env-vars-20260814.{en,ko}.md` | | #1118 | #1108 | `33322d66` | `1118-cli-dry-sequence-breakers-20260814.{en,ko}.md` | | #1121 | #1110 | `f0bf3a2c` | `1121-utils-used-by-annotations-20260814.{en,ko}.md` | Each report was written from the merged diff (`git show <commit>`) and the linked issue body, not from the PR description alone. Each records at least one thing the issue did not contain. ## Findings recorded in the reports **#1121 (issue #1110).** The issue was wrong on a load-bearing point. It recorded `create_causal_mask` as carrying no `Used by:` annotation. It carried one, and that annotation named Llama, Mixtral, Gemma, Cohere, Phi, GLM4, StarCoder2 and OLMo among its users while grep at that commit shows `mixtral.rs`, `phi.rs`, `phi3small.rs`, `starcoder2.rs`, `llama3.rs`, `gemma.rs`, `gemma2.rs`, `cohere.rs`, `glm4.rs`, `olmoe.rs` and `qwen3_moe.rs` each calling it zero times. Those families moved to the implicit-causal fused-SDPA path (`mask: None` when `seq_len > 1`). The PR therefore replaced a misleading roster rather than adding a missing one, which is a stronger defect than the issue described: a missing annotation sends a contributor to look, a wrong one tells them not to. Measured counts at that commit: 44 non-test callers under `src/models` (the issue's 46 counted `diffusion_gemma/tests.rs` and `phi3small_tests.rs`), 55 across all of `src`. **#1119 (issue #1104).** All six variable defaults were traced to their definition sites rather than to the issue text. Two out-of-scope defects are recorded rather than fixed: `docs/turbo-kv-cache.md` around lines 298 to 312 still carries the same pre-#899 "not a server knob" framing this PR corrected in `environment-variables.md`, and in `src/execution/memory_estimate.rs` the `Err(_)` arm of `resolve_paged_slab_blocks` warns "using the derived slab size" and then returns `None`, which keeps the 32-block pool default instead, so the warning describes behavior that does not happen. **#1118 (issue #1108).** The issue was reframed away from a flag-parity feature request into a docs fix after investigation showed the CLI omits nine server sampling knobs, not one. The change is comment-and-help-text only, 13 added lines all matching `^\s*(///|//)`. The second `dry_multiplier` field in `src/main.rs` belongs to `ServeArgs`, which already exposes `--dry-sequence-breakers` sixteen lines below, so it was deliberately left alone. ## Notes `TECHNICAL_REPORTS/` is listed in `.gitignore` but is tracked through the `TECHNICAL_REPORTS/.keep-reports` marker, so the six files were staged with `git add -f`. ## Test plan - [x] `git status --porcelain` shows exactly the six intended files staged and nothing else - [x] Every claim in each report checked against the merged commit: caller counts re-run with `git grep` at `f0bf3a2c`, variable defaults re-read at their definition sites, diffstats taken from `git show --stat` - [x] No build, test, lint, or format surface involved: the diff is six new Markdown files under `TECHNICAL_REPORTS/`
7 tasks
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
docs/code-guidelines.mdopens with the shared-functionUsed by:rule and namescreate_causal_mask,softcapandrepeat_kvinutils.rsas the components to track. Two of the three carried no annotation, and the third carried one that was actively wrong. This adds accurate annotations to all three and records the policy for helpers with too many callers to enumerate.Every list was derived by grep at this commit, not copied from the issue body or from memory.
What changed
src/lib/mlxcel-core/src/utils.rs,create_causal_mask: replaced the stale list (it named Llama, Mixtral, Cohere, Phi, GLM4 and StarCoder2, none of which call it any more) with a rule-level annotation. 44 non-test callers undersrc/modelsgrouped by why they materialize an explicit mask (hybrid and mixed-layer stacks, sliding-window and chunked families, VLM decoders, MLA and custom-attention decoders), the non-src/modelscallers, an explicit "not used by" for the mainstream dense decoders that rely on fused-SDPA implicit causality, and the grep one-liner that regenerates the exact set.src/lib/mlxcel-core/src/utils.rs,repeat_kv: literal list of all 14 callers (12 undersrc/models, plussrc/vision/encoders/deepseekocr_qwen2.rsandsrc/audio/qwen3_omni_moe/speech_layers.rs) with the reason most decoders never call it.src/lib/mlxcel-core/src/utils.rs,softcap: names its single production caller, RecurrentGemma, and records that Gemma 2 and Gemma 3 route through the fusedcompiled_softcap/compiled_softcap_sdpakernels instead, so a change here does not reach them.docs/code-guidelines.md: new "When the caller list is too long to enumerate" section holding the policy (rule plus representatives plus a "not used by" half plus the regenerating grep), withcreate_causal_maskas the worked example. Also notes that public items use///, unlike the//in the older format example.All three annotations use
///at the end of the doc block, matching the neighbouringcreate_causal_mask_with_left_paddingin the same file.Measured counts
The issue's table gave 46 / 12 / 1. Measured at this commit:
create_causal_maskgrep -rln '\bcreate_causal_mask(' src/models src/multimodal src/vision --include='*.rs'repeat_kvsrc/models, 13 withsrc/vision, 14 withsrc/audiogrep -rln '\brepeat_kv(' src --include='*.rs'softcapgrep -rn '\bsoftcap(' src --include='*.rs'The issue also recorded
create_causal_maskas unannotated. It did carry an annotation at HEAD, but a stale one, which is why this replaces rather than adds.Test plan
.rsfile: every added and removed line matches^\s*(///|//).cargo fmt --checkclean.python3 scripts/ci/check_cross_repo_refs.pyclean.Closes #1110