Feat: Add file tree, add include and exclude prefix for auto ingestion #639
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| }); | ||
| } | ||
| }} | ||
| className="justify-start items-center group ml-2 hidden group-hover:block" |
There was a problem hiding this comment.
use visible/invisible instead of hidden/block
This will prevent the row from resizing when it adds the icon button to the row
Suggested change
| className="justify-start items-center group ml-2 hidden group-hover:block" | |
| className="justify-start items-center group ml-2 invisible group-hover:visible" |
njhale
requested changes
Nov 20, 2024
StrongMonkey
force-pushed
the
filetree
branch
from
58ed35e
to
e804f6f
Compare
StrongMonkey
requested review from
ryanhopperlowe,
njhale,
thedadams,
g-linville and
iwilltry42
njhale
requested changes
Nov 21, 2024
njhale
approved these changes
Nov 21, 2024
thedadams
requested changes
Nov 21, 2024
apiclient/types/knowledgesource.go
Outdated
Comment on lines
39
to
40
| FilePathPrefixInclude []string `json:"filePathPrefixInclude,omitempty"` | ||
| FilePathPrefixExclude []string `json:"filePathPrefixExclude,omitempty"` |
There was a problem hiding this comment.
Do we need this both here and KnowledgeSourceManifest? Seems like just in KnowledgeSourceManifest is enough.
pkg/api/handlers/knowledgesource.go
Outdated
Comment on lines
28
to
29
| FilePathPrefixExclude: knowledgeSource.Spec.Manifest.FilePathPrefixExclude, | ||
| FilePathPrefixInclude: knowledgeSource.Spec.Manifest.FilePathPrefixInclude, |
There was a problem hiding this comment.
Can be removed if we use the fields from KnowledgeSourceManifest.
There was a problem hiding this comment.
yeah, right. I will drop that
Comment on lines
104
to
111
| matchInclude := isFileMatchPrefixPattern(file.Spec.FileName, source.Spec.Manifest.FilePathPrefixInclude) | ||
| matchExclude := isFileMatchPrefixPattern(file.Spec.FileName, source.Spec.Manifest.FilePathPrefixExclude) | ||
| switch { | ||
| case source.Spec.Manifest.AutoApprove != nil && *source.Spec.Manifest.AutoApprove: | ||
| file.Spec.Approved = typed.Pointer(true) | ||
| case matchExclude: | ||
| file.Spec.Approved = typed.Pointer(false) | ||
| case matchInclude: | ||
| file.Spec.Approved = typed.Pointer(true) | ||
| } |
There was a problem hiding this comment.
nit: only do the processing that's necessary.
Suggested change
| matchInclude := isFileMatchPrefixPattern(file.Spec.FileName, source.Spec.Manifest.FilePathPrefixInclude) | |
| matchExclude := isFileMatchPrefixPattern(file.Spec.FileName, source.Spec.Manifest.FilePathPrefixExclude) | |
| switch { | |
| case source.Spec.Manifest.AutoApprove != nil && *source.Spec.Manifest.AutoApprove: | |
| file.Spec.Approved = typed.Pointer(true) | |
| case matchExclude: | |
| file.Spec.Approved = typed.Pointer(false) | |
| case matchInclude: | |
| file.Spec.Approved = typed.Pointer(true) | |
| } | |
| switch { | |
| case source.Spec.Manifest.AutoApprove != nil && *source.Spec.Manifest.AutoApprove: | |
| file.Spec.Approved = typed.Pointer(true) | |
| case isFileMatchPrefixPattern(file.Spec.FileName, source.Spec.Manifest.FilePathPrefixInclude): | |
| file.Spec.Approved = typed.Pointer(false) | |
| case isFileMatchPrefixPattern(file.Spec.FileName, source.Spec.Manifest.FilePathPrefixInclude): | |
| file.Spec.Approved = typed.Pointer(true) | |
| } |
ryanhopperlowe
approved these changes
Nov 21, 2024
Noticing that the add button is getting cutoff when the file name is too long. I think we need to truncate the text and add a tooltip to it |
ryanhopperlowe
requested changes
Nov 21, 2024
|
@ryanhopperlowe good catch, should be fixed 🙏 |
Beautiful 🙌 |
ryanhopperlowe
approved these changes
Nov 21, 2024
iwilltry42
approved these changes
Nov 22, 2024
thedadams
approved these changes
Nov 22, 2024
Signed-off-by: Daishan Peng <[email protected]>
Signed-off-by: Daishan Peng <[email protected]>
Signed-off-by: Daishan Peng <[email protected]>
Signed-off-by: Daishan Peng <[email protected]>
Signed-off-by: Daishan Peng <[email protected]>
Signed-off-by: Daishan Peng <[email protected]>
StrongMonkey
force-pushed
the
filetree
branch
from
8045809
to
88e4c53
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR add file tree to show knowledge files. Demo is here https://www.loom.com/share/0e707405c1194ea1a9a1ffd595347a22
It also adds a backend piece of functionality to include/exclude folder prefix so that user can add a folder in UI to automatically ingest files inside the folder.