Skip to content

Python: fix(checkpoint): preserve dicts whose keys collide after str() - #8272

Open
LI (ktz03) wants to merge 1 commit into
microsoft:mainfrom
ktz03:fix/checkpoint-dict-key-collision
Open

Python: fix(checkpoint): preserve dicts whose keys collide after str()#8272
LI (ktz03) wants to merge 1 commit into
microsoft:mainfrom
ktz03:fix/checkpoint-dict-key-collision

Conversation

@ktz03

Copy link
Copy Markdown

Description

Fixes #8256. Checkpoint encoding converts dict keys with str(k). Distinct keys like 1 and "1" collapsed and silently lost a value.

Narrow fix: if len({str(k) for k in d}) != len(d), pickle the whole mapping (same path as reserved-marker keys). Non-colliding non-string keys still stringify.

Validation

  • Unit: test_encode_dict_with_stringified_key_collision_uses_pickle
  • Existing test_encode_dict_with_non_string_keys still passes

When dict keys collide after str() (e.g. 1 and "1"), JSON encoding silently overwrote values. Pickle those mappings instead (microsoft#8256).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused implementation resolves the reported data-loss case and includes appropriate regression coverage.

Pull request overview

Prevents checkpoint data loss when distinct dictionary keys stringify identically.

Changes:

  • Pickles mappings with stringified-key collisions.
  • Adds a restricted-decoding round-trip regression test.
File summaries
File Description
python/packages/core/agent_framework/_workflows/_checkpoint_encoding.py Detects collisions before dictionary encoding.
python/packages/core/tests/workflow/test_checkpoint_encode.py Verifies colliding keys retain both values.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@ktz03

Copy link
Copy Markdown
Author

Fork CI for this PR may need a maintainer to Approve workflows before Python tests run (same as other community PRs). Happy to address review feedback once checks appear.

@FOWEPJF255

Copy link
Copy Markdown
Contributor

Nice narrow fix for the str(k) key-collision case in checkpoint encoding 鈥?pickle fallback when len({str(k)}) != len(d) is easy to reason about and CI is green.

Should be ready for maintainer review on #8256.

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

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: Checkpoint encoding silently loses values when dictionary keys collide after string conversion

3 participants