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

feat: add receipt data structure, receipt repository, and get receipts data #8

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

4shb0rne
Copy link
Contributor

@4shb0rne 4shb0rne commented Mar 9, 2025

Adding basic structure for receipt, receipt repository (to fetch data from persistance storage), and slash command for getting all receipt data in that specific channel
image

Issue : #6

Note :

  • Currently receipt structure save receiptDate data in string since i have concern where there is different format of a date like DD/MM/YYYY and MM/DD/YYYY (can update this later)

Demo :
https://youtu.be/FTD9vMYv-mQ

@4shb0rne 4shb0rne force-pushed the feat/save-receipt-data branch from a9dd259 to f32c9bb Compare March 9, 2025 14:36
Copy link
Member

@Sing-Li Sing-Li left a comment

Choose a reason for hiding this comment

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

few comments above - to make it more generic allowing for freeform evolution of the app.

@@ -61,6 +61,7 @@ yarn install

<h2>Usage 💬</h2>

- **`/receipt list`**: Show list of receipt data in specific channel
Copy link
Member

Choose a reason for hiding this comment

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

excellent work! Thanks for your contributions. We can definitely up-level this project for 2025.

3. DO NOT ADD ANY METADATA OR EXTRA FIELDS.
4. ENSURE THE JSON IS PARSEABLE.
5. MUST START WITH { AND END WITH }
export const RECEIPT_SCAN_PROMPT = `
Copy link
Member

@Sing-Li Sing-Li Mar 9, 2025

Choose a reason for hiding this comment

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

kudos! Separating the prompts is the most important thing to do in "prompt engineering heavy" project like this one. We will end up with bits and pieces of prompts everywhere if not careful and the project will become unmanageable over time.

This is why we have #7

Can you please modify this file to become more of a "prompt library" and/or "prompt repository" as specified above. You don't have to add any of the other cases. Just put these existing prompts into a structure as described so you call something like PromptLib.getReceiptScanPrompt() (just a suggestion, use your own naming) to get the RECEIPT_SCAN_PROMPT etc. And then add one or two unit tests. This will start a basis that other can readily contribute to 🙏

Be sure to separate the set of prompts by model (name including version, num parameters, quantization) in the implementation. This will allow the app to migrate to different but similar models without code rewrite -- just testing with a variation and then update the prompts.

A "bonus idea" is to implement some sort of "prompt engineers dev-mode" where the prompts are persisted at runtime and then made modifiable in the App Settings, allowing the end admin/user to customize their own prompt for the llm that they may be using. (but pls don't do it for this PR)

}
`

export const RECEIPT_VALIDATION_PROMPT = `
You are an OCR system that determines whether an uploaded image is a **RECEIPT** or not. Your response must follow these strict rules.
Copy link
Member

Choose a reason for hiding this comment

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

brilliant! I think something like this can become prompt-engineering best practices.

@@ -0,0 +1,14 @@
export const INVALID_IMAGE_RESPONSE =
Copy link
Member

Choose a reason for hiding this comment

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

good enough set of exceptions handled. contributors can always add to this as the project evolve.

@@ -0,0 +1,51 @@
import {
Copy link
Member

@Sing-Li Sing-Li Mar 9, 2025

Choose a reason for hiding this comment

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

I don't know what to say here.

I think the implementation is good enough for this specific purpose.

But on the other hand, we MUST layer this so the app can grow without having to rewrite all of this.

It is the entire reason why we have:

#6

I think I need to ask you to add shallow implementation of the following:
- separate getbyUserXXXXX onto another top layer (since app does server-wide handling on user's behalf)
- add some sort of chained (index) operation - such as getReceiptsFromRoomOrderedByScannedReceiptDate (again, pls don't use these my verbose naming 🙏 ) as a basis for other contributors to build on as we better define what the "schema" + "indexes" should really be in this app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants