Skip to content

[Bugfix][Wheel] Preserve integer map keys in structured non-tensor reads - #4603

Open
heranliu wants to merge 1 commit into
kvcache-ai:mainfrom
heranliu:fix/structured-msgpack-integer-keys
Open

heranliu wants to merge 1 commit into
kvcache-ai:mainfrom
heranliu:fix/structured-msgpack-integer-keys

Conversation

@heranliu

@heranliu heranliu commented Oct 12, 2026 •

Copy link
Copy Markdown

Description

put_dataproto() accepts non-tensor object-array values containing dictionaries with integer keys and encodes them with msgpack_ragged. Reading the same object fails with ValueError: int is not allowed for map key when strict_map_key=True.

Set strict_map_key=False in both MessagePack decoding paths so reads accept the map keys already supported by the encoder. This preserves integer keys, including nested dictionaries and nullable rows, without changing the stored format or top-level field-name rules.

Add six regression cases covering inferred and explicit schemas, nested integer keys, and rows with and without nulls.

Rebased onto main at ac4d29bff2e8bb98c36799aaf19e6c73529f0088. The fix and tests now modify only the canonical paths requested in review:

  • python/mooncake/structured_object_store.py
  • python/tests/store/test_structured_object_store.py

The legacy copies removed by #3633 remain deleted. All three empty-row dtype regression tests merged in #4611 are preserved alongside this PR's six integer-key regression cases.

Suggested Merge Priority

  • P2 — Normal: Routine bug fix

Module

  • Python Wheel (mooncake-wheel)

Type of Change

  • Bug fix

How Has This Been Tested?

Source-level validation on macOS with Python 3.12.7:

  • Executed the relocated python/tests/store/test_structured_object_store.py: 167 passed, 18 skipped.
  • All nine cases from this PR and [Wheel] Keep integer dtype for typed_ragged rows when a row is an empty list #4611 pass together. Against the latest unmodified main source, the three [Wheel] Keep integer dtype for typed_ragged rows when a row is an empty list #4611 cases pass and all six integer-key cases fail with the expected strict_map_key=True error; those six pass with this fix.
  • The test runner loads the canonical source module and uses the repository's existing in-memory store fixture with real MessagePack, NumPy, and Torch. Because this host lacks mooncake._store, the external runner supplies only a minimal process-local ReplicateConfig data object (replica_num, group_ids, and session_ids). It explicitly skips 15 tests requiring the native Store; one native tensor-serialization test and two fast-copy tests also skip. The runner is outside the PR, and does not replace encoding or decoding logic.
  • Native Store integration tests were not run locally. These source-level results do not validate the native bindings or services.

All checks below pass on both changed files:

pre-commit run --files \
  python/mooncake/structured_object_store.py \
  python/tests/store/test_structured_object_store.py
ruff check \
  python/mooncake/structured_object_store.py \
  python/tests/store/test_structured_object_store.py
python scripts/ci/check_changed_python_format.py origin/main \
  python/mooncake/structured_object_store.py \
  python/tests/store/test_structured_object_store.py
git diff --check origin/main...HEAD

Ruff version: 0.6.9, matching CI. Pre-commit made no file changes.

  • Unit tests pass (source-level validation described above)
  • Integration tests pass (not run locally; native extension unavailable)

Checklist

  • I have performed a self-review of my own code
  • I have formatted my code using ./scripts/code_format.sh (not applicable: Python-only changes)
  • I have run pre-commit on the files changed in this PR and all hooks pass
  • I have updated the documentation (not applicable: restores the existing codec round-trip behavior)
  • I have added tests to prove my changes are effective
  • For changes >500 LOC: I have filed an RFC issue (not applicable)

AI Assistance Disclosure

  • No AI tools were used
  • AI tools were used (specify below)

Codex assisted with the fix, regression tests, rebase to the canonical Python paths, and source-level validation.

@zxpdemonio

Copy link
Copy Markdown
Collaborator

Required: Please rebase this PR onto the current main and move both changes to the canonical paths under python/: python/mooncake/structured_object_store.py and python/tests/store/test_structured_object_store.py.

PR #3633 removed the legacy mooncake-wheel copies, so resolving the conflict by keeping the old paths would reintroduce duplicate sources. Please rerun the relocated structured-object tests after rebasing.

@heranliu
heranliu force-pushed the fix/structured-msgpack-integer-keys branch from 948c137 to 4517982 Compare October 19, 2026 08:30
@heranliu
heranliu requested a review from ykwd as a code owner October 19, 2026 08:30

Copy link
Copy Markdown
Author

Thanks for the guidance! Updated in 45179825: rebased onto the latest main and moved the fix and tests to the canonical python/ paths, keeping the legacy copies removed. I’ve rerun the regression cases and updated the PR description with the validation results.

@github-actions github-actions Bot added the Store label Oct 19, 2026
@heranliu
heranliu force-pushed the fix/structured-msgpack-integer-keys branch from 4517982 to b79c785 Compare October 20, 2026 05:54

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants