Implemented Automatic Detection of chip counts and colors # 7 #25

2025-02-21T14:29:13-08:00

Vutukuri15 commented

2025-02-21 14:29:13 -08:00

(Migrated from github.com)

This PR introduces an image recognition feature that allows users to take or upload a photo of their poker chips. The system detects chip colors and counts, displaying the detected values in an editable format for verification.

MantashaNoyela (Migrated from github.com) reviewed 2025-02-21 14:29:13 -08:00

djwesty (Migrated from github.com) requested changes 2025-02-21 17:51:27 -08:00

djwesty (Migrated from github.com) left a comment

Great work. This was a tricky feature.

Ping me for another look after the tests are added.

Great work. This was a tricky feature. Ping me for another look after the tests are added.

components/ChipDetection.tsx

djwesty (Migrated from github.com) commented

2025-02-21 17:33:04 -08:00

I think it would be good to make this an env variable, like the API key.
That way we can change the model on-the-fly easier

You can resolve this comment when done

I think it would be good to make this an `env` variable, like the API key. That way we can change the model on-the-fly easier You can resolve this comment when done

djwesty (Migrated from github.com) commented

2025-02-21 17:43:47 -08:00

Now that we have environment variables using dotenv, developers need to be aware of this so they can build and run the app locally. Since you may not have done this before, here is my suggestion

Create and commit a file at the root called .env.example This file will might look similar to

API_KEY={Put Open AI key here}
GPT_MODEL=gpt-4o-mini
#GPT_MODEL=gpt-4-turbo # More expensive model, use sparingly

The purpose here, is we force the developer to enter their secret key which is not committed, but also make it easy to uncomment/switch models.

Update the README.md with a section under the build/run, for how to populate .env. Essentially we instruct the developer to copy the .env.example with the new name .env (cp .env.example .env) and to paste their api key.

You can resolve this comment when done

Now that we have environment variables using `dotenv`, developers need to be aware of this so they can build and run the app locally. Since you may not have done this before, here is my suggestion 1. Create and commit a file at the root called `.env.example` This file will might look similar to ``` API_KEY={Put Open AI key here} GPT_MODEL=gpt-4o-mini #GPT_MODEL=gpt-4-turbo # More expensive model, use sparingly ``` The purpose here, is we force the developer to enter their secret key which is not committed, but also make it easy to uncomment/switch models. 2. Update the README.md with a section under the build/run, for how to populate `.env`. Essentially we instruct the developer to copy the `.env.example` with the new name `.env` (`cp .env.example .env`) and to paste their api key. You can resolve this comment when done

components/__tests__/ChipDetection.test.tsx

djwesty (Migrated from github.com) commented

2025-02-21 17:32:05 -08:00

I stubbed these tests out before we traded issues, but now would be the time to implement some tests.

Tag me for here for another look when done

I stubbed these tests out before we traded issues, but now would be the time to implement some tests. Tag me for here for another look when done

package.json

djwesty (Migrated from github.com) commented

2025-02-21 17:48:29 -08:00

Can you give these new package additions a second look and confirm that we actually need them all.
These one, for example, do not seemed to be referenced by name in any components.
Sometimes that happens, but often it can suggest a package was added during development, but later not needed and should be removed again

You can resolve this comment when done

Can you give these new package additions a second look and confirm that we actually need them all. These one, for example, do not seemed to be referenced by name in any components. Sometimes that happens, but often it can suggest a package was added during development, but later not needed and should be removed again You can resolve this comment when done