feat(rag): Add lab rag convert CLI command #2902

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merged

mergify merged 1 commit into instructlab:main from jwm4:jwm4-rag-convert

Jan 17, 2025

Contributor

jwm4 commented Jan 10, 2025 •

edited

Loading

Adds new ilab rag convert command which takes in input documents from a taxonomy or a directory of original document files (e.g., PDFs) and outputs Docling JSON for each input document.

Issue resolved by this Pull Request:
Resolves #2890

Dev Docs related to this Pull Request:
Link to Dev Doc or PR: instructlab/dev-docs#161

Checklist:

Commit Message Formatting: Commit titles and messages follow guidelines in the
conventional commits.
Changelog updated with breaking and/or notable changes for the next minor release.
Documentation has been updated, if necessary.
Unit tests have been added, if necessary.
Functional tests have been added, if necessary.
E2E Workflow tests have been added, if necessary.

Contributor

mergify bot commented Jan 10, 2025

This pull request has merge conflicts that must be resolved before it can be
merged. @jwm4 please rebase it. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

mergify bot added testing needs-rebase ci-failure and removed ci-failure labels

bbrowning reviewed

View reviewed changes

src/instructlab/rag/process_docs.py Outdated Show resolved Hide resolved

dmartinol reviewed

View reviewed changes

src/instructlab/rag/process_docs.py Outdated Show resolved Hide resolved

dmartinol reviewed

View reviewed changes

src/instructlab/cli/rag/convert.py Show resolved Hide resolved

dmartinol reviewed

View reviewed changes

src/instructlab/cli/rag/convert.py Outdated Show resolved Hide resolved

dmartinol reviewed

View reviewed changes

src/instructlab/cli/rag/rag.py Outdated Show resolved Hide resolved

dmartinol reviewed

View reviewed changes

src/instructlab/rag/process_docs.py Outdated Show resolved Hide resolved

dmartinol reviewed

View reviewed changes

src/instructlab/rag/process_docs.py Outdated Show resolved Hide resolved

dmartinol reviewed

View reviewed changes

src/instructlab/rag/process_docs.py Outdated Show resolved Hide resolved

mergify bot added ci-failure and removed needs-rebase ci-failure labels

cdoern reviewed

View reviewed changes

Contributor

cdoern left a comment

just reviewing the tests. These align with the team's expectations of basic unit tests for a new implementation, no major concerns from me

tests/test_lab_rag_convert.py Show resolved Hide resolved

tests/test_lab_rag_convert.py Outdated Show resolved Hide resolved

tests/test_lab_rag_convert.py Show resolved Hide resolved

dmartinol reviewed

View reviewed changes

tests/test_lab_rag_convert.py Outdated Show resolved Hide resolved

dmartinol reviewed

View reviewed changes

tests/test_lab_rag_convert.py Show resolved Hide resolved

mergify bot removed the ci-failure label

nathan-weinberg self-requested a review

January 16, 2025 15:21

jwm4 marked this pull request as ready for review

January 16, 2025 19:31

jwm4 requested a review from a team

January 16, 2025 19:42

jwm4 changed the title ~~[Draft] ilab rag convert~~ feat(rag): Add lab rag convert CLI command

jwm4 force-pushed the jwm4-rag-convert branch from 180a27e to 1f01cf2 Compare

January 16, 2025 21:04

mergify bot removed the ci-failure label

jwm4 marked this pull request as draft

January 16, 2025 21:06

mergify bot added ci-failure and removed ci-failure labels

jwm4 marked this pull request as ready for review

January 17, 2025 00:58

mergify bot added ci-failure and removed ci-failure labels

nathan-weinberg approved these changes

View reviewed changes

Member

nathan-weinberg left a comment

I have one question for Charlie about the testdata but otherwise this is looking good to me - approving from side

tests/testdata/default_config.yaml Show resolved Hide resolved

nathan-weinberg added the hold label

mergify bot added the one-approval label

cdoern reviewed

View reviewed changes

Contributor

cdoern left a comment

nothing major, I appreciate using the new config_class and the added comments to some of the existing assertions, doc strings, etc.

src/instructlab/cli/rag/convert.py Outdated Show resolved Hide resolved

src/instructlab/cli/rag/convert.py Outdated Show resolved Hide resolved

src/instructlab/cli/rag/convert.py Outdated Show resolved Hide resolved

src/instructlab/clickext.py Show resolved Hide resolved

src/instructlab/rag/convert.py Show resolved Hide resolved

src/instructlab/rag/convert.py Show resolved Hide resolved

cdoern approved these changes

View reviewed changes

Contributor

cdoern left a comment

lgtm. @jwm4 can you squash the commits and then I will remove the hold?

mergify bot removed the one-approval label


          feat(rag): Add lab rag convert CLI command

be9101b

Signed-off-by: Bill Murdock <[email protected]>

jwm4 force-pushed the jwm4-rag-convert branch from 306054b to be9101b Compare

January 17, 2025 18:07

Contributor Author

jwm4 commented Jan 17, 2025

Commits are squashed now.

jwm4 removed the hold label

mergify bot merged commit bc784d7 into instructlab:main

29 checks passed

jwm4 mentioned this pull request

InstructLab Maintainer nomination for Bill Murdock #2931

Closed

dmartinol mentioned this pull request

[FOR SHARING PURPOSES ONLY] RAG ingestion and chat pipelines #2736

Closed

11 tasks

jwm4 deleted the jwm4-rag-convert branch

February 12, 2025 13:52

booxter mentioned this pull request

mypy failures on main #3253

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing