Feat/json schemas for llm output#87
Merged
srijanpatel merged 15 commits intomainfrom Jan 15, 2025
Merged
Conversation
…chema to a simplified schema representation
…d schema handling
…nd codemirror-json-schema, enhancing JSON schema support in the frontend
… including codemirror-json-schema and related modules, to streamline the frontend build
…json() extension in CodeMirror
…al escaped characters and improve error handling
Collaborator
Author
|
@ellipsis-dev review this |
Contributor
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 0818003 in 2 minutes and 35 seconds
More details
- Looked at
796lines of code in11files - Skipped
2files when reviewing. - Skipped posting
7drafted comments based on config settings.
1. backend/app/execution/workflow_executor.py:228
- Draft comment:
Consider using a logging framework instead ofprintfor better control over log levels and outputs. - Reason this comment was not posted:
Confidence changes required:50%
The code inworkflow_executor.pyusesprintstatements for logging, which is not a best practice for production code. It should use a logging framework instead.
2. backend/app/nodes/llm/_providers.py:39
- Draft comment:
Ensure that all references to_setup_azure_configurationare updated tosetup_azure_configurationto avoid runtime errors. - Reason this comment was not posted:
Comment did not seem useful.
3. backend/app/nodes/llm/_utils.py:482
- Draft comment:
Consider refactoring the logic for handlingoutput_json_schemaandoutput_schemato make it clearer and more robust. Ensure that the default schema is applied correctly when both areNone. - Reason this comment was not posted:
Confidence changes required:50%
Thegenerate_textfunction in_utils.pyhas a potential issue with theoutput_json_schemaandoutput_schemahandling. If both areNone, it defaults to a simple schema, but this logic could be clearer and more robust.
4. backend/app/nodes/llm/generative/best_of_n.py:126
- Draft comment:
Ensure thatjson_schema_to_simple_schemacorrectly handles all necessary cases when convertingoutput_json_schemato a simple schema. - Reason this comment was not posted:
Confidence changes required:50%
Inbest_of_n.py, theoutput_json_schemais being converted to a simple schema usingjson_schema_to_simple_schema. This conversion should be verified to ensure it handles all necessary cases.
5. backend/app/nodes/llm/single_llm_call.py:72
- Draft comment:
Ensure thatjson_schema_to_modelcorrectly handles all necessary cases when convertingoutput_json_schemato a Pydantic model. - Reason this comment was not posted:
Confidence changes required:50%
Insingle_llm_call.py, theoutput_json_schemais being converted to a Pydantic model usingjson_schema_to_model. This conversion should be verified to ensure it handles all necessary cases.
6. backend/app/utils/pydantic_utils.py:129
- Draft comment:
Consider logging theValueErrorinjson_schema_to_pydantic_typefor better debugging and traceability. - Reason this comment was not posted:
Confidence changes required:30%
Inpydantic_utils.py, the functionjson_schema_to_pydantic_typeraises aValueErrorfor unsupported types. It might be beneficial to log this error for better debugging.
7. frontend/src/components/nodes/nodeSidebar/NodeSidebar.tsx:89
- Draft comment:
Ensure that the JSON cleaning process inextractSchemaFromJsonSchemais robust and handles edge cases effectively. - Reason this comment was not posted:
Confidence changes required:40%
InNodeSidebar.tsx, theextractSchemaFromJsonSchemafunction attempts to clean and parse JSON. Ensure that this cleaning process is robust and handles edge cases.
Workflow ID: wflow_KgEOymDNSk4egYDZ
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
JeanKaddour
approved these changes
Jan 15, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request includes several changes to enhance the handling of JSON schemas and response formats in the LLM-related modules. The most important changes include adding support for JSON schema validation, improving error handling, and refactoring some utility functions.
Enhancements to JSON schema handling:
backend/app/nodes/llm/_providers.py: Addedresponse_formattoOllamaOptionsto specify the format of the response.backend/app/nodes/llm/_utils.py: Added parametersoutput_json_schemaandoutput_schematogenerate_textand implemented logic to handle these parameters, including converting simple schemas to JSON schemas. [1] [2]backend/app/nodes/llm/generative/best_of_n.py: Updatedsetup_subworkflowto handleoutput_json_schemaand convert it to a simple schema if needed. [1] [2]backend/app/nodes/llm/single_llm_call.py: Addedoutput_json_schematoSingleLLMCallNodeConfigand updated therunmethod to use this schema for response validation. [1] [2]Error handling improvements:
backend/app/execution/workflow_executor.py: Added a try-except block to handleValidationErrorwhen processing precomputed outputs, logging a warning message if validation fails.Refactoring and utility functions:
backend/app/nodes/llm/_utils.py: Refactoredsetup_azure_configurationfunction and movedjson_schema_to_modelimport to a more appropriate location. [1] [2]backend/app/utils/pydantic_utils.py: Added utility functionconvert_output_schema_to_json_schemato convert simple output schemas to JSON schemas.