fix(RAG): Fix RAG support for taxonomies with compositional skills#3060
fix(RAG): Fix RAG support for taxonomies with compositional skills#3060mergify[bot] merged 1 commit intoinstructlab:maininstructlab/instructlab:mainfrom
Conversation
Signed-off-by: Bill Murdock <bmurdock@redhat.com>
| knowledge_files: list[Path] = [] | ||
| for leaf_node in leaf_nodes.values(): | ||
| knowledge_files.extend(leaf_node[0]["filepaths"]) | ||
| if leaf_node and "filepaths" in leaf_node[0]: |
There was a problem hiding this comment.
Nit: should "filepaths" be a constant somewhere?
There was a problem hiding this comment.
I agree. Specifically, this string needs to match the string here in SDG so there should be some common set of constants that both of these pieces of code pull from for this field and for all of the others in that object. Sharing constants across core and SDG would be a big hassle, but fortunately all of that code is going to migrate to core which will make it a lot easier. For now, I will open a tech debt issue on the tracker so we don't lose track of this.
There was a problem hiding this comment.
(BTW, rather than making these string constants, it would probably be better to make this a class with actual structured names for the fields, e.g., using typing.NamedTuple -- I will note that in the tech debt issue)
|
@Mergifyio backport release-v0.23 |
✅ Backports have been createdDetails
|
…-3060 fix(RAG): Fix RAG support for taxonomies with compositional skills (backport #3060)
This fixes a bug in RAG convert that causes it to fail on taxonomies that have compositional skills. The correct behavior is to ignore the compositional skills because they have no documents, but instead this was crashing when it encountered them. This PR also adds a compositional skill to the sample test taxonomy, which would have let us catch this bug.
Issue resolved by this Pull Request:
Resolves #3008
Checklist:
conventional commits.