feat: Use existing Knowledge Base #638

fsatsuki · 2024-12-10T01:46:57Z

Issue #, if available:
None

Description of changes:
You can also use a manually created knowledge base or another chatbot knowledge base.
This PR depends on #607

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

axelpina · 2024-12-10T14:07:41Z

Hi,

There's a typo in has_exist_knowlednge_base_id -> has_exist_knowledge_base_id.

statefb

今回もコメントが多く申し訳ありません。ご確認お願いします！

statefb · 2024-12-12T04:36:11Z

backend/embedding_statemachine/bedrock_knowledge_base/store_knowledge_base_id.py

このファイルのdiffにはどのような意図がありますか？

CDKの方でCfnOutputの出力順序を変更しています。
新規にKBを作成するルートと、既存のKBを使うルートで処理の分岐が発生し、CfnOutputの順番が変動するためです。
そのため、常に配列の0番目を取得する stack_output[0] では目的の値が取れなくなったためコードを変更しています。

statefb · 2024-12-12T04:36:52Z

backend/tests/test_repositories/test_conversation.py

        self.assertEqual(
-            content[1].body,
+            base64.b64encode(content[1].body).decode(),


このdiffにはどのような意図がありますか？

この処理を入れることでエラーが解消されます。

[error log]

bash-5.2$ cd /projects/bedrock-claude-chat/backend && poetry run python tests/test_repositories/test_conversation.py ..F. ====================================================================== FAIL: test_store_and_find_conversation (__main__.TestConversationRepository.test_store_and_find_conversation) ---------------------------------------------------------------------- Traceback (most recent call last): File "/projects/bedrock-claude-chat/backend/tests/test_repositories/test_conversation.py", line 302, in test_store_and_find_conversation self.assertEqual( AssertionError: b"\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00[155 chars]\x82" != 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1[48 chars]YII=' ---------------------------------------------------------------------- Ran 4 tests in 0.024s

statefb · 2024-12-12T05:17:46Z

backend/tests/test_usecases/test_chat.py

internet toolおよびconverse apiをモックされている理由を教えていただけますか？スロットリング制限回避のためでしょうか。

すいません、これ削除します。

statefb · 2024-12-12T05:20:34Z

backend/tests/test_usecases/utils/bot_factory.py

@@ -26,6 +33,22 @@ def create_test_private_bot(
    bedrock_knowledge_base=None,
    bedrock_guardrails=None,
 ):
+    if bedrock_knowledge_base is None:


BotModel.bedrock_knowledge_baseはNone許容だと思いますが、ここでNoneを回避し作成している意図を教えていただけますか？

テストコードのデバッグ中に試行錯誤した時のものでした。
テストが通るようになっているため、元に戻します。

statefb · 2024-12-12T05:30:10Z

cdk/lib/constructs/embedding.ts

-      startIngestionJob.next(getIngestionJob).next(checkIngestionJobStatus)
+      new sfn.Choice(this, "CheckDataSourceId")
+        .when(
+          sfn.Condition.not(sfn.Condition.stringEquals("$.DataSourceId", "")),


DataSourceIDの取得により分岐をなくせないか検討ください！

Sfnの中で流通させるデータ構造を変更することで実装できました。

statefb · 2024-12-12T05:55:47Z

backend/app/usecases/bot.py

@@ -548,6 +556,7 @@ def fetch_all_bots_by_user_id(
                        sync_status=bot.sync_status,
                        has_knowledge=bot.has_knowledge(),
                        has_agent=bot.is_agent_enabled(),
+                        has_exist_knowledge_base_id=bot.has_exist_knowledge_base_id(),


alias変更不要？

has_exist_knowledge_base_idを削除

statefb · 2024-12-12T05:58:57Z

backend/app/usecases/bot.py

@@ -641,6 +650,7 @@ def fetch_bot_summary(user_id: str, bot_id: str) -> BotSummaryOutput:
            owned=True,
            sync_status=bot.sync_status,
            has_knowledge=bot.has_knowledge(),
+            has_exist_knowledge_base_id=bot.has_exist_knowledge_base_id(),


BotSummaryにhas_exist_knowledge_base_idは不要、has_knowledgeで十分かと思いました

has_exist_knowledge_base_idを削除

statefb · 2024-12-12T06:00:14Z

backend/app/usecases/chat.py

@@ -266,7 +267,7 @@ def chat(
            if display_citation:
                instructions.append(PROMPT_TO_CITE_TOOL_RESULTS)

-        elif bot.has_knowledge():
+        elif bot.has_knowledge() or bot.has_exist_knowledge_base_id():


（ここで良いのかわかりませんが）has_knowledgeはメソッドの意味合い的にkb_idを保持するかどうかも包含すべきかなと思います。

has_exist_knowledge_base_idを削除
def has_knowledge() を変更して、既存のKBを使う時もTrueを返すように変更します。

statefb · 2024-12-12T06:00:52Z

backend/embedding_statemachine/bedrock_knowledge_base/fetch_stack_output.py

@@ -38,12 +39,12 @@ def handler(event, context):
        elif output["OutputKey"] == "GuardrailVersion":
            guardrail_version = output["OutputValue"]

-    result = []
-    for data_source_id in data_source_ids:
+    # If no data sources exist, create a result with just the KnowledgeBaseId


DataSource分岐不要？(BotStack側のコメント参照)

削除できました。

statefb · 2024-12-12T06:03:09Z

cdk/lib/bedrock-custom-bot-stack.ts

+      kb = KnowledgeBase.fromKnowledgeBaseAttributes(this, 'MyKnowledgeBase', {
+        knowledgeBaseId: props.existKnowledgeBaseId,
+        executionRoleArn: executionRoleArn
+      })


DataSourceを取得しStackOutuputするようにすれば、他の分岐がなくなり可読性が高くなるかと思いますが、いかがでしょううか？

（動作確認していませんが）たとえば下記のように取得し、fromDataSourceIDなどのメソッドを利用し実装可能かどうかご検討ください

const customResource = new AwsCustomResource(stack, 'ListDataSources', { onCreate: { service: 'Bedrock', action: 'listDataSources', parameters: { knowledgeBaseId: 'your-knowledge-base-id', }, physicalResourceId: AwsCustomResource.PhysicalResourceId.of('ListDataSources'), }, policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: AwsCustomResourcePolicy.ANY_RESOURCE }), }); const dataSourceIds = customResource.getResponseField('dataSources');

Sfnの中で流通させるデータ構造を変更することで条件分岐をなくすことができました。

fsatsuki added 7 commits December 6, 2024 06:25

wip

dba936b

wip

556d9e7

Merge branch 'v2' into existing_kb

30fa481

Merge branch 'issue-592' into existing_kb

90fd22e

wip

1620e42

disable

abd1146

fix ci and test code

55dcd32

fsatsuki requested review from statefb, wadabee and Yukinobu-Mine as code owners December 10, 2024 01:46

modify cdk test code

7fcc2f8

fsatsuki and others added 2 commits December 11, 2024 00:46

has_exist_knowlednge_base_id -> has_exist_knowledge_base_id.

bf67036

Merge branch 'v2' into existing_kb

9e572e1

statefb reviewed Dec 12, 2024

View reviewed changes

statefb self-assigned this Dec 12, 2024

fsatsuki added 7 commits December 12, 2024 06:48

knowledge_base_idとexist_knowledge_base_idが排他であることを強制する

1977cd6

review comment対応

06529d8

コメント対応

3752bbd

不要な修正を削除, CIエラー解消

628bff6

Sfnの処理で使うデータ構造を修正

c83f659

UI変更

19d42db

UI変更

eecf6d8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Use existing Knowledge Base #638

feat: Use existing Knowledge Base #638

fsatsuki commented Dec 10, 2024

axelpina commented Dec 10, 2024

statefb left a comment

statefb Dec 12, 2024

fsatsuki Dec 12, 2024

statefb Dec 12, 2024

fsatsuki Dec 12, 2024

statefb Dec 12, 2024

fsatsuki Dec 13, 2024

statefb Dec 12, 2024

fsatsuki Dec 12, 2024

statefb Dec 12, 2024

fsatsuki Dec 13, 2024

statefb Dec 12, 2024

fsatsuki Dec 12, 2024

statefb Dec 12, 2024

fsatsuki Dec 12, 2024

statefb Dec 12, 2024

fsatsuki Dec 12, 2024

statefb Dec 12, 2024

fsatsuki Dec 12, 2024

fsatsuki Dec 13, 2024

statefb Dec 12, 2024

fsatsuki Dec 13, 2024

feat: Use existing Knowledge Base #638

Are you sure you want to change the base?

feat: Use existing Knowledge Base #638

Conversation

fsatsuki commented Dec 10, 2024

axelpina commented Dec 10, 2024

statefb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment