Add chat history feature #1988

fujita-h · 2024-09-25T00:17:55Z

This commit adds the following new files:

HistoryItem: A new component for displaying chat history items.
HistoryPanel: A new component for displaying the chat history panel.
HistoryButton: A new component for opening the chat history.
HistoryProviders: A new module containing history provider classes.

These changes include functionality to support future enhancements to the chat history feature.

The chat history feature is only enabled when you explicitly set the environment variable USE_CHAT_HISTORY.

The history function uses the browser's IndexedDB. I understand that there is also an approach to use CosmosDB, but this requires enabling authentication to identify users, so I decided to start with IndexedDB, which does not depend on authentication.

To make it easier to implement other data stores, I made it easier to implement additional data stores with HistoryProviders.

Related: #977 #1863 #259

Purpose

Does this introduce a breaking change?

When developers merge from main and run the server, azd up, or azd deploy, will this produce an error?
If you're not sure, try it out on an old environment.

[ ] Yes
[x] No

Does this require changes to learn.microsoft.com docs?

This repository is referenced by this tutorial
which includes deployment, settings and usage instructions. If text or screenshot need to change in the tutorial,
check the box below and notify the tutorial author. A Microsoft employee can do this for you if you're an external contributor.

[ ] Yes
[x] No

Type of change

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

Code quality checklist

See CONTRIBUTING.md for more details.

The current tests all pass (python -m pytest).
I added tests that prove my fix is effective or that my feature works
I ran python -m pytest --cov to verify 100% coverage of added lines
I ran python -m mypy to check for type errors
I either used the pre-commit hooks or ran ruff and black manually on my code.

This commit adds the following new files: - HistoryItem: A new component for displaying chat history items. - HistoryPanel: A new component for displaying the chat history panel. - HistoryButton: A new component for opening the chat history. - HistoryProviders: A new module containing history provider classes. These changes include functionality to support future enhancements to the chat history feature.

fujita-h · 2024-09-25T00:20:50Z

@microsoft-github-policy-service agree

pamelafox · 2024-09-25T14:40:37Z

Ooo, very interesting approach with the IndexedDB! It sounds like you've designed it very thoughtfully, I'll take a look soon.

pamelafox · 2024-09-30T23:58:44Z

app/frontend/src/pages/chat/Chat.tsx

                // AI Chat Protocol: Client must pass on any session state received from the server
-                session_state: answers.length ? answers[answers.length - 1][1].session_state : null
+                // Set new UUID as session state for the first request
+                session_state: answers.length ? answers[answers.length - 1][1].session_state : crypto.randomUUID()


Do you think we'd also use this randomUUID for apps with logged in users, or would we use Entra OIDs for them? Am wondering if it'd be better to have a helper function to generate this ID.
And if the history storage was in CosmosDB, would we still generate the UUID on the client? Want to make sure this is forward-compatible if possible.

Thank you for pointing that out.
You're right, it seems we should think about this carefully.

We need to assign an ID to each "series of chats" to identify them, regardless of whether the user is logged in or not. We'll call this sessionId.

In the case of IndexedDB

On the client side (React), sessionId does not necessarily have to be a UUID, as long as it is a unique key within the client side.

There is also no need to exchange sessionId with the server side using session_state (the current implementation of storing sessionId in session_state is in anticipation of expansion to CosmosDB).

In the case of CosmosDB

It is expected that the design will set the user's Entra OID as the partition key and sessionId as the unique key.

In CosmosDB, conversationId does not need to be a UUID, as long as it is unique within the partition.

As mentioned above, sessionId does not need to be a UUID on the client side (React).

sessionId is a key in CosmosDB. Therefore, it is preferable to generate it on the server side rather than on the client side.

As the name session_state suggests, it is natural that the sessionId stored here is generated on the server side.

When chat messages are sent and received, the sessionId needs to be exchanged between the server side and the client side. session_state can be used for this.

Fix Plan

The plan is to summarize these points and fix them in the following plan.

If the request from the client contains session_state, it will be used as sessionId. If not, a new one will be generated on the server and added to session_state.

sessionId does not necessarily need to be UUID, but unless there is a specific reason, there is no major problem with UUID, so we will basically generate it with uuid.uuid4().

Considering the possibility that UUID may be inconvenient when other storage methods are added in the future, we will create a helper function that generates sessionId according to the storage method used.

Thanks for that thorough design discussion, that sounds good!

pamelafox · 2024-10-01T00:00:54Z

app/frontend/package.json

    "i18next": "^23.12.2",
    "i18next-browser-languagedetector": "^8.0.0",
    "i18next-http-backend": "^2.5.2",
+    "idb": "^8.0.0",


This seems like a reasonable addition according to packagephobia and snyk:

https://packagephobia.com/result?p=idb
https://snyk.io/advisor/npm-package/idb

pamelafox · 2024-10-01T00:04:50Z

app/backend/config.py

 CONFIG_SPEECH_SERVICE_LOCATION = "speech_service_location"
 CONFIG_SPEECH_SERVICE_TOKEN = "speech_service_token"
 CONFIG_SPEECH_SERVICE_VOICE = "speech_service_voice"
+CONFIG_CHAT_HISTORY_ENABLED = "chat_history_enabled"


Given that a "chat history" feature means different things to different people, I think it might help for the config and env variable to be more specific.

For example, for the env var:

USE_CHAT_HISTORY_BROWSER

In future, we might have:
USE_CHAT_HISTORY_COSMOS

That'd be similar to how we did speech output.

app/frontend/src/locales/es/translation.json

pamelafox · 2024-10-01T00:06:04Z

app/frontend/src/api/models.ts

    showSpeechInput: boolean;
    showSpeechOutputBrowser: boolean;
    showSpeechOutputAzure: boolean;
+    showChatHistory: boolean;


Suggested change

showChatHistory: boolean;

showChatHistoryBrowser: boolean;

Per my other comment, I think we should be clear about what's enabled.

Changed variable names to make their functions clearer.

Changed to generate session state on the server side.

fujita-h · 2024-10-01T18:12:56Z

I have corrected the points you pointed out, so please review again.
The environment variable to enable this feature has been changed to USE_CHAT_HISTORY_BROWSER.

zedhaque · 2024-10-01T18:30:31Z

Thank you for the PR! 😊 I’m testing it, and it works locally (auth), but I had to make two minor Bicep changes to deploy it to Azure. These changes are:

adding app variables in main.bicep

  ENABLE_LANGUAGE_PICKER: enableLanguagePicker
  USE_CHAT_HISTORY_BROWSER: useChatHistoryBrowser
  USE_SPEECH_INPUT_BROWSER: useSpeechInputBrowser

adding chat history in main.parameters.json

    "enableLanguagePicker": {
      "value": "${ENABLE_LANGUAGE_PICKER=false}"
    },
    "useChatHistoryBrowser": {
      "value": "${USE_CHAT_HISTORY_BROWSER =false}"
    },

fujita-h · 2024-10-02T00:10:19Z

Thank you for pointing that out. I had forgotten about bicep.
ENABLE_LANGUAGE_PICKER and USE_SPEECH_INPUT_BROWSER are outside the scope of this PR, so I have omitted them.

app/frontend/src/locales/fr/translation.json

Co-authored-by: Wassim Chegham <[email protected]>

…-search-openai-demo into feat-chat-history

pamelafox

I'm approving the change, but will await a review of the es translations and will check with @mattgotteiner before merging.

mattgotteiner

approved pending translation review

ibrahimnasir0 · 2024-10-04T18:35:27Z

@fujita-h Where we will be able to see this chat history ?
Can you guide on that

fujita-h · 2024-10-04T18:54:49Z

@ibrahimnasir0 Just set the environment variables as follows:
azd env set USE_CHAT_HISTORY_BROWSER true

ibrahimnasir0 · 2024-10-08T14:36:33Z

Hi @fujita-h
I'm working on integrating Cosmos DB with this updated project to save chat history along with the user OID (since login and Entra ID are enabled). Could you provide some guidance on how to proceed with this?

Add translations for chat history in Spanish and French

f4b0182

pamelafox reviewed Sep 30, 2024

View reviewed changes

pamelafox reviewed Oct 1, 2024

View reviewed changes

Update e2e test

afc8d1b

pamelafox reviewed Oct 1, 2024

View reviewed changes

app/frontend/src/locales/es/translation.json Show resolved Hide resolved

pamelafox reviewed Oct 1, 2024

View reviewed changes

fujita-h added 2 commits October 1, 2024 16:32

Refactor chat history configuration variables

063b64c

Changed variable names to make their functions clearer.

Refactor session state handling

c6fd15c

Changed to generate session state on the server side.

Add support for chat history feature to bicep

99474e8

manekinekko reviewed Oct 2, 2024

View reviewed changes

pamelafox and others added 4 commits October 2, 2024 13:46

Update workflows, docs, typing

88d4d59

Apply suggestions from code review

88c1df4

Co-authored-by: Wassim Chegham <[email protected]>

Revert e2e test change

bc22f15

Merge branch 'feat-chat-history' of https://github.com/fujita-h/azure…

e3caa07

…-search-openai-demo into feat-chat-history

pamelafox approved these changes Oct 3, 2024

View reviewed changes

mattgotteiner approved these changes Oct 3, 2024

View reviewed changes

Merge branch 'main' into feat-chat-history

291943e

pamelafox approved these changes Oct 4, 2024

View reviewed changes

pamelafox merged commit be26d31 into Azure-Samples:main Oct 4, 2024
17 checks passed

fujita-h deleted the feat-chat-history branch October 19, 2024 02:18

fujita-h mentioned this pull request Oct 21, 2024

Feature: Store chat history in Cosmos DB #2063

Merged

5 tasks

Add chat history feature #1988

Add chat history feature #1988

Uh oh!

Conversation

fujita-h commented Sep 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Does this introduce a breaking change?

Does this require changes to learn.microsoft.com docs?

Type of change

Code quality checklist

Uh oh!

fujita-h commented Sep 25, 2024

Uh oh!

pamelafox commented Sep 25, 2024

Uh oh!

pamelafox Sep 30, 2024

Choose a reason for hiding this comment

Uh oh!

fujita-h Oct 1, 2024

Choose a reason for hiding this comment

In the case of IndexedDB

In the case of CosmosDB

Fix Plan

Uh oh!

pamelafox Oct 1, 2024

Choose a reason for hiding this comment

Uh oh!

pamelafox Oct 1, 2024

Choose a reason for hiding this comment

Uh oh!

pamelafox Oct 1, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pamelafox Oct 1, 2024

Choose a reason for hiding this comment

Uh oh!

pamelafox Oct 1, 2024

Choose a reason for hiding this comment

Uh oh!

fujita-h commented Oct 1, 2024

Uh oh!

zedhaque commented Oct 1, 2024

Uh oh!

fujita-h commented Oct 2, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pamelafox left a comment

Choose a reason for hiding this comment

Uh oh!

mattgotteiner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ibrahimnasir0 commented Oct 4, 2024

Uh oh!

fujita-h commented Oct 4, 2024

Uh oh!

ibrahimnasir0 commented Oct 8, 2024

Uh oh!

Uh oh!

fujita-h commented Sep 25, 2024 •

edited

Loading