chore(docs): guard the docs-* targets and document the manual split by inureyes · Pull Request #1122 · lablup/mlxcel

inureyes · 2026-08-13T15:31:20Z

Summary

Resolves the (a)/(b) question in #1111 as case (b), and applies the guard-and-document fix: the manual sources are maintained in a separate documentation tree, the thirteen docs-* targets now stop with an explanation instead of failing partway through, and docs/README.md says so.

Decision

The manual sources (docs/en, docs/ko, docs/shared, docs/requirements.txt, docs/scripts) are maintained in a separate documentation tree, along with that tree's own copies of the four mkdocs configs. They have never existed in this repository's history, which is why git log --all -- docs/en is empty. The full rationale is recorded in a comment on #1111.

What changed

Makefile: a shared docs-guard prerequisite. All thirteen docs-* targets (docs-install, docs-serve, docs-serve-en, docs-serve-ko, docs-build, docs-build-ko, docs-build-all, docs-build-strict, docs-pdf-setup, docs-pdf-en, docs-pdf-ko, docs-pdf, docs-clean) now depend on it, rather than carrying thirteen copies of the same check. The guard is a presence check on docs/en, not an unconditional refusal: where the sources exist the targets run exactly as before, and where they do not the build stops with a message naming what is missing, explaining that the configs' docs_dir / custom_dir / nav: entries are paths into the other tree, and pointing at https://mlxcel.lablup.ai/en/manual/. That property is deliberate, so one Makefile stays correct in both trees.

Makefile: ## help strings. Each of the thirteen gains the suffix (manual sources not in this checkout), so make help no longer advertises them as working here. The suffix is a statement about this repository and is intentionally local.

The four mkdocs configs are untouched. mkdocs.yml, mkdocs.ko.yml, mkdocs.pdf.yml, and mkdocs.ko.pdf.yml belong to the tree that owns the manual and this repository is kept in sync with it, so rewriting their nav: blocks to point at the top-level docs/*.md files here would be wrong for that tree and undone by the next sync.

docs/README.md: a "The MkDocs manual" section. It states the split (top-level docs/*.md are the GitHub-facing documents that live here, the manual is built elsewhere, and the configs' path entries refer to that other tree), and records that the two sets not overlapping is deliberate rather than drift. docs/en/... and docs/ko/... move out of "Expected future layout examples" for the same reason: they describe a tree that exists, not a layout this repository is heading toward. Intro item 2 is reconciled with the new section. The remaining two entries under "Expected future layout examples" (docs/github/..., docs/git/...) are genuinely still future and stay.

No GitHub-facing docs/*.md document changed (acceptance criterion 4). The only file touched under docs/ is the index.

Test plan

All thirteen targets run individually: each exits 1 at the guard with the explanatory message, and no uv, ln, or zensical command is reached.
Positive path: with a docs/en directory present, make docs-guard exits 0, so the targets are unblocked wherever the sources exist.
make help renders all thirteen with the caveat, in each of the four sections its greps place them in; docs-guard itself is absent from help because it carries no ## string.
python3 scripts/ci/check_cross_repo_refs.py passes.
No docs/*.md document other than the index appears in the diff.

Note on the issue body

The table in #1111 lists docs/overrides/ as absent. It is untracked rather than deliberately omitted, so it is missing from a fresh clone, but the custom_dir it backs has no content either way. Same practical outcome for the build; the guard message does not repeat the claim.

Closes #1111

The thirteen `docs-*` targets build the MkDocs manual from sources (`docs/en`, `docs/ko`, `docs/shared`, `docs/requirements.txt`, `docs/scripts`) that are maintained in a separate documentation tree and are not part of this repository. Running one here failed partway through: `make docs-install` died inside `uv pip install -r docs/requirements.txt`, and working around that hit a broken symlink and then a site build against a nonexistent `docs_dir`. Nothing said the manual was built elsewhere, so the only way to find out was to run a target and read the failure. All thirteen now depend on a shared `docs-guard` prerequisite. The guard is a presence check on `docs/en`, not an unconditional refusal: where the sources exist the targets run exactly as before, and where they do not the build stops immediately with a message naming what is missing, why the configs' `nav:` entries do not describe this tree, and where to read the published manual. One Makefile stays correct in both trees. Each `##` help string gains a short suffix so `make help` no longer advertises thirteen working targets. The four mkdocs configs are left unchanged. They belong to the tree that owns the manual, so rewriting their `nav:` blocks to point at the top-level `docs/*.md` files here would be wrong for that tree and undone by the next sync. `docs/README.md` gains a "The MkDocs manual" section explaining the split: the top-level `docs/*.md` files are the GitHub-facing documents that live here, the manual is built elsewhere, and the configs' `docs_dir`, `custom_dir`, and `nav:` entries are paths into that other tree. `docs/en/...` and `docs/ko/...` move out of "Expected future layout examples" for the same reason, since they describe a tree that exists rather than a layout this repository is heading toward. The GitHub-facing documents themselves are unchanged. Verified by running all thirteen targets (each stops at the guard with exit 1 and no `uv` or `zensical` invocation), by re-running the guard with a `docs/en` directory present to confirm it passes, and by reading `make help`. Closes #1111

Bilingual report covering the (a)/(b) resolution, why the guard is a presence check rather than an unconditional refusal so one Makefile stays correct in both trees, why the four mkdocs configs were left untouched, and the two-direction validation of the guard.

## 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/`

Three corrections from review, two of which invalidated claims the first commit made. The integration test still could not reach the validator, and the first commit made it worse. `run_generate` resolves `-m` before calling `validate_pipeline_parallel_args`, because the validators read the resolved model directory, so a subprocess invocation cannot reach the validator without a real model on disk. The test's `-m` value is a valid bare repo segment, so once the flag name was fixed and clap stopped rejecting the argv at parse time, the resolver expanded it against `$MLXCEL_DEFAULT_ORG` and went to HuggingFace: running the exact argv fails with `authentication failed (HTTP 401)` after an outbound request, in a test that is not `#[ignore]`d. The test is now scoped to what a subprocess can actually check, that the parser accepts `--pp-size 2 --tp-size 2`, by appending `--help` so clap exits as soon as parsing succeeds. It asserts positively on exit status rather than on the absence of a string, so it cannot pass by dying early. Verified both ways against the built binary: with `--tp-size` it exits 0, with `--tensor-parallel-size` it exits 2. Runtime dropped from 1.22s to 0.09s because the network call is gone. The validator keeps its direct unit coverage in `validate_pipeline_parallel_args_accepts_2d_pp_tp`. The `docs/en/` paths were not broken references. `mkdocs.yml` sets `docs_dir: docs/en` and its nav names `distributed/tensor-parallelism.md` and `distributed/pipeline-parallelism.md`, which resolve to exactly the two paths the comment cited. They are pages of the operator manual whose sources are maintained in a separate documentation repository; `docs/README.md` calls that split "deliberate, not drift" and the `docs-guard` Makefile target from #1122 is built on the same fact. Deleting them is the exact failure #1122's report listed as a high-impact risk. The comment now keeps both manual pages, explains why their sources are not in this tree, and names `docs/distributed.md` as the in-checkout summary. This knowingly does not satisfy the issue's "no `docs/en/` reference remains in `src/`" criterion, which rested on a false premise. The CHANGELOG entry is removed rather than reworded. The `ensure!` condition is the exact negation of the early return above it, so the guard can never fail and the message can never be emitted; the repository's own `validate_pipeline_parallel_args_rejects_2d_without_pp_enabled` asserts `is_ok()` for that input. No user has seen this message, so there is no user-visible change to record. The unreachable guard is left alone, since changing it would violate the issue's "validation logic is unchanged" criterion, and is filed as a follow-up. Reports rewritten to match. Refs #1112

…1129) ## Summary The 2D (PP x TP) guard in `validate_pipeline_parallel_args` named `--tensor-parallel-size`, a spelling no mlxcel binary accepts. It now names `--tp-size`. Review turned up two things that matter more than the original defect, and both changed what this PR does. **The guard is unreachable.** Its `ensure!` condition is the exact negation of the early return eight lines above it, so it can never fail and the message can never be emitted. The repository's own `validate_pipeline_parallel_args_rejects_2d_without_pp_enabled` asserts `is_ok()` for `pp_size = 1, tp_size = 2` and its comment says the validator returns early. **No user has ever seen this message**, so this is a latent text fix with no user-visible change, and it ships with no CHANGELOG entry. The dead condition is left alone (the issue's fourth criterion is that the validation logic is unchanged) and filed as a follow-up. **The `docs/en/` paths are not broken references.** The issue's premise was wrong and I initially acted on it. Details below. ## What changed - `src/commands/generate.rs`: the `ensure!` message names `--tp-size`. Message text only; the condition, the `total_ranks` guard, and every other arm of the function are byte-identical. - `src/commands/generate.rs`: the validator comment keeps both operator-manual pages, explains that their sources live in the separate documentation repository by design, and adds `docs/distributed.md` as the in-checkout summary. - `tests/pp_tp_2d_real_models.rs`: both tests used `--tensor-parallel-size` on the command line. The non-ignored one is rescoped, made hermetic, and now asserts positively. Details below. ## The test was vacuous, and the obvious fix made it worse `pp_tp_2d_validator_accepts_combination` is not `#[ignore]`d, so it ran in CI, but it passed vacuously: clap rejected `--tensor-parallel-size` at argument parsing, and the test's only assertion was that a particular old rejection string is **absent** from the output, which is trivially true of a process that died before producing any. My first attempt fixed the flag name and added a guard asserting the args were not rejected at parse time. That was still wrong, because a subprocess cannot reach this validator at all. `run_generate` resolves `-m` first, since the validators read the resolved model directory: ```rust args.model.model = resolve_model_source_with_override(&args.model.model, args.model.models_dir.as_deref())?; validate_tensor_parallel_args(&args)?; validate_pipeline_parallel_args(&args)?; ``` And the test's `-m nonexistent-model-path-for-validator-only-check` is a valid bare repo segment, so once clap stopped rejecting the argv, the resolver expanded it against `$MLXCEL_DEFAULT_ORG` and went to the network. Running the exact argv: ``` [mlxcel] 'nonexistent-model-path-for-validator-only-check' -> mlx-community/nonexistent-... [mlxcel] model '...' not found locally; downloading into the mlxcel store... Error: failed to download model '...': authentication failed (HTTP 401). ``` Exit 1, before either validator, after an outbound HuggingFace request on every CI run. An offline runner would eat a connect timeout and still pass. The shipped version appends `--help`, so clap exits as soon as parsing succeeds, and asserts positively on exit status. It is hermetic and cannot pass by dying early. Verified both directions against the built binary: - `--pp-size 2 --tp-size 2 --help` exits 0. - `--pp-size 2 --tensor-parallel-size 2 --help` exits 2 with `error: unexpected argument '--tensor-parallel-size' found`. Runtime went from 1.22s to 0.09s because the network call is gone. Nothing is lost by narrowing the scope: the validator has direct unit coverage in `validate_pipeline_parallel_args_accepts_2d_pp_tp`. This also restores what the test's own comment always claimed it did; the code had drifted from the comment. ## The `docs/en/` references were real, and one acceptance criterion is knowingly not met The issue asserted `docs/en/` "has never been part of this repository" and made "no `docs/en/` reference remains in `src/`" a criterion. True of this git tree, but the conclusion does not follow: ``` mkdocs.yml:8:docs_dir: docs/en mkdocs.yml:160: - Tensor Parallelism: distributed/tensor-parallelism.md mkdocs.yml:161: - Pipeline Parallelism: distributed/pipeline-parallelism.md ``` Under `docs_dir: docs/en` those resolve to exactly the two paths the comment cited. `docs/README.md` states that `docs/en`, `docs/ko` and `docs/shared` are maintained in a separate documentation repository and that the root mkdocs configs name paths into it, verbatim: "That is deliberate, not drift." The `docs-guard` Makefile target from #1122 is built on the same fact, and that PR's own report lists "someone fixes the dangling navs by pointing them at `docs/*.md`, breaking the tree that owns them" as a high-impact risk. Deleting the references would have been that exact mistake. So the comment keeps both manual pages and explains the split, and `docs/distributed.md` is named alongside as the in-checkout summary (it covers the PP and TP knobs in separate sections and has no 2D section, so it does not replace them). **The third acceptance criterion is deliberately not satisfied**, and the second only in the sense that no *broken* reference remains. ## The sweep Every `--flag` token in `src/commands/generate.rs` was diffed against the 64 long flags the built binary advertises, rather than read by eye: ``` $ grep -o -- '--[a-z][a-z0-9-]*' src/commands/generate.rs | sort -u \ | comm -23 - <(mlxcel generate --help | grep -o -- '--[a-z][a-z0-9-]*' | sort -u) --tensor-parallel-size ``` That was the only one. `--pp-micro-batch-size`, `--pp-size`, `--pp-layers`, `--estimate-memory`, `--no-memory-check`, `--max-tokens`, `--recommend-quant` and `--surgery` all resolve. ## Test plan - [x] `MLX_CUDA_ARCHITECTURES=121 cargo test --profile test-fast --features cuda --test pp_tp_2d_real_models`: 1 passed, 1 ignored, 0 failed. - [x] Hermetic: no network request; the argv exits inside clap. - [x] Non-vacuous, verified in both directions against the built binary (exit 0 vs exit 2, above). - [x] `grep -rn -- "--tensor-parallel-size" src/` returns nothing. - [x] `cargo fmt --all -- --check` clean. - [x] `cargo clippy --profile test-fast --features cuda --lib --tests -- -D warnings` clean. ## Acceptance criteria - [x] Every flag named in the message is one the binary accepts. - [ ] ~~No `docs/en/` reference remains in `src/`~~ deliberately not met: the paths name real manual pages, see above. No *broken* reference remains. - [x] `grep -rn -- "--tensor-parallel-size" src/` returns nothing. - [x] Message text only; the validation logic is unchanged. ## Follow-ups - The unreachable `ensure!`, and `validate_pipeline_parallel_args_rejects_2d_without_pp_enabled`, whose name says "rejects" while it asserts `is_ok()`. - Machine-check the flag names in `tests/cli_help_consistency.rs` so the sweep above is enforced rather than performed once. Closes #1112

inureyes added status:review Under review type:chore Maintenance tasks (build, CI, etc.) priority:low Low priority labels Aug 13, 2026

inureyes added status:done Completed and removed status:review Under review labels Aug 13, 2026

inureyes merged commit 2c6b346 into main Aug 13, 2026
8 checks passed

inureyes deleted the chore/issue-1111-docs-target-guard branch August 13, 2026 15:34

inureyes mentioned this pull request Aug 13, 2026

docs: add technical reports for PRs #1118, #1119 and #1121 #1123

Merged

3 tasks

inureyes mentioned this pull request Aug 13, 2026

fix(cli): name the real --tp-size flag in the 2D-parallelism error #1129

Merged

10 tasks

inureyes mentioned this pull request Aug 13, 2026

chore(build): guard webpage-build against the missing MkDocs manual sources #1138

Open

5 tasks

Navigation Menu

Uh oh!

chore(docs): guard the docs-* targets and document the manual split - #1122

chore(docs): guard the docs-* targets and document the manual split#1122
inureyes merged 2 commits into
mainfrom
chore/issue-1111-docs-target-guard

inureyes commented Aug 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

inureyes commented Aug 13, 2026

Summary

Decision

What changed

Test plan

Note on the issue body

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant