Skip to content

chore(ci): check environment-variables.md against src - #2486

Merged
inureyes merged 2 commits into
lablup:mainfrom
hoyajigi:chore/issue-1140-env-var-docs-check
Oct 20, 2026
Merged

inureyes merged 2 commits into
lablup:mainfrom
hoyajigi:chore/issue-1140-env-var-docs-check

Conversation

@hoyajigi

Copy link
Copy Markdown
Member

Summary

Adds a toolchain-free gate that keeps docs/environment-variables.md in sync with the MLXCEL_* names read under src/, and documents the operator-facing variables that were missing so main passes it. Every MLXCEL_* string literal in src/**/*.rs must now be on the page or carry a reasoned entry in the checker's allowlist.

Related issues

Closes #1140

Type of change

  • chore — build, CI, dependencies, release infrastructure
  • docs — documentation only

Test plan

  • python3 scripts/ci/check_env_var_docs.py: OK (294 names: 206 documented, 88 allowlisted)
  • python3 -m unittest tests/test_check_env_var_docs.py: 15 tests OK
  • make verify-env-var-docs: OK; full tests/ Python suite: 139 OK, 28 skipped
  • Negative checks on the real tree: a new std::env::var("MLXCEL_NEW_THING") fails and names its path:line; deleting the MLXCEL_KIMI_K3_MAX_MEDIA_TOKENS row fails (both reverted)
  • actionlint .github/workflows/ci.yml: no findings in the new job (the 8 shellcheck infos are pre-existing on main)
  • insert_apache_header.py --check, git diff --check
  • Rust build/tests: not run, no Rust changed

Notes for reviewers

  • Re-measured counts (the issue's 155/147/25 were measured at d5abf1fa): 294 names in src, 213 on the page, 110 missing. Of those, 22 operator-facing variables are now documented (speculative decoding, OpenXLA, kernel diagnostics, logging/profiling, media limits, build-time GIT_COMMIT, REQUIRE_MODELS), each from the code that reads it; 88 are allowlisted by group with a reason (C++ macros from bridge.define, cargo:rustc-env names, XLA golden-freeze and graph-dump switches, child-process handshakes, sample counts, probe parameters, fixture paths).
  • Stale rows: 34 names on the page are not in src/**/*.rs, but 29 are read by C++/CUDA, tests/ or scripts and 5 are already marked "Removed in refactor: run mlxcel generate on the engine and retire CxxGenerator #2176", so none were deleted. The optional page-to-src reverse check is not implemented; the script's docstring explains why.
  • Wiring: make verify-env-var-docs joins verify and verify-rocm and the help list; the env-var-docs CI job sits beside kernel-dtype-keys outside the changes filter so a docs-only row deletion is also caught. CONTRIBUTING.md now lists ten make verify prerequisites and seven cheap gates.
  • Allowlist hygiene: the checker also fails on an allowlist entry no longer in src, an entry that is also on the page, or an empty scan.
  • "Documented" means mentioned anywhere on the page, as the issue suggests, so a row deletion is not caught if the name still appears in another row's prose.

Checklist

  • PR title uses a Conventional Commits prefix
  • One logical change per PR
  • Updated docs/
  • No secrets, credentials, or .env files committed

docs/environment-variables.md had drifted from the variables the code
reads: 110 MLXCEL_* names that appear as string literals under src/**/*.rs
were absent from the page. Most of them are test fixtures, probe knobs and
build-internal names that should stay off an operator reference, but 22 are
read at run time by production code (or are contributor-facing gates of the
kind the page already lists) and had no row.

Document each in the section that fits, from the code that reads it:
values and parse semantics, default, effect, and the component that reads
it. Speculative decoding gains the batched-MTP ragged window, the lablup#203
escape hatch, drafter quantization and the lablup#1204 tree-drafting knobs; the
OpenXLA section gains the server context buckets, load timing and the
Phi-4-multimodal audio precision; the diagnostic sections gain the DSA and
MiniMax-M3 dense fallbacks, the CUDA qmm_naive tile pin (with its paired
MLXCEL_TRACE_QMM_TILE, read only from CUDA source), the shapeless-compile
audit, MTP gap logging, the audio probe dump, the VoiceChat profile
outputs, bench phase marks and the MLXCEL_API_KEY log redaction. Kimi K3's
per-request media-token budget, MLXCEL_GIT_COMMIT (build time) and
MLXCEL_REQUIRE_MODELS (tests and CI) complete the set.

The remaining 88 names are allowlisted with reasons by the check added in
the next commit.
Nothing kept docs/environment-variables.md in sync with the MLXCEL_*
variables the code reads, so every new call site was a silent chance for
drift, found only when someone happened to look.

scripts/ci/check_env_var_docs.py (standard library only, no toolchain)
collects every MLXCEL_* string literal in src/**/*.rs and fails, naming
the variable and up to three files that contain it, when a name is neither
mentioned on the page nor in the script's ALLOWLIST. The failure message
states both resolutions: document the variable, or allowlist it with a
reason. It also fails when an allowlist entry is no longer in src or is
also on the page, so the allowlist cannot rot, and when the scan finds
nothing.

The allowlist, not a path or #[cfg(test)] exclusion, carries the exceptions
(88 entries, grouped by reason: test-only, fixture-authoring,
build-internal, internal diagnostic). MLXCEL_FREEZE_GOLDENS is read from
#[cfg(test)] code in a file with no test marker in its name, and several
*_tests.rs modules are also compiled into diagnostic builds, so a blanket
exclusion would hide names without recording that anyone judged them.

tests/test_check_env_var_docs.py covers the checker on synthetic trees: a
new undocumented call site, a deleted row for a name still read, stale and
contradictory allowlist entries, and an empty scan must each fail.

Wire it in like kernel-dtype-keys: `make verify-env-var-docs`, a
prerequisite of `make verify` and `make verify-rocm`, and an `env-var-docs`
CI job that is not behind the `changes` filter, because a docs-only change
that deletes a row has to be gated as well as a src change. CONTRIBUTING.md
and the page itself name the new gate.

Closes lablup#1140
@inureyes inureyes added status:review Under review type:chore Maintenance tasks (build, CI, etc.) priority:low Low priority labels Oct 20, 2026
@inureyes

Copy link
Copy Markdown
Member

PR Implementation Hardening Summary

  • Target PR: chore(ci): check environment-variables.md against src #2486 (chore(ci): check environment-variables.md against src)
  • Linked Issue: chore: Add a CI check keeping environment-variables.md in sync with src #1140
  • Review Findings:
    • Audited scripts/ci/check_env_var_docs.py, tests/test_check_env_var_docs.py, docs/environment-variables.md, Makefile, .github/workflows/ci.yml, and CONTRIBUTING.md against all acceptance criteria in chore: Add a CI check keeping environment-variables.md in sync with src #1140.
    • Confirmed stdlib-only implementation, explicit reasoned allowlist (88 internal/test/fixture/build variables), 22 newly documented operator-facing variables verified against their call sites, bidirectional allowlist hygiene checks (stale or doubly-listed entries rejected), unconditional CI execution outside the changes path filter, and make verify / make verify-rocm wiring.
  • Correctness / Security / Performance:
    • Correctness: Passes on the merged main tree (294 MLXCEL_* names in 2080 *.rs files: 206 documented, 88 allowlisted) and passes all 15 unit tests in tests/test_check_env_var_docs.py.
    • Security: CI workflow step uses persist-credentials: false and contents: read; MLXCEL_API_KEY documentation accurately notes it is a log-redaction secret rather than an auth source.
    • Performance: Runs in ~0.2s with no toolchain dependency.
  • Validation:
    • python3 scripts/ci/check_env_var_docs.py -> OK
    • python3 -m unittest tests/test_check_env_var_docs.py -v -> 15 passed
    • GitHub Actions CI: all 28 checks green.

@inureyes
inureyes merged commit 2f52b3a into lablup:main Oct 20, 2026
28 checks passed
@inureyes inureyes added status:done Completed and removed status:review Under review labels Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority:low Low priority status:done Completed type:chore Maintenance tasks (build, CI, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore: Add a CI check keeping environment-variables.md in sync with src

2 participants