feat: Key-value extraction #320

remi-guan · 2024-12-28T14:25:14Z

Description

Our new feature to extract data from PDF by key-value. Integrated with our async backend API.

Code Structure

Modify app/components/playground/ActionContainer.tsx to add a new tab. The enum PlaygroundTabs is also required to edit.
Implement a container app/components/playground/ExtractKeyValuePairContainer.tsx. I've put the upload logic within, and use our store app/hooks/usePlaygroundStore.ts to preserve the input/output, so you don't lose the data while you're working with the file with the other features. (Like you can do both extracting text while you're extracting the key-value) Also add types of the store file app/types/PlaygroundTypes.ts.
I used a tutorial with a free, easy-to-use library React Joyride. By adding steps to each dom element you can easily define the tutorial steps. Create the component and put it inside the container, then you should get a tutorial. app/components/tutorials/ExtractKeyValuePairTutorial.tsx
Implement a test.

API Integration

I'm using async extract_key_value API by uploading to a preassigned s3 URL by /async/upload API with parameters to trigger an extract key value action. And continue to fetch the result with /async/fetch API. This logic has already been implemented by runAsyncRequestJob function.

I also have a file implemented this feature with the sync API endpoint /extract_key_value. This logic is not being used, since sync API usually reach 30s timeout limit causing we get an error.

However, the file has still been preserved since I saw that we can use it to implement a retry mechanism like this module MarkdownExtractContainer.tsx.

But implementing such logic require more time, and our current code is able to use now. So I haven't scheduled time to do that yet.

Related Issue

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update
Code refactoring
Performance improvement

How Has This Been Tested?

Tested:

Upload a file, enter a key and description to extract.
Tutor should automatically pop over.(runs only once, unless you clear your local storage)
Inputs and the extract result should be preserved if you switch tabs.
“Back to file” and “Download” button.
Toast is working fine.
Scrolling is working fine.

Screenshots (if applicable)

Checklist

My code follows the project's style guidelines
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

Additional Notes

Copilot reviewed 6 out of 15 changed files in this pull request and generated no comments.

Files not reviewed (9)

.vscode/settings.json: Language not supported
package.json: Language not supported
test-results/.last-run.json: Language not supported
app/components/playground/KeyValueInputs.tsx: Evaluated as low risk
app/components/tutorials/ExtractKeyValuePairTutorial.tsx: Evaluated as low risk
app/hooks/usePlaygroundStore.ts: Evaluated as low risk
app/types/PlaygroundTypes.ts: Evaluated as low risk
tests/coreTests.ts: Evaluated as low risk
app/components/playground/ActionContainer.tsx: Evaluated as low risk

Comments suppressed due to low confidence (2)

app/components/playground/ExtractKeyValuePairContainer.tsx:205

The 'formattedExtractResult' memoization logic assumes the structure of the 'extractResult'. If the structure changes, it could break. Consider adding a check or comment to clarify the assumption.

const content = typeof selectedFile.extractResult === 'string' ? JSON.parse(selectedFile.extractResult) : selectedFile.extractResult;

app/actions/apiInterface.ts:54

The new parameter 'vqaExtractInstruction' should be documented properly in the interface.

vqaExtractInstruction?: Record<string, string>;

lingjiekong · 2024-12-29T18:28:32Z

Suggestion: for this kind of complex feature, it will be great to record a video to ease the effort on the reviewer side.

lingjiekong · 2024-12-29T19:08:43Z

@remi-guan Good progress on implementing this feature and I tested after pulling your code locally and still have couple problems that I want you to work with @boqiny to resolve.

@remi-guan make sure you add a bit more details regarding what is the backend api that you are integrating with, so it ease the review for the backend engineers in the future.
@remi-guan after extracting key value is done, I move back to the file by clicking "back to file", I feel we miss a feature to move to the extract key value without re-rerunning it again by clicking "extract key-value". I feel we can add a "back to key-value result"-ish button without re-running.
@boqiny it looks like the quota update is not right and no matter the page of files, we are always decrement 1 page.
@boqiny what is the extract key value strategy for now?

lingjiekong · 2024-12-29T19:09:42Z

lingjiekong · 2024-12-29T19:14:24Z

@remi-guan, we used to have two frontend dev setup.

old for the current frontend: https://www.notion.so/goldpiggy/TS-Frontend-Dev-Setup-V1-for-current-frontend-ada0f7cdf74c424c8767ed692150cc88?pvs=4
new for the future frontend or ts related project: https://www.notion.so/goldpiggy/TS-Frontend-Dev-Setup-V2-using-t3-376dd057f5b54e5bb0df1f806b8757ea?pvs=4

You can take a look for the current setup and please feel free to comment and modify the notion page regarding how to better setup future frontend or ts related project.

lingjiekong · 2024-12-29T19:10:05Z

.vscode/settings.json

@@ -8,4 +8,7 @@
  "editor.codeActionsOnSave": {
      "source.fixAll.eslint": "explicit"
  },
+  "cSpell.words": [
+    "cambio"
+  ],
 }


nit: new line at the end of file.

lingjiekong · 2024-12-29T19:11:21Z

app/actions/runSyncExtractKeyValue.ts

it looks like you are using async extract_key_value api by uploading to a preasigned s3 url to trigger a extract key value and continue to fetch the result. Therefore, shall we name this to async instead of sync in the file name?

Oh, this is previous code. First, I implemented the code with sync API, that is the file you're commenting on.

After a few tests, Charles told me that sync API usually reaches the 30s timeout, so I better to use the async version.

Which has been currently implemented in app/components/playground/ExtractKeyValuePairContainer.tsx#136~197. I didn't extract that logic to a separated function because I've seen we have similar code in the other modules.

So the sync API has not being used, I'm still preserving this file is because it seems the other pages are using this logic as some retry mechanism.

Reply to your comment:

No, I didn't mix the async and sync code up.

I preserved this file for later use. Although I seem better to mention this in my PR description. I'll update my PR, so this file is more reasonable to you later today.

Yes. our current backend is not very well implemented using AWS API gateway which has a tight 30 seconds timeout for sync api.

lingjiekong · 2024-12-29T19:18:33Z

@remi-guan , I am not a frontend expert, could you please take this opportunity to explain regarding what are the changes in each file, so future developer will have a sense on how to add similar frontend features in the future.

remi-guan · 2024-12-30T01:39:09Z

@remi-guan Good progress on implementing this feature and I tested after pulling your code locally and still have couple problems that I want you to work with @boqiny to resolve.

@remi-guan make sure you add a bit more details regarding what is the backend api that you are integrating with, so it ease the review for the backend engineers in the future.

@remi-guan after extracting key value is done, I move back to the file by clicking "back to file", I feel we miss a feature to move to the extract key value without re-rerunning it again by clicking "extract key-value". I feel we can add a "back to key-value result"-ish button without re-running.

@boqiny it looks like the quota update is not right and no matter the page of files, we are always decrement 1 page.

@boqiny what is the extract key value strategy for now?

@lingjiekong

Okay, I'll edit my PR to include that details.
Sounds good, it's easy to add, I can implement that feature today.

remi-guan · 2024-12-30T03:17:40Z

remi-guan · 2024-12-30T12:41:50Z

The new commits:

Add a button to back to result after back to file.
Fix an issue of the extraction result was conflicted with the another modules.
Refactor the code of state management, approximately removed 50 lines and bringing better logic structure and code quality.
Reformat the code with prettier to pass the lint.

Copilot reviewed 7 out of 21 changed files in this pull request and generated no comments.

Files not reviewed (14)

.vscode/settings.json: Language not supported
app/globals.css: Language not supported
package.json: Language not supported
test-results.json: Language not supported
test-results/.last-run.json: Language not supported
app/components/playground/ActionContainer.tsx: Evaluated as low risk
app/components/playground/KeyValueInputs.tsx: Evaluated as low risk
app/actions/apiInterface.ts: Evaluated as low risk
app/actions/runSyncExtractKeyValue.ts: Evaluated as low risk
app/actions/uploadFile.ts: Evaluated as low risk
app/components/Button.tsx: Evaluated as low risk
README.md: Evaluated as low risk
app/components/playground/ExtractKeyValuePairContainer.tsx: Evaluated as low risk
app/hooks/usePlaygroundStore.ts: Evaluated as low risk

lingjiekong

LGTM.

remi-guan added 6 commits December 27, 2024 20:26

feat: Key-value extraction

bd052f7

feat: Tutorial implemented for key value extraction

21f26b4

fix: Key-value extraction UI and backend

016d60f

feat: Use async API, save input and result into store

60aaef0

fix: Seprated input status when file switching

f8f4a6e

fix: Multiple loading toast if tab switched when extracting

1a6bd94

remi-guan requested review from goldmermaid and lingjiekong as code owners December 28, 2024 14:25

lingjiekong requested a review from Copilot December 29, 2024 14:20

Copilot AI reviewed Dec 29, 2024

View reviewed changes

lingjiekong requested a review from boqiny December 29, 2024 19:01

lingjiekong reviewed Dec 29, 2024

View reviewed changes

remi-guan added 2 commits December 30, 2024 20:28

feat: Back to file button after result generated

7d69954

refactor: Format with prettier

f78c09c

lingjiekong requested review from Copilot and lingjiekong and removed request for boqiny December 31, 2024 04:19

Copilot AI reviewed Dec 31, 2024

View reviewed changes

lingjiekong approved these changes Dec 31, 2024

View reviewed changes

lingjiekong merged commit eb678e7 into main Dec 31, 2024
2 checks passed

remi-guan self-assigned this Jan 1, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Key-value extraction #320

feat: Key-value extraction #320

remi-guan commented Dec 28, 2024 •

edited

Loading

lingjiekong commented Dec 29, 2024

lingjiekong commented Dec 29, 2024

lingjiekong commented Dec 29, 2024

Description

Related Issue

Type of Change

How Has This Been Tested?

Screenshots (if applicable)

Checklist

Additional Notes

lingjiekong commented Dec 29, 2024

lingjiekong Dec 29, 2024

lingjiekong Dec 29, 2024

remi-guan Dec 30, 2024

lingjiekong Dec 31, 2024

lingjiekong commented Dec 29, 2024

remi-guan commented Dec 30, 2024

remi-guan commented Dec 30, 2024

Description

Related Issue

Type of Change

How Has This Been Tested?

Screenshots (if applicable)

Checklist

Additional Notes

remi-guan commented Dec 30, 2024

lingjiekong left a comment

feat: Key-value extraction #320

feat: Key-value extraction #320

Conversation

remi-guan commented Dec 28, 2024 • edited Loading

Description

Code Structure

API Integration

Related Issue

Type of Change

How Has This Been Tested?

Screenshots (if applicable)

Checklist

Additional Notes

Choose a reason for hiding this comment

lingjiekong commented Dec 29, 2024

lingjiekong commented Dec 29, 2024

lingjiekong commented Dec 29, 2024

Description

Related Issue

Type of Change

How Has This Been Tested?

Screenshots (if applicable)

Checklist

Additional Notes

lingjiekong commented Dec 29, 2024

lingjiekong Dec 29, 2024

Choose a reason for hiding this comment

lingjiekong Dec 29, 2024

Choose a reason for hiding this comment

remi-guan Dec 30, 2024

Choose a reason for hiding this comment

lingjiekong Dec 31, 2024

Choose a reason for hiding this comment

lingjiekong commented Dec 29, 2024

remi-guan commented Dec 30, 2024

remi-guan commented Dec 30, 2024

Description

Related Issue

Type of Change

How Has This Been Tested?

Screenshots (if applicable)

Checklist

Additional Notes

remi-guan commented Dec 30, 2024

Choose a reason for hiding this comment

lingjiekong left a comment

Choose a reason for hiding this comment

remi-guan commented Dec 28, 2024 •

edited

Loading