Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ParseJSONDataComponent prevent unnecessary array wrapping #4357

Merged
merged 3 commits into from
Nov 11, 2024

Conversation

bhatsanket
Copy link
Contributor

Summary: Prevent wrapping non-array inputs in arrays to ensure correct JSON parsing and filtering.

This PR addresses an issue where the ParseJSONDataComponent was always wrapping the input in an array, even if the input was not an array. This caused incorrect JSON parsing and filtering.
Changes
Modified the filter_data method to check if the input is a list before wrapping it.
If the input is not a list, it is processed as a single value.
If the input is a list, each item is processed separately.

Testing:
Run jq query - .

  • input non array
    image

output
image

  • input array
    image

output
image

@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Nov 1, 2024
Copy link

codspeed-hq bot commented Nov 11, 2024

CodSpeed Performance Report

Merging #4357 will degrade performances by 21.05%

Comparing bhatsanket:fix/ParseJSONDataComponent (4424571) with main (a107650)

Summary

⚡ 3 improvements
❌ 1 regressions
✅ 11 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main bhatsanket:fix/ParseJSONDataComponent Change
test_build_flow_from_request_data 780.3 ms 705.6 ms +10.58%
test_invalid_run_with_input_type_chat 14.6 ms 18.5 ms -21.05%
test_successful_run_with_input_type_text 456 ms 378.9 ms +20.34%
test_successful_run_with_output_type_any 455.2 ms 408.1 ms +11.55%

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Nov 11, 2024
@italojohnny italojohnny force-pushed the fix/ParseJSONDataComponent branch from bf80f13 to fe242d3 Compare November 11, 2024 13:50
@italojohnny italojohnny enabled auto-merge (squash) November 11, 2024 13:51
@italojohnny italojohnny self-requested a review November 11, 2024 17:53
Copy link
Member

@italojohnny italojohnny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi there! Hope you're doing well, and thanks for the PR!
Would you mind also correcting the following lines in the tests?

diff --git a/src/backend/tests/integration/components/helpers/test_parse_json_data.py b/src/backend/tests/integration/components/helpers/test_parse_json_data.py
index d0aff47ca..7b5a1c3ff 100644
--- a/src/backend/tests/integration/components/helpers/test_parse_json_data.py
+++ b/src/backend/tests/integration/components/helpers/test_parse_json_data.py
@@ -37,7 +37,7 @@ async def test_from_message():
         ParseJSONDataComponent,
         inputs={
             "input_value": ComponentInputHandle(clazz=ChatInput, inputs={}, output_name="message"),
-            "query": ".[0].key",
+            "query": ".key",
         },
         run_input="{'key':'value1'}",
     )
@@ -47,7 +47,7 @@ async def test_from_message():
         ParseJSONDataComponent,
         inputs={
             "input_value": ComponentInputHandle(clazz=ChatInput, inputs={}, output_name="message"),
-            "query": ".[0].key.[0].field2",
+            "query": ".key.[0].field2",
         },
         run_input='{"key":[{"field1": 1, "field2": 2}]}',
     )

https://github.com/bhatsanket/langflow-original/blob/557ee6884205eeb77103eace9f5f2babf47b02e7/src/backend/tests/integration/components/helpers/test_parse_json_data.py#L29

https://github.com/bhatsanket/langflow-original/blob/557ee6884205eeb77103eace9f5f2babf47b02e7/src/backend/tests/integration/components/helpers/test_parse_json_data.py#L50

@dosubot dosubot bot removed the lgtm This PR has been approved by a maintainer label Nov 11, 2024
@italojohnny italojohnny self-requested a review November 11, 2024 18:08
auto-merge was automatically disabled November 11, 2024 18:16

Head branch was pushed to by a user without write access

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Nov 11, 2024
@bhatsanket
Copy link
Contributor Author

bhatsanket commented Nov 11, 2024

Hi there! Hope you're doing well, and thanks for the PR! Would you mind also correcting the following lines in the tests?

diff --git a/src/backend/tests/integration/components/helpers/test_parse_json_data.py b/src/backend/tests/integration/components/helpers/test_parse_json_data.py
index d0aff47ca..7b5a1c3ff 100644
--- a/src/backend/tests/integration/components/helpers/test_parse_json_data.py
+++ b/src/backend/tests/integration/components/helpers/test_parse_json_data.py
@@ -37,7 +37,7 @@ async def test_from_message():
         ParseJSONDataComponent,
         inputs={
             "input_value": ComponentInputHandle(clazz=ChatInput, inputs={}, output_name="message"),
-            "query": ".[0].key",
+            "query": ".key",
         },
         run_input="{'key':'value1'}",
     )
@@ -47,7 +47,7 @@ async def test_from_message():
         ParseJSONDataComponent,
         inputs={
             "input_value": ComponentInputHandle(clazz=ChatInput, inputs={}, output_name="message"),
-            "query": ".[0].key.[0].field2",
+            "query": ".key.[0].field2",
         },
         run_input='{"key":[{"field1": 1, "field2": 2}]}',
     )

https://github.com/bhatsanket/langflow-original/blob/557ee6884205eeb77103eace9f5f2babf47b02e7/src/backend/tests/integration/components/helpers/test_parse_json_data.py#L29

https://github.com/bhatsanket/langflow-original/blob/557ee6884205eeb77103eace9f5f2babf47b02e7/src/backend/tests/integration/components/helpers/test_parse_json_data.py#L50

@italojohnny Updated. Thnx!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Nov 11, 2024
@italojohnny italojohnny enabled auto-merge (squash) November 11, 2024 18:33
@italojohnny italojohnny requested review from Cristhianzl and removed request for Cristhianzl November 11, 2024 19:22
bhatsanket and others added 3 commits November 11, 2024 20:25
@ogabrielluiz ogabrielluiz force-pushed the fix/ParseJSONDataComponent branch from 159f705 to 4424571 Compare November 11, 2024 23:26
@italojohnny italojohnny merged commit 19963de into langflow-ai:main Nov 11, 2024
28 checks passed
diogocabral pushed a commit to headlinevc/langflow that referenced this pull request Nov 26, 2024
…low-ai#4357)

* fix(ParseJSONDataComponent): prevent unnecessary array wrapping

Prevent wrapping non-array inputs in arrays to ensure correct JSON parsing and filtering.

* [autofix.ci] apply automated fixes

* updated failing test

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants