Repository navigation
Conversation
Aionw
requested review from
Ann-1024,
luketong777,
stmatengss and
ykwd
as code owners
August 24, 2026 08:38
15 of 31 tasks
catyans
approved these changes
Sep 2, 2026
catyans
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. The wrapper now has one canonical source under python/mooncake, and the PEP 517, legacy-wheel, and CMake paths all install it. Optional Torch handling preserves base-wheel importability; the focused lifecycle and packaging tests pass, and CI is green.
…migrate-shared-segment-to # Conflicts: # .github/CODEOWNERS # python/tests/packaging/test_installed_wheel.py # python/tests/packaging/test_project.py
catyans
approved these changes
Sep 2, 2026
catyans
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. Re-reviewed current head 2b7ba4f: the merge from main only resolves the three reported conflicts by retaining both the shared-segment ownership/packaging checks and the new EP module checks. The previously reviewed migration behavior is unchanged. CI is currently queued.
…migrate-shared-segment-to # Conflicts: # python/tests/packaging/test_project.py
Conflict resolution: - .github/labeler.yml: kept main's cli_bench.py and transfer_engine_topology_dump.py globs under Transfer Engine and appended this branch's shared_segment.py and python/tests/engine/**/* globs. - python/tests/packaging/test_installed_wheel.py: kept main's http_metadata_server and mooncake_config smoke asserts and appended the shared_segment asserts after them, matching the alphabetical import order. - python/tests/packaging/test_project.py: kept both test_shared_segment_has_one_authoritative_source and main's test_http_metadata_service_has_one_authoritative_source / test_async_store_has_one_authoritative_source as separate tests.
This branch has not been deployed
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.
Description
Implements one module-sized Phase 2 increment of #3534 for the shared-segment Python helper.
mooncake.shared_segmentfrommooncake-integration/shared_segment.pytopython/mooncake/shared_segment.pyas its sole tracked source;mooncake.shared_segmentimport and nativemooncake.engineboundary;This PR does not rename the native engine extension or add the Phase 3
mooncake.enginefacade.Overlap
No open PR migrates the shared-segment helper, but #4152 (open) adds a
hugetlboption tomooncake-integration/shared_segment.py, the file this PR moves; whichever lands second must carry that change topython/mooncake/shared_segment.pyand its tests. Since this PR was opened, #3586, #3624, #3626, #3630, #3632, and #3635 have landed onmain; this branch includes them through itsmainsyncs, most recently the 2026-09-28 merge ofmain(d9152a4). #3585 (Reshard) is still open and remains out of scope. This PR does not duplicate any of those migrations.Module
mooncake-transfer-engine)mooncake-store)mooncake-reshard)mooncake-ep)mooncake-pg)mooncake-integration)mooncake-p2p-store)mooncake-wheel)mooncake-common)mooncake-rl)Type of Change
How Has This Been Tested?
Original submission (before merging
mainon 2026-09-28; superseded by "After mergingmain" below):Test commands:
MOONCAKE_TEST_WHEEL=/tmp/mooncake-shared-segment-wheel.VDwWPN/mooncake_transfer_engine-0.3.12.post1-cp310-cp310-linux_x86_64.whl \ python -m pytest -q python/tests/engine/test_shared_segment.py python/tests/packaging CMAKE_BUILD_PARALLEL_LEVEL=128 python -m build --wheel \ --outdir /tmp/mooncake-shared-segment-wheel.VDwWPN python -m twine check \ /tmp/mooncake-shared-segment-wheel.VDwWPN/mooncake_transfer_engine-0.3.12.post1-cp310-cp310-linux_x86_64.whl pre-commit run --files $(git diff --name-only --diff-filter=ACMR origin/main...HEAD)Test results:
twine checkpassed; the wheel was installed with--no-depsinto a fresh virtual environment and imported underpython -Iwithout repositoryPYTHONPATH)The available environment had no CUDA/NPU runtime, so live VMM/HostRegister allocation was not exercised. Native capability branches and cleanup are covered with focused binding fakes, and the native shared-segment binding compiled in the root non-CUDA wheel build.
After merging
main(d9152a4, 2026-09-28):PYTHONPATH=python:mooncake-wheel python -m pytest -q python/tests/engine/test_shared_segment.py python/tests/packaging pre-commit run --files $(git diff --name-only --diff-filter=ACMR origin/main...HEAD)MOONCAKE_TEST_WHEEL/MOONCAKE_TEST_LEGACY_WHEEL..github/labeler.yml,python/tests/packaging/test_installed_wheel.py,python/tests/packaging/test_project.pywere resolved by keeping both sides.ruff checkplusscripts/ci/check_changed_python_format.py, ruff 0.6.9) passes.cmake-format, which only rewrites thetent_metricsblock inmooncake-integration/CMakeLists.txtinherited frommain;mainat d9152a4 fails the same way, and this PR's own CMake lines pass.twine check, and installed-wheel smoke runs from the original submission were not repeated after the merge. CI does not run them either (it builds only the legacyscripts/build_wheel.shwheel and runsscripts/test_installation.sh), so they still need a rerun.Checklist
./scripts/code_format.sh(no C/C++ source changes; all relevant pre-commit formatting hooks pass)main, except the pre-existingcmake-formatissue noted under "After mergingmain")AI Assistance Disclosure
OpenAI Codex assisted with the scoped module migration, reference audit, test implementation, packaging validation, and PR preparation. The human submitter remains responsible for reviewing and defending every changed line.
Claude Code (Anthropic) merged
maininto this branch on 2026-09-28, resolved the conflicts, re-ran the focused tests listed under "After mergingmain", and updated this description.