Repository navigation
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
github-actions
Bot
added
the
CLA signed
|
I have read the CLA Document and I hereby sign the CLA |
1 similar comment
|
I have read the CLA Document and I hereby sign the CLA |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| for k, v in input_dict.items(): | ||
| if k == "arguments": | ||
| # Tool arguments are caller JSON, not SDK fields to normalize. | ||
| output_dict[k] = deepcopy(v) |
There was a problem hiding this comment.
When ToolCallRequest.from_dict() is given a valid read-only Mapping such as types.MappingProxyType({"order_id": 1}), this line raises TypeError: cannot pickle 'mappingproxy' object before msgspec can validate it. arguments is explicitly typed as Mapping[str, ...], and the previous traversal accepted such implementations by materializing their items into a normal dictionary, so wrapper or immutable mappings now regress; copy mappings by traversing their JSON contents while leaving their keys untouched rather than calling deepcopy on the mapping object itself.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1e58c74: replaced deepcopy with a key-preserving structural copy — mappings (including read-only/custom ones like MappingProxyType) become fresh dicts, lists/tuples become fresh lists, and cycles raise LMStudioValueError as before. Added regressions for top-level/nested read-only and custom mappings, cycles, and shared nested values. Focused schema/history tests: 70 passed; Ruff, format and strict mypy pass.
2c9e402 to
1e58c74
Compare
Tool call arguments imported with
Chat.from_history(dict)or appended as anAssistantResponsemust retain caller-defined keys. The previous normalizer changed nested keys and could collapse distinctorder_idandorderIdvalues.Preserve the
argumentsboundary with JSON structural copying: mappings become fresh dictionaries without renaming keys, lists/tuples become fresh lists, and scalar values remain unchanged. Unlike the earlier PR revision'sdeepcopyapproach, this normalizer materializes read-only mappings such asMappingProxyTypeaccepted byToolCallRequest.from_dict. ID-based cycle detection raisesLMStudioValueError("Data structure cycles are not supported."), while nested lists remain allowed inside arguments. Enclosing SDK aliases and typed tool-request validation remain unchanged; generated models are untouched.Coverage includes dictionary history and append, structured append, both collision orders, nested containers, bidirectional copy independence, Chat copies, legacy plugin payloads, and mapping validation. Ten additional cases in this revision cover top-level/nested read-only and custom mappings, mapping/proxy/list/tuple/envelope cycles, and nested sequences with shared references.
Validation on Python 3.12.9:
MYPYPATH=src).No live integration, interpreter/platform matrix or distribution build was run. Selected Python I/O instrumentation is not OS/native behavior proof.