Add support for externalUserId for chats. #25
Conversation
WalkthroughThe updates enhance chat functionality by integrating an Changes
Poem
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- chat.test.ts (3 hunks)
- chat.ts (9 hunks)
- client.ts (1 hunks)
- cortex.ts (3 hunks)
Files skipped from review due to trivial changes (1)
- client.ts
Additional comments not posted (12)
chat.test.ts (4)
70-70: LGTM! VerifyexternalUserIdhandling incortex.chat.The addition of
externalUserIdto thecortex.chatmethod call looks good.Ensure that the
externalUserIdfield is properly handled in thecortex.chatmethod.
77-77: LGTM!The check for
externalUserIdin thegetChatResobject ensures that the field is correctly returned from thegetChatmethod.
165-165: LGTM! VerifyexternalUserIdhandling incortex.chat.The addition of
externalUserIdto thestatusStreamobject in thestreaming chattest looks good.Ensure that the
externalUserIdfield is properly handled in thecortex.chatmethod.
193-193: LGTM!The check for
externalUserIdin thechatResultobject ensures that the field is correctly returned from thechatmethod.cortex.ts (2)
93-93: LGTM!The addition of
externalUserIdto theCortexCreateChatOptsBaseinterface aligns with the objective of associating external user identifiers with chat operations.
196-196: LGTM! VerifyexternalUserIdhandling inChat.create.The addition of
externalUserIdto thechatmethod in both the streaming and non-streaming cases looks good.Ensure that the
externalUserIdfield is properly handled in theChat.createmethod.Also applies to: 205-205
chat.ts (6)
12-12: LGTM!The addition of
externalUserIdto theCreateChatOptsBaseinterface aligns with the objective of associating external user identifiers with chat operations.
50-50: LGTM!The addition of
externalUserIdto theChatListIteminterface aligns with the objective of associating external user identifiers with chat operations.
61-61: LGTM!The addition of
externalUserIdto theChatListOptsinterface aligns with the objective of associating external user identifiers with chat operations.
78-78: LGTM!The addition of
externalUserIdto theChatclass constructor aligns with the objective of associating external user identifiers with chat operations.
118-118: LGTM! VerifyexternalUserIdhandling increateContentSyncandcreateContentStreaming.The addition of
externalUserIdto thecreateContentSyncandcreateContentStreamingmethods looks good.Ensure that the
externalUserIdfield is properly handled in thecreateContentSyncandcreateContentStreamingmethods.Also applies to: 125-125
209-211: LGTM! VerifyexternalUserIdhandling in thelistmethod andChatListItemobject.The addition of
externalUserIdto the query parameters in thelistmethod and to theChatListItemobject looks good.Ensure that the
externalUserIdfield is properly handled in thelistmethod and theChatListItemobject.Also applies to: 226-226
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- chat.test.ts (3 hunks)
- chat.ts (9 hunks)
- client.ts (1 hunks)
- cortex.ts (3 hunks)
Files skipped from review as they are similar to previous changes (4)
- chat.test.ts
- chat.ts
- client.ts
- cortex.ts
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- chat.test.ts (3 hunks)
- chat.ts (10 hunks)
- client.ts (1 hunks)
- cortex.ts (3 hunks)
Files skipped from review as they are similar to previous changes (3)
- chat.test.ts
- client.ts
- cortex.ts
Additional comments not posted (6)
chat.ts (6)
12-12: Addition ofexternalUserIdtoCreateChatOptsBaselooks good.The
externalUserIdfield is correctly added as an optional string.
50-50: Addition ofexternalUserIdtoChatListItemlooks good.The
externalUserIdfield is correctly added as an optional string.
61-61: Addition ofexternalUserIdtoChatListOptslooks good.The
externalUserIdfield is correctly added as an optional string or null.
78-78: Addition ofexternalUserIdtoChatconstructor looks good.The
externalUserIdfield is correctly added as an optional string.
98-103: Inclusion ofexternalUserIdincreateContentSynclooks good.The
externalUserIdfield is correctly destructured from theoptsobject and included in the POST request payload.
Line range hint
129-172:
Inclusion ofexternalUserIdincreateContentStreaminglooks good.The
externalUserIdfield is correctly destructured from theoptsobject, included in the POST request payload, and used in theChatconstructor.
I thought not, since we don't currently support changing users in the middle of a chat. At some point we should consider implementing forking for chats, but we don't have that right now. |
|
That sounds right. Thanks for refreshing my memory. |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- chat.ts (10 hunks)
- client.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- client.ts
Additional comments not posted (9)
chat.ts (9)
12-12: Addition ofexternalUserIdinCreateChatOptsBaseis correct.The addition aligns with the PR's objective and is appropriately marked as optional.
50-50: Addition ofexternalUserIdinChatListItemis correct.The addition aligns with the PR's objective and is appropriately marked as optional.
61-61: Verify the necessity of| nullinexternalUserIdtype definition.The addition aligns with the PR's objective and is appropriately marked as optional. However, consider if
| nullis necessary, asundefinedmight suffice.
78-78: Addition ofexternalUserIdinChatclass constructor is correct.The addition aligns with the PR's objective and is appropriately marked as optional.
98-103: Addition ofexternalUserIdincreateContentSyncmethod is correct.The addition aligns with the PR's objective and is appropriately included in the request body.
129-134: Addition ofexternalUserIdincreateContentStreamingmethod is correct.The addition aligns with the PR's objective and is appropriately included in the request body.
164-172: Addition ofexternalUserIdinChatinstantiation withincreateContentStreamingis correct.The addition aligns with the PR's objective and is appropriately included in the
Chatobject instantiation.
191-191: Addition ofexternalUserIdinChatinstantiation withingetmethod is correct.The addition aligns with the PR's objective and is appropriately included in the
Chatobject instantiation.
213-215: Addition ofexternalUserIdinlistmethod is correct.The addition aligns with the PR's objective and is appropriately included in the query parameters and the
ChatListIteminstantiation.Also applies to: 230-230
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- indexers/shopify-indexer.ts (1 hunks)
Additional comments not posted (1)
indexers/shopify-indexer.ts (1)
84-84: Approve the conversion ofidto string.The conversion of the
idproperty to a string ensures consistent data types within thedocumentsarray. This change is straightforward and should not have any adverse side effects. However, verify if this conversion is necessary for all use cases and ensure that all parts of the codebase that interact with this data are compatible with this change.
Summary by CodeRabbit
externalUserIdin various chat functionalities including chat creation, listing, and responses.idproperty format by convertingitem.idto a string in Shopify indexer.