Improve MD_JSON mode #490
Improve MD_JSON mode #490
Conversation
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested.
- Reviewed the entire pull request up to 85d584a
- Looked at
468lines of code in7files - Took 1 minute and 54 seconds to review
More info
- Skipped
0files when reviewing. - Skipped posting
0additional comments because they didn't meet confidence threshold of85%.
Workflow ID: wflow_3C5li4I2DmYLhXjz
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. We'll respond in a few minutes. Learn more here.
| @@ -13,13 +14,21 @@ def from_streaming_response( | |||
| cls, completion: Iterable[Any], mode: Mode, **kwargs: Any | |||
| ) -> Generator[BaseModel, None, None]: # noqa: ARG003 | |||
| json_chunks = cls.extract_json(completion, mode) | |||
|
|
|||
| if mode == Mode.MD_JSON: | |||
| json_chunks = extract_json_from_stream(json_chunks) | |||
There was a problem hiding this comment.
The new utility function extract_json_from_stream is used here to handle the MD_JSON mode. This change seems logical, but there are no updates to the documentation or tests to reflect this change. Please ensure that the documentation and tests are updated accordingly. This comment is also applicable to other places where similar changes are made.
There was a problem hiding this comment.
👍 Looks good to me!
- Performed an incremental review on 8bc656f
- Looked at
306lines of code in14files - Took 2 minutes and 5 seconds to review
More info
- Skipped
0files when reviewing. - Skipped posting
0additional comments because they didn't meet confidence threshold of85%.
Workflow ID: wflow_e0F0DWlkQ1YcZnLn
Not what you expected? You can customize the content of the reviews using rules. Learn more here.
There was a problem hiding this comment.
👍 Looks good to me!
- Performed an incremental review on 3e44a6b
- Looked at
773lines of code in21files - Took 1 minute and 59 seconds to review
More info
- Skipped
0files when reviewing. - Skipped posting
1additional comments because they didn't meet confidence threshold of85%.
1. docs/concepts/caching.md:33:
- Assessed confidence :
50% - Comment:
The PR modifies a lot of markdown files. Please ensure that if a newmdfile is created indocs, it's added tomkdocs.yml. - Reasoning:
The PR introduces utility functions to handle MD_JSON mode and applies them across the codebase. The changes seem to be well implemented and the code is clean. The PR also includes updates to the documentation and tests, which is good. However, I noticed that the PR modifies a lot of files, including some markdown files in thedocsdirectory. According to the rules, if a newmdfile is created indocs, it should be added tomkdocs.yml. I need to check if this rule has been followed.
Workflow ID: wflow_DKpzSOxxx2XeLkmN
Not what you expected? You can customize the content of the reviews using rules. Learn more here.
Summary:
This PR improves the handling of MD_JSON mode by introducing utility functions to extract JSON from code blocks and streams, applying these functions across the codebase, updating the handling of response models, updating tests, and making changes in the documentation files.
Key points:
extract_json_from_codeblock,extract_json_from_stream, andextract_json_from_stream_asyncinutils.py.iterable.py,partial.py, andfunction_calls.pyto improve handling of MD_JSON mode.patch.py.patch.pytoutils.py.util.pyand added new tests intest_utils.py.Generated with ❤️ by ellipsis.dev