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

Use openai categorize products #164

Merged
merged 10 commits into from
Jun 29, 2024
Merged

Conversation

jamalsoueidan
Copy link
Owner

@jamalsoueidan jamalsoueidan commented Jun 28, 2024

PR Type

Enhancement, Dependencies, Tests


Description

  • Implemented new Azure function openaiProductCategorize for product categorization.
  • Added openai package to dependencies in package.json.
  • Modified ScheduleProductZodSchema to replace parentId with collectionIds array.
  • Updated ProductSchema in Mongoose to reflect changes to collectionIds.
  • Adjusted getProductObject helper in Jest to use collectionIds and title.
  • Extended GraphQL types with CollectionFragmentFragment and CollectionsQuery.
  • Added new codegen script to package.json.

Changes walkthrough 📝

Relevant files
Tests
5 files
add.spec.ts
Update product creation tests to remove `parentId`.           

src/functions/customer/controllers/product/add.spec.ts

  • Removed parentId from product creation tests.
+0/-1     
create-user-collection.spec.ts
Update tests to reflect createUserCollection renaming.     

src/functions/customer/orchestrations/customer/create/create-user-collection.spec.ts

  • Renamed createCollection to createUserCollection in tests.
+6/-3     
update-product.spec.ts
Add tests for OpenAI product categorization.                         

src/functions/customer/orchestrations/product/update/update-product.spec.ts

  • Mocked OpenAIServiceProductCategorize.
  • Added tests for product categorization using OpenAI.
  • +39/-8   
    add.spec.ts
    Update product addition tests to remove parentId.               

    src/functions/customer/services/product/add.spec.ts

    • Removed parentId from product addition tests.
    +1/-8     
    update.spec.ts
    Update product update tests to remove parentId.                   

    src/functions/customer/services/product/update.spec.ts

    • Removed parentId from product update tests.
    +0/-1     
    Enhancement
    13 files
    add.ts
    Update product creation schema to remove `parentId`.         

    src/functions/customer/controllers/product/add.ts

    • Removed parentId from product creation schema.
    +0/-1     
    create.ts
    Rename createCollection to createUserCollection in orchestrations.

    src/functions/customer/orchestrations/customer/create.ts

    • Renamed createCollection to createUserCollection.
    +7/-7     
    create-user-collection.ts
    Rename createCollection to createUserCollection.                 

    src/functions/customer/orchestrations/customer/create/create-user-collection.ts

    • Renamed createCollection to createUserCollection.
    +2/-2     
    update-article.ts
    Replace parentId with collectionIds in article update logic.

    src/functions/customer/orchestrations/customer/update/update-article.ts

    • Replaced parentId with collectionIds in article update logic.
    +4/-4     
    update-product.ts
    Integrate OpenAI service for product categorization.         

    src/functions/customer/orchestrations/product/update/update-product.ts

  • Integrated OpenAI service for product categorization.
  • Removed logic related to parentId.
  • +19/-61 
    add.ts
    Update product addition logic to remove parentId.               

    src/functions/customer/services/product/add.ts

    • Removed parentId from product addition logic.
    +3/-4     
    openai-product.function.ts
    Add Azure function for OpenAI product categorization.       

    src/functions/openai-product.function.ts

    • Added new Azure function for OpenAI product categorization.
    +10/-0   
    product.ts
    Add controller for OpenAI product categorization.               

    src/functions/openai/controllers/product.ts

    • Added controller for OpenAI product categorization.
    +23/-0   
    product-categorize.ts
    Add service for product categorization using OpenAI.         

    src/functions/openai/services/product-categorize.ts

    • Added service for product categorization using OpenAI.
    +99/-0   
    schedule.types.ts
    Replace parentId with collectionIds in schedule product schema.

    src/functions/schedule/schedule.types.ts

  • Replaced parentId with collectionIds in schedule product schema.
  • +2/-2     
    product.schema.ts
    Replace parentId with collectionIds in product schema.     

    src/functions/schedule/schemas/product.schema.ts

    • Replaced parentId with collectionIds in product schema.
    +5/-4     
    product.ts
    Update product helper to use collectionIds and title.       

    src/library/jest/helpers/product.ts

    • Updated product helper to use collectionIds and title.
    +3/-2     
    admin.generated.d.ts
    Add GraphQL types for collections.                                             

    src/types/admin.generated.d.ts

    • Added GraphQL types for collections.
    +14/-0   
    Dependencies
    2 files
    package-lock.json
    Add openai package to dependencies.                                           

    package-lock.json

    • Added openai package to dependencies.
    +81/-3   
    package.json
    Add openai package and codegen script.                                     

    package.json

    • Added openai package to dependencies.
    • Added codegen script.
    +3/-1     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

     - Implement new Azure function `openaiProductCategorize` for product categorization
     - Add `openai` package to dependencies in `package.json`
     - Modify `ScheduleProductZodSchema` to replace `parentId` with `collectionIds` array
     - Update `ProductSchema` in Mongoose to reflect changes to `collectionIds`
     - Adjust `getProductObject` helper in Jest to use `collectionIds` and `title`
     - Extend GraphQL types with `CollectionFragmentFragment` and `CollectionsQuery`
     - Add new `codegen` script to `package.json`
    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5] 4
    🧪 Relevant tests Yes
    🔒 Security concerns No
    ⚡ Key issues to review Possible Bug:
    The renaming of createCollection to createUserCollection in various files might cause issues if not all references were updated. Ensure all instances where the old function was called are properly updated to avoid runtime errors.
    Data Consistency:
    The removal of parentId and introduction of collectionIds across multiple schemas and tests should be thoroughly checked to ensure that data consistency is maintained and that no features relying on parentId are broken.
    Dependency Management:
    The addition of the openai package increases the project's dependency footprint. Ensure that this package is properly maintained and assess its impact on the project's overall performance and security.

    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add error handling for the OpenAI API call to manage failures or unexpected responses

    To ensure robustness and maintainability, add error handling for the OpenAI API call. This
    will help in managing API failures or unexpected responses effectively.

    src/functions/openai/services/product-categorize.ts [55-67]

    -const response = await openai.chat.completions.create({
    -  model: "gpt-4o-2024-05-13",
    -  messages: [
    -    {
    -      role: "system",
    -      content,
    +let response;
    +try {
    +  response = await openai.chat.completions.create({
    +    model: "gpt-4o-2024-05-13",
    +    messages: [
    +      {
    +        role: "system",
    +        content,
    +      },
    +    ],
    +    max_tokens: 300,
    +    response_format: {
    +      type: "json_object",
         },
    -  ],
    -  max_tokens: 300,
    -  response_format: {
    -    type: "json_object",
    -  },
    -});
    +  });
    +} catch (error) {
    +  console.error('OpenAI API call failed:', error);
    +  // Handle error appropriately
    +}
     
    Suggestion importance[1-10]: 10

    Why: Adding error handling for the OpenAI API call is essential for robustness, ensuring that API failures or unexpected responses are managed effectively.

    10
    Add error handling for the asynchronous operation to manage exceptions or rejections

    Consider adding error handling for the asynchronous operation createUserCollection. This
    will ensure that any exceptions or rejections are properly managed and can provide a
    fallback or error message to the calling function.

    src/functions/customer/orchestrations/customer/create.ts [36-41]

    -const collectionMetaobject: Awaited<ReturnType<typeof createUserCollection>> =
    -  yield context.df.callActivity(
    +let collectionMetaobject;
    +try {
    +  collectionMetaobject = yield context.df.callActivity(
         createUserCollectionName,
         activityType<typeof createUserCollection>({
           user,
         })
    +} catch (error) {
    +  console.error('Failed to create user collection:', error);
    +  // Handle error appropriately
    +}
     
    Suggestion importance[1-10]: 9

    Why: Adding error handling for the asynchronous operation is crucial for robustness and reliability, ensuring that any exceptions or rejections are properly managed.

    9
    Update the node-fetch dependency to support newer Node.js versions

    Consider updating the node-fetch dependency to a version that supports Node.js 16 and
    above, as the current version in the dependency tree only supports up to Node.js 14. This
    change will ensure compatibility with newer Node.js versions and maintain the integrity of
    the application.

    package-lock.json [10014]

    -"node-fetch": "^2.6.7",
    +"node-fetch": "^3.0.0",
     
    Suggestion importance[1-10]: 8

    Why: Updating node-fetch to a version that supports newer Node.js versions is important for maintaining compatibility and ensuring the application can run on the latest Node.js environments.

    8
    Align agentkeepalive version with the one specified in openai dependencies to avoid conflicts

    The agentkeepalive dependency version added is higher than the one specified in the openai
    package dependencies. This could lead to version conflicts or unexpected behavior. Align
    the versions to avoid potential issues.

    package-lock.json [4624]

    -"agentkeepalive": "^4.5.0",
    +"agentkeepalive": "^4.2.1",
     
    Suggestion importance[1-10]: 7

    Why: Aligning dependency versions helps prevent conflicts and ensures that the application behaves as expected, although the impact may be minor if the versions are close.

    7
    Update the Node.js version requirement for form-data-encoder to ensure compatibility with other dependencies

    The node engine compatibility for form-data-encoder is set to versions starting from
    12.20, which might not be compatible with other dependencies requiring higher versions.
    Consider setting a higher minimum version to ensure compatibility across all dependencies.

    package-lock.json [6880]

    -"node": ">= 12.20"
    +"node": ">= 14"
     
    Suggestion importance[1-10]: 6

    Why: Ensuring that the Node.js version requirements are consistent across dependencies can help avoid compatibility issues, though this change is more precautionary.

    6
    Best practice
    Replace the beta version of web-streams-polyfill with a stable release

    The web-streams-polyfill version in the formdata-node dependencies is set to a beta
    version. It is recommended to use a stable release version to avoid potential issues with
    features that are not fully tested or documented.

    package-lock.json [6887]

    -"web-streams-polyfill": "4.0.0-beta.3"
    +"web-streams-polyfill": "^4.0.0"
     
    Suggestion importance[1-10]: 9

    Why: Using stable release versions instead of beta versions is a best practice to avoid potential issues with untested or undocumented features, ensuring more reliable and predictable behavior.

    9
    Performance
    Optimize data fetching by using batch operations or optimized queries

    To improve the efficiency of the updateProduct function, consider using a batch operation
    or a more optimized query when fetching locations, especially if the number of locations
    can be large.

    src/functions/customer/orchestrations/product/update/update-product.ts [51-53]

    -const locations = await LocationModel.find({
    -  _id: { $in: product.locations.map((l) => l.location) },
    -}).orFail(
    +const locationIds = product.locations.map((l) => l.location);
    +const locations = await LocationModel.aggregate([
    +  { $match: { _id: { $in: locationIds } } },
    +  { $group: { _id: null, locations: { $push: '$$ROOT' } } }
    +]).then(data => data[0]?.locations || []);
     
    Suggestion importance[1-10]: 8

    Why: Optimizing data fetching can significantly improve performance, especially when dealing with large datasets.

    8
    Enhancement
    Enhance type safety and clarity by adding more detailed type annotations

    It's recommended to include more detailed type annotations for the user parameter in the
    function signature to ensure type safety and clarity in the function's usage.

    src/functions/customer/orchestrations/customer/create/create-user-collection.ts [5-8]

     export const createUserCollection = async ({
       user,
     }: {
    -  user: Pick<User, "username">;
    +  user: Pick<User, "username" | "id" | "email">; // Assuming 'id' and 'email' are also relevant
     }) => {
     
    Suggestion importance[1-10]: 7

    Why: Adding more detailed type annotations improves type safety and clarity, but it is a minor enhancement rather than a critical fix.

    7

    @jamalsoueidan jamalsoueidan merged commit 33c8656 into main Jun 29, 2024
    3 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant