Skip to content

check-fusion-consistency.py Check 2 is satisfied by a COMMENT: a model TU can regress to a hand-rolled gate-up and stay green #1351

Description

@localai-bot

Found while repairing review findings on #1330 (MUSIC3-DEPTH-DEVICE, #1309).

scripts/check-fusion-consistency.py Check 2 asks whether a model TU that
computes a gated-MLP epilogue routes its gate-up through a shared merged-GEMM
seam. It answers that question by running _MERGED_GEMM_SEAM over the raw
file text
:

def uses_merged_gemm_seam(text: str) -> bool:
    return bool(_MERGED_GEMM_SEAM.search(text))

def scan_models_gemm(models_dir: Path) -> ...:
    text = path.read_text(encoding="utf-8", errors="ignore")   # no comment strip
    n = count_gated_mlp_act(text)
    if n:
        out[path.stem] = (n, uses_merged_gemm_seam(text))

The only comment-aware helper in the file, allowlisted_names, strips #
comments from the allowlist file, not from the scanned sources. So a mention
of MlpGateUpMethod in a C++ comment satisfies the check as fully as a call.

Reproduction

Calling the checker's own functions on
src/vllm/model_executor/models/minimax_music3_depth_device.cpp, replacing the
real layers::UnquantizedMlpGateUpMethod call with an inline hand-rolled
vt::MatmulBT + vt::SiluAndMul path and leaving the two comment mentions
(:32, an #include trailing comment, and :234) in place:

variant count_gated_mlp_act uses_merged_gemm_seam
as shipped 2 True
hand-rolled, comments kept 3 Truechecker still satisfied
hand-rolled, comments removed 1 False — only now would it fire

A fresh reviewer reached the same result end to end: the checker returns rc=0
green with the hand-rolled path in place, and the numbers are bit-identical, so
no other gate sees it either.

Why this matters

The seam checkers are the tree's answer to "did this capability route through
the shared path". .agents/reachability.md already records that they cannot see
whether anything calls a capability. This is a different and narrower hole:
Check 2 cannot see whether the routing it reports is real. Any model TU that has
ever mentioned a merged-GEMM seam in prose is exempt from Check 2 forever.

What is NOT the defect

Renaming MlpGateUp to MlpGateUpXX is matched by the
MlpGateUp[A-Za-z]*Method regex by design (the comment at :70-72 says so,
to cover UnquantizedMlpGateUpGeluMethod). That is not a gap.

Suggested shape, not taken here

Strip line and block comments before both count_gated_mlp_act and
uses_merged_gemm_seam, with a red-before test that pins the hand-rolled +
comments-kept variant as a FAILURE. Note that stripping comments also changes
the site count, so the fixture needs to pin both numbers, and that several TUs
may newly fail — this is a semantic checker change and per AGENTS.md
## Changing the rules or a checker it needs its own spec, a red-before test or
mutation, and green-after evidence. It must not be closed by widening the regex.

Not fixed in #1330: that PR is a model row, and a checker change does not belong
in it. Listed under ## Owed in .agents/specs/minimax-music3.md §17.7, owned
by row MUSIC3-DEPTH-DEVICE.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions