UN-2653 [FIX] Fixed issue in JSON structure parsing#1467
Conversation
Signed-off-by: Deepak <89829542+Deepak-Kesavan@users.noreply.github.com>
Deepak-Kesavan
requested review from
chandrasekharan-zipstack,
harini-venkataraman and
jaags-dev
Summary by CodeRabbit
WalkthroughThe changes refactor JSON repair and parsing logic in the prompt service. The manual heuristic-based approach in Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as AnswerPrompt.handle_json
participant Helper as json_repair_helper.repair_json_with_best_structure
participant RepairLib as json_repair.repair_json
Caller->>Helper: Call repair_json_with_best_structure(json_str)
Helper->>RepairLib: repair_json(json_str)
RepairLib-->>Helper: repaired JSON (original)
Helper->>RepairLib: repair_json("[" + json_str + "]")
RepairLib-->>Helper: repaired JSON (wrapped array)
Helper-->>Caller: Return best parsed structure
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…es-when-enforce-type-is-json-in-unstract-but-the-same-works-fine-when-enforce-type-is-text
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
prompt-service/src/unstract/prompt_service/utils/json_repair_helper.py (1)
8-22: Consider more specific return type annotation.The function returns
Anywhich reduces type safety. Consider using a union type likedict[str, Any] | list[Any] | strto be more specific about possible return types.-def repair_json_with_best_structure(json_str: str) -> Any: +def repair_json_with_best_structure(json_str: str) -> dict[str, Any] | list[Any] | str:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (2)
prompt-service/src/unstract/prompt_service/services/answer_prompt.py(2 hunks)prompt-service/src/unstract/prompt_service/utils/json_repair_helper.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
prompt-service/src/unstract/prompt_service/utils/json_repair_helper.py (3)
44-49: LGTM! Excellent logic for detecting unnecessary array wrapping.This condition correctly identifies when the array wrapping didn't provide additional value - when the wrapped result is just a single-element array containing the same data as the original parse.
56-58: LGTM! Smart detection of concatenated JSON objects.This logic correctly identifies when the original string likely contained multiple comma-separated JSON objects (like
{},{},{}) that were successfully parsed into a proper array structure through wrapping.
27-29: Validate json_repair behavior for wrapped arraysThe code prepends
[when callingrepair_json. If the input is missing a closing bracket (or is otherwise malformed), this may still produce invalid JSON or unexpected results.• Location:
prompt-service/src/unstract/prompt_service/utils/json_repair_helper.pylines 27–29parsed_with_wrap = repair_json( json_str="[" + json_str, return_objects=True, ensure_ascii=False )Please verify in your environment that
json_repaircorrectly handles cases such as:
- Input
"{"key":"value"}"→ yields[{"key":"value"}]- Input
"{\"key\":\"value\"},{\"key2\":\"value2\"}"→ yields[{"key":"value"},{"key2":"value2"}]- Completely malformed input (e.g.
"invalid_json") raises an error or returns a well-defined fallbackIf the library’s behavior isn’t as expected, add explicit validation or error handling around the wrapped string before parsing.
prompt-service/src/unstract/prompt_service/services/answer_prompt.py (2)
12-14: LGTM! Clean import of the new utility function.The import is well-structured and follows good practices by importing only the specific function needed.
249-250: Confirm JSON repair behavior equivalenceI wasn’t able to locate the prior inline “repair” logic in this branch to compare against
repair_json_with_best_structure. Please ensure the new utility truly matches the old behavior by:
- Reviewing or adding unit tests for all edge cases previously handled (malformed objects, arrays, comma-separated entries).
- Verifying that
ensure_ascii=Falseandreturn_objects=Truealign with the former implementation’s semantics.- Running integration tests in
prompt_service/services/answer_prompt.pyto catch any downstream parsing issues.Once covered by tests, this refactoring is safe and improves maintainability.
chandrasekharan-zipstack
left a comment
chandrasekharan-zipstack
left a comment
There was a problem hiding this comment.
LGTM - left some nit comments
…es-when-enforce-type-is-json-in-unstract-but-the-same-works-fine-when-enforce-type-is-text
|
|
Will consider these changes later. Resolving it to unblock staging. |
vishnuszipstack
deleted the
UN-2653-bug-rc-26-the-json-structure-collapses-when-enforce-type-is-json-in-unstract-but-the-same-works-fine-when-enforce-type-is-text
branch
UN-2653 Fixed issue in JSON structure parsing Signed-off-by: Deepak <89829542+Deepak-Kesavan@users.noreply.github.com>
4 participants
What
Why
How
Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
Database Migrations
Env Config
Relevant Docs
Related Issues or PRs
Dependencies Versions
Notes on Testing
Screenshots
Checklist
I have read and understood the Contribution Guidelines.