fix(state): persist json checkpoints as utf-8 - #7255
Conversation
|
Thanks for the pull request. First-time contributors need an associated open issue before we can review a PR.
See the contributing guide. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesUTF-8 checkpoint I/O
Suggested reviewers: ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Thank you for the guidance. I created tracking issue #7256 and linked it from the PR body with Closes #7256. Local verification so far:
I also attempted the focused pytest target, but this bare checkout is missing the repo test/dependency setup (pytest-timeout/jsonref). I am happy to add or adjust focused tests if the maintainers prefer a different shape here. |
Problem
JsonProviderpersists checkpoint JSON with the platform default text encoding in both the sync and async paths. The matching checkpoint readers also rely on the default encoding.That makes checkpoint round-trips environment-dependent when serialized state contains non-ASCII text on systems whose default encoding is not UTF-8.
Before / after
Before this change, checkpoint writes and reads used the process/platform default encoding.
After this change, sync and async checkpoint writes/readers use
encoding="utf-8", matching JSON's expected interoperable encoding and keeping checkpoint behavior stable across platforms.Verification
python -m py_compile lib/crewai/src/crewai/state/provider/json_provider.py lib/crewai/tests/test_checkpoint.pygit diff --checkJsonProvidersync + async non-ASCII checkpoint round trip via file import: passedI also attempted:
python -m pytest lib/crewai/tests/test_checkpoint.py -k "JsonProviderFork or checkpoint_uses_utf8" -qThat local run was blocked by environment setup in this bare checkout: first the configured
--timeout=60option required a missing pytest-timeout plugin, then collection required repo dependencies such asjsonref. The added tests are narrow and exercise the same provider paths as the passing direct smoke test.Closes #7256