docs(patches): give every patch a schema-checked upstream status record - #95
Draft
dorado269 wants to merge 2 commits into
Draft
docs(patches): give every patch a schema-checked upstream status record#95dorado269 wants to merge 2 commits into
dorado269 wants to merge 2 commits into
Conversation
dorado269
requested review from
JohnQinAMD,
jiejingzhangamd,
limou102 and
xiaobochen-amd
as code owners
August 6, 2026 06:40
dorado269
marked this pull request as draft
August 6, 2026 06:54
dorado269
force-pushed
the
dorado/dev/patch-upstream-status-records
branch
from
August 7, 2026 07:58
f115bbc to
cf92b9e
Compare
dorado269
marked this pull request as ready for review
August 7, 2026 07:59
The single patch.upstream.status.md table could not answer the question that
decides whether a patch can go: does it change bytes in THIS engine, on THIS
pinned base. It also had no gate, so a patch could be added without a row --
patch_mooncake_mamba_unpack.py went unindexed from 2026-08-02 to 2026-08-05.
Replace it with one <patch-prefix>.upstream.status.yaml beside each patch (25
records), a patch.upstream.status.yaml index grouped by the library patched, and
a single patch.archived.yaml for retired patches. Three JSON schemas validate
them; scripts/validate-patch-status.py also cross-checks the set against the
tree, so a patch without a record, an index whose totals drift, or an archived
entry that cannot be recovered fails the lint gate.
Each record carries the fields the old table lacked: per-engine op/no-op with
its evidence, an enumerated reason the patch is still alive, and drop_signal --
how you find out it is dead. Writing that down surfaced four things:
- Four patches can only be marked `unverified`: whether they are load-bearing
depends on what the pinned base already carries and nobody has checked.
- HIPFILE_GIT_REF is unpinned, so two images from the same infera commit built
either side of rocm-systems#7386 (2026-07-16) hold different hipfile code.
- patch_hicache_rocm_staged_write_back.py does not drop when its MERGED PR
lands -- sglang#28534 is what INTRODUCED the defect; it drops on #30350.
- Six patches keep applying cleanly after upstream fixes them, so only the
pinned version says they are dead; drop_signal now enumerates which.
Nothing is deleted: the legacy vllm-dsv4 patches move to patches/archived/ via
git mv so history follows, and the old table is kept as
patch.upstream.status.superseded.md.
Image behaviour is unchanged. Every Dockerfile patch loop globs *.py or
patch_*.py, so no .upstream.status.yaml can be executed, and nothing copies
patches/ wholesale, so _schema/ and archived/ never enter an image.
The gate itself had two holes, both from discovery only running patch -> record:
- extra_files was a global set with no check that the claiming record sits
anywhere near the file it claims, so one line in any record -- in any
directory -- marked an unrelated new patch as covered and the gate passed.
A record may now only name files beside it, which is what the schema's own
description ("further files that are part of the same patch") already said.
- Nothing walked record -> patch, so a record whose patch was renamed or
deleted was never loaded, never schema-checked, and never reported. A git mv
of a patch is the realistic version of this.
Three tests cover those, each confirmed failing against the validator without
the fix.
Finally, yamllint over deploy/docker/**.yaml, which caught eleven values losing
text: an unquoted ` #` opens a YAML comment, so a PR number written mid-sentence
silently ends the value, and the result is still valid YAML that still passes
its schema.
action: Get #2725 reviewed; it has had no activity ... -> "Get"
reusable_on: An aiter older than #3033, e.g. the earlier ... -> "An aiter older than"
Quoting the eleven restores the text; that diff is quotes only, no wording
changed. --strict, because the truncation is reported as a warning and a hook
that exits 0 on warnings would not have caught it. line-length is 160 rather
than the default 80: these records quote upstream issue titles and engine error
strings verbatim, and those stay byte-exact to stay greppable. Scoped to
deploy/docker/ -- deploy/operator/ is a much larger body of existing k8s YAML
and is left alone for now.
Signed-off-by: yihou <yihou@amd.com>
dorado269
force-pushed
the
dorado/dev/patch-upstream-status-records
branch
from
August 7, 2026 08:07
cf92b9e to
8e4158b
Compare
6 tasks
…M-5.2 patches work.todo.md inventories all seven sglang GLM-5.2 patches against live upstream and finds exactly three with no PR of anyone's: the ROCm hicache allocator, the mooncake early-send wait event, and the DSA decode host-sync deadlock. Those are now sglang#33968, #33970 and #33973, all drafts. pr.done.md records the outcome. The three records move from `no-upstream-pr` to `upstream-pr-open`, gain an upstream_prs entry, and have their open_actions rewritten to say what is still owed. by_status moves 11/5 -> 8/8. Adapting each patch onto upstream main corrected the records that described them: - The hicache fix is NOT a one-line override in the shape of the merged MUSA PR #23361, as its header claimed. get_device() returns "cuda" on ROCm, so HIP has no key of its own; and memory_pool_host.py keys the table twice with a torch.device OBJECT, which is not dict-key-equal to "cuda" (measured: different hash), so those two pools always take the defaultdict default. Adding only a key misses them. - The DSA record credits base_spec_worker for supplying extend_seq_lens_cpu. It is eagle_worker_common.prepare_for_draft_extend:105. Also found a better upstream argument than the record had: the backend already declares needs_cpu_seq_lens = False to opt out of the D2H sync, so its eager fallback contradicts itself. - The mooncake patch's prefill.py edit also fixes mori, which reads the barrier but was never handed one on the overlap path. Neither header said so. Only the 2a half of the DSA diff went upstream. 2b overlaps sglang#32209 and rests on a concurrency-32 failure whose cause is unidentified, so that patch outlives its PR -- recorded in the index rather than left to be rediscovered. Every PR is a draft, and the records say why in the field that decides it: the MI355X and multi-node clusters were unreachable, so none has been re-run against the original fault on hardware that reproduces it. This session established equivalence and scope against the proven local fix on MI300X and nothing more. The index's verification note warns not to read these rows as "fixed upstream". One figure was corrected after the fact: `.max().item()` was first measured at 44 ms, but that was a first-call measurement carrying lazy-init cost. Warmed, it is 0.5-3 ms. Fixed here and in sglang#33973's body. The claim the PR rests on is unchanged and qualitative -- the call blocks at all, on a branch only some DP ranks take. work/ is gitignored: the validation scripts and working log are useful to re-run when the cluster returns, but they are not repo deliverables. Their conclusions are in the records and in the two reports. validate-patch-status.py passes and the edited .diff still applies --fuzz=0 to the pinned base. Signed-off-by: yihou <yihou@amd.com>
dorado269
marked this pull request as draft
August 7, 2026 10:15
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.
Why
deploy/docker/patch.upstream.status.mdwas a single hand-maintained table. It could not answer the question that actually decides whether a patch can be deleted — does this patch change bytes in this engine, on this pinned base — and it had no gate, so a patch could be added without a row.patch_mooncake_mamba_unpack.pywent unindexed from 2026-08-02 to 2026-08-05.What
Every patch under
deploy/docker/patches/now has a machine-checked record beside it.<patch-prefix>.upstream.status.yamldeploy/docker/patch.upstream.status.yamlpatches/archived/patch.archived.yamlpatches/_schema/*.jsonscripts/validate-patch-status.pyValidation is strict:
additionalProperties: falsethroughout, dates pattern-checked, commits[0-9a-f]{7,40}, digests^sha256:[0-9a-f]{64}$, engines and libraries enumerated. Beyond schema shape, the validator also checks that every patch on disk has a record, that the index totals match what is there, and that an archived entry claimingdeletednames a commit it can be recovered from — so the set cannot silently drift from the tree.Wired into
.pre-commit-config.yaml, which CI already runs with--all-files.tests/unit/test_patch_status_records.pyhas 8 tests, 6 of which check the validator actually refuses bad input.Beyond the fields the old table had, each record carries per-engine
effect(op / no-op / not-applied / unverified) with its evidence, an enumeratedalive_because.reason, anddrop_signal— how you will find out the patch is dead.Four things writing this down surfaced
unverified. Whetherpatch_vllm_mooncake_blocksize.py,patch_moriio_write.pyand the DSv4 MoRIIO patches are load-bearing depends on what the pinned base already carries, and nobody has checked. Each is one grep inside the image.HIPFILE_GIT_REFis unpinned.build_hipfile.shclones HEAD ofROCm/rocm-systems, so two images built from the same infera commit either side of rocm-systems#7386 (merged 2026-07-16) contain different hipfile code. A reproducibility problem independent of any patch.patch_hicache_rocm_staged_write_back.py'sMERGEDPR (sglang#28534) is what introduced the defect; it drops on the open #30350. Its own drop check is a precondition test, not an anchor test, because #30350 never touches our anchor.drop_signal.Review notes
patches/vllm-dsv4/legacy/→patches/archived/vllm-dsv4/and the superseded.mdis kept aspatches/archived/patch.upstream.status.superseded.md, both viagit mvso history follows. Delete the superseded.mdhere if you would rather it went.*.pyorpatch_*.py, so no.upstream.status.yamlcan be executed. Nothing copiespatches/wholesale, so_schema/andarchived/never enter an image. The archived legacy patches are no longer copied into the vLLM image at all.Dockerfile.vllmpins one — the rest recorddigest_source: unresolved) and PR authors.patches/archived/patch.upstream.status.superseded.mdis in this branch to diff against.Verification
No image was built, so the four
unverifiedrows stay unverified — they are recorded as such rather than guessed at.