Knowledge #241

StrongMonkey · 2024-10-18T21:47:17Z

Knowledge UX revamp:
This includes

File based approved
New UX on how to add files through files/onedrive/notion/websites

Signed-off-by: Daishan Peng <[email protected]>

thedadams · 2024-10-19T03:20:14Z

pkg/api/handlers/agent.go

+		return err
+	}
+
+	file.Spec.Approved = &[]bool{body.Approve}[0]


Suggested change

file.Spec.Approved = &[]bool{body.Approve}[0]

file.Spec.Approved = &body.Approve

thedadams · 2024-10-19T03:21:09Z

pkg/api/handlers/agent.go

+
+	file.Spec.Approved = &[]bool{body.Approve}[0]
+
+	return req.Storage.Update(req.Context(), &file)


Can we do a check here to see if we actually need to update?

thedadams · 2024-10-19T03:33:45Z

pkg/controller/handlers/knowledge/knowledge.go

@@ -288,6 +303,15 @@ func compileFileStatuses(ctx context.Context, client kclient.Client, ws *v1.Work
 	return final, errors.Join(errs...)
 }

+func (a *Handler) BindWorkspace(req router.Request, resp router.Response) error {


This will cause the file to be re-enqueued when the workspace changes, which seems to be the opposite of what the comment indicates.

However, the workspace handler already lists the files, which would already have the desired effect. So, I think this can be removed.

Yeah, I have been seeing issue where new file added doesn't trigger an ingestion. But haven't seen it anymore. Removed.

thedadams · 2024-10-19T03:35:28Z

pkg/controller/handlers/uploads/remoteknowledgesource.go

@@ -362,6 +367,7 @@ func compileKnowledgeFilesFromOneDriveConnector(ctx context.Context, c client.Cl
 		} else if err != nil {
 			errs = append(errs, err)
 		}
+


nit: remove line.

ryanhopperlowe · 2024-10-21T16:39:05Z

Getting this error whenever I open the knowledge accordion in /admin/agents/<id>

EDIT: this is fixed

tylerslaton · 2024-10-21T17:21:04Z

Getting this error whenever I open the knowledge accordion in /admin/agents/<id>

Same here.

ryanhopperlowe · 2024-10-21T16:33:08Z

ui/admin/app/components/knowledge/FileItem.tsx

                            </TypographyP>
                        </div>

-                        {statusIcon}
+                        {isApproved ? (
+                            // eslint-disable-next-line


what is this ignoring?

When disabling eslint can you specify the rule so you don't accidentally conceal other breakages?

Fixed by using a button

ryanhopperlowe · 2024-10-21T16:40:26Z

ui/admin/app/components/knowledge/file/FileModal.tsx

+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    getKnowledgeFiles: any;


2 things here:

use Todo instead of any

can you add a type here?

ui/admin/app/components/knowledge/FileItem.tsx

ryanhopperlowe · 2024-10-21T16:46:38Z

ui/admin/app/components/knowledge/FileItem.tsx

+                            <div
+                                className="hover:cursor-pointer"
+                                onClick={() => {
+                                    setIsApproved(false);
+                                    approveFile(file, false);
+                                }}
+                            >
+                                <FileStatusIcon status={file.ingestionStatus} />
+                            </div>


We shouldn't ever create a "clickable" div (it breaks accessibility features). Instead use <Button variant="link"> or <Button variant="ghost">

ui/admin/app/components/knowledge/IngestionStatus.tsx

ryanhopperlowe · 2024-10-21T17:16:11Z

ui/admin/app/components/knowledge/notion/NotionModal.tsx

+                                    ]?.title
+                                }
+                                remoteKnowledgeSourceType={
+                                    item.remoteKnowledgeSourceType!


It is ganranteed the type will never be empty because we already filter the files by checking if `item.remoteKnowledgeSourceType== 'notion`` so this is just bypassing type check

ryanhopperlowe · 2024-10-21T17:16:18Z

ui/admin/app/components/knowledge/notion/NotionModal.tsx

+                                approveFile={async (file, approved) => {
+                                    await KnowledgeService.approveKnowledgeFile(
+                                        agentId,
+                                        file.id!,


actually I made it required, it should always be there

can you still remove the !

Sorry, forgot to remove it.

ryanhopperlowe · 2024-10-21T17:17:37Z

ui/admin/app/components/knowledge/onedrive/AddLinkModal.tsx

+        <Dialog open={isOpen} onOpenChange={onOpenChange}>
+            <DialogContent
+                aria-describedby={undefined}
+                className="bd-secondary data-[state=open]:animate-contentShow fixed top-[50%] left-[50%] max-h-[85vh] w-[90vw] max-w-[400px] translate-x-[-50%] translate-y-[-50%] rounded-[6px] bg-white dark:bg-secondary p-[25px] shadow-[hsl(206_22%_7%_/_35%)_0px_10px_38px_-10px,_hsl(206_22%_7%_/_20%)_0px_10px_20px_-15px] focus:outline-none"


why do we need to override the way dialogs look here?

generated by ai :)

can you remove it then? we want things to stay consistent

ryanhopperlowe · 2024-10-21T17:18:19Z

ui/admin/app/components/knowledge/onedrive/AddLinkModal.tsx

+    const handleSave = async () => {
+        await KnowledgeService.updateRemoteKnowledgeSource(
+            agentId,
+            onedriveSource!.id!,


use an early return or throw instead of a !

ryanhopperlowe · 2024-10-21T17:20:20Z

ui/admin/app/components/knowledge/onedrive/OneDriveModal.tsx

there are quite a few instances of ! operators here. Can you sweep through the rest of the PR and make sure you've removed what you can?

StrongMonkey · 2024-10-21T21:02:32Z

Getting this error whenever I open the knowledge accordion in /admin/agents/

Same here.

@ryanhopperlowe @tylerslaton should be fixed.

Signed-off-by: Daishan Peng <[email protected]>

ryanhopperlowe · 2024-10-21T22:01:09Z

It says notion is syncing on a brand new agent before I've even connected it. Is this expected?

ryanhopperlowe · 2024-10-21T22:03:58Z

There's some weird scroll behavior here where items are being displayed even when they exist outside of the dialog

ui/admin/app/components/knowledge/website/WebsiteModal.tsx

Signed-off-by: Daishan Peng <[email protected]>

ryanhopperlowe · 2024-10-21T22:46:28Z

ui/admin/app/components/knowledge/website/WebsiteModal.tsx

+                <RemoteSourceSettingModal
+                    agentId={agentId}
+                    isOpen={isSettingModalOpen}
+                    onOpenChange={setIsSettingModalOpen}
+                    remoteKnowledgeSource={websiteSource!}
+                />
+                <AddWebsiteModal
+                    agentId={agentId}
+                    websiteSource={websiteSource!}
+                    startPolling={startPolling}
+                    isOpen={isAddWebsiteModalOpen}
+                    onOpenChange={setIsAddWebsiteModalOpen}
+                />


Suggested change

<RemoteSourceSettingModal

agentId={agentId}

isOpen={isSettingModalOpen}

onOpenChange={setIsSettingModalOpen}

remoteKnowledgeSource={websiteSource!}

/>

<AddWebsiteModal

agentId={agentId}

websiteSource={websiteSource!}

startPolling={startPolling}

isOpen={isAddWebsiteModalOpen}

onOpenChange={setIsAddWebsiteModalOpen}

/>

{websiteSource && (

<>

<RemoteSourceSettingModal

agentId={agentId}

isOpen={isSettingModalOpen}

onOpenChange={setIsSettingModalOpen}

remoteKnowledgeSource={websiteSource}

/>

<AddWebsiteModal

agentId={agentId}

websiteSource={websiteSource}

startPolling={startPolling}

isOpen={isAddWebsiteModalOpen}

onOpenChange={setIsAddWebsiteModalOpen}

/>

</>

)}

Signed-off-by: Daishan Peng <[email protected]>

tylerslaton

This looks good to me, I do wonder if we could add in a drag/drop for the local file option but as is this UX worked for me and the various cases I tried. Approved!

Knowledge UX revamp

33006ea

Signed-off-by: Daishan Peng <[email protected]>

StrongMonkey requested review from thedadams, ryanhopperlowe and tylerslaton October 18, 2024 21:47

Knowledge UX revamp

d99027c

Signed-off-by: Daishan Peng <[email protected]>

StrongMonkey force-pushed the knowledge branch from 556f8b0 to d99027c Compare October 18, 2024 21:50

thedadams reviewed Oct 19, 2024

View reviewed changes

ryanhopperlowe reviewed Oct 21, 2024

View reviewed changes

StrongMonkey force-pushed the knowledge branch 2 times, most recently from 0871cef to 9024aee Compare October 21, 2024 21:13

Address comments

2741f7b

Signed-off-by: Daishan Peng <[email protected]>

StrongMonkey force-pushed the knowledge branch from 9024aee to 2741f7b Compare October 21, 2024 21:15

StrongMonkey requested review from thedadams and ryanhopperlowe October 21, 2024 21:22

ryanhopperlowe reviewed Oct 21, 2024

View reviewed changes

ui/admin/app/components/knowledge/website/WebsiteModal.tsx Show resolved Hide resolved

StrongMonkey added 2 commits October 21, 2024 15:33

Fix scroll issue

d254fd5

Signed-off-by: Daishan Peng <[email protected]>

Remove extra !

aec42ac

Signed-off-by: Daishan Peng <[email protected]>

ryanhopperlowe reviewed Oct 21, 2024

View reviewed changes

Address comments

6a23daf

Signed-off-by: Daishan Peng <[email protected]>

tylerslaton approved these changes Oct 21, 2024

View reviewed changes

ryanhopperlowe approved these changes Oct 21, 2024

View reviewed changes

StrongMonkey merged commit 3308e96 into obot-platform:main Oct 21, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Knowledge #241

Knowledge #241

StrongMonkey commented Oct 18, 2024 •

edited

Loading

thedadams Oct 19, 2024

thedadams Oct 19, 2024

thedadams Oct 19, 2024

StrongMonkey Oct 21, 2024

thedadams Oct 19, 2024

ryanhopperlowe commented Oct 21, 2024 •

edited

Loading

tylerslaton commented Oct 21, 2024

ryanhopperlowe Oct 21, 2024

StrongMonkey Oct 21, 2024

ryanhopperlowe Oct 21, 2024

ryanhopperlowe Oct 21, 2024

ryanhopperlowe Oct 21, 2024

StrongMonkey Oct 21, 2024

ryanhopperlowe Oct 21, 2024

StrongMonkey Oct 21, 2024

ryanhopperlowe Oct 21, 2024

StrongMonkey Oct 21, 2024

ryanhopperlowe Oct 21, 2024

StrongMonkey Oct 21, 2024

ryanhopperlowe Oct 21, 2024

ryanhopperlowe Oct 21, 2024

ryanhopperlowe Oct 21, 2024

StrongMonkey commented Oct 21, 2024

ryanhopperlowe commented Oct 21, 2024 •

edited

Loading

ryanhopperlowe commented Oct 21, 2024 •

edited

Loading

ryanhopperlowe Oct 21, 2024 •

edited

Loading

tylerslaton left a comment

	file.Spec.Approved = &[]bool{body.Approve}[0]
	file.Spec.Approved = &body.Approve


		file.Spec.Approved = &[]bool{body.Approve}[0]

		return req.Storage.Update(req.Context(), &file)

		// eslint-disable-next-line @typescript-eslint/no-explicit-any
		getKnowledgeFiles: any;

Knowledge #241

Knowledge #241

Conversation

StrongMonkey commented Oct 18, 2024 • edited Loading

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

ryanhopperlowe commented Oct 21, 2024 • edited Loading

tylerslaton commented Oct 21, 2024

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

StrongMonkey commented Oct 21, 2024

ryanhopperlowe commented Oct 21, 2024 • edited Loading

ryanhopperlowe commented Oct 21, 2024 • edited Loading

ryanhopperlowe Oct 21, 2024 • edited Loading

Choose a reason for hiding this comment

tylerslaton left a comment

Choose a reason for hiding this comment

StrongMonkey commented Oct 18, 2024 •

edited

Loading

ryanhopperlowe commented Oct 21, 2024 •

edited

Loading

ryanhopperlowe commented Oct 21, 2024 •

edited

Loading

ryanhopperlowe commented Oct 21, 2024 •

edited

Loading

ryanhopperlowe Oct 21, 2024 •

edited

Loading