[community] Refactor PebbloRetrievalQA #84

Raj725 · 2024-08-14T05:49:53Z

Thank you for contributing to LangChain!

PR title: "package: description"
- Where "package" is whichever of langchain, community, core, experimental, etc. is being modified. Use "docs: ..." for purely docs changes, "templates: ..." for template changes, "infra: ..." for CI changes.
- Example: "community: add foobar LLM"
PR message: Delete this entire checklist and replace with
- Description: a description of the change
- Issue: the issue # it fixes, if applicable
- Dependencies: any dependencies required for this change
- Twitter handle: if your PR gets announced, and you'd like a mention, we'll gladly shout you out!
Add tests and docs: If you're adding a new integration, please include
1. a test for the integration, preferably unit tests that do not rely on network access,
2. an example notebook showing its use. It lives in docs/docs/integrations directory.
Lint and test: Run make format, make lint and make test from the root of the package(s) you've modified. See contribution guidelines for more: https://python.langchain.com/docs/contributing/

Additional guidelines:

Make sure optional dependencies are imported within a function.
Please do not add dependencies to pyproject.toml files (even optional ones) unless they are required for unit tests.
Most PRs should not touch more than one package.
Changes should be backwards compatible.
If you are adding something to community, do not re-import it in langchain.

If no one reviews your PR within a few days, please @-mention one of baskaryan, efriis, eyurtsev, ccurme, vbarda, hwchase17.

gr8nishan · 2024-08-14T06:22:35Z

libs/community/langchain_community/chains/pebblo_retrieval/base.py

_acall function does not give call to prompt API. Is prompt API call in _call function not required ?

The prompt API call in _acall is needed, but the team member who worked on it didn't add it. Since _acall is an async function, the API calls inside it should also be async.

I'll include it in the next PR. Keeping this PR to refactoring only since it's already over 500 lines.

gr8nishan · 2024-08-14T06:25:51Z

libs/community/langchain_community/chains/pebblo_retrieval/utilities.py

+        pebblo_resp = None
+        payload = app.dict(exclude_unset=True)
+
+        if self.classifier_location == "local":


if someone has set the classifier location as local and also has sent the API key will we be calling classifier from both daxa and local.. Is it the desired flow ?

Yes. This PR doesn't change the existing flow—just refactored the existing code.

gr8nishan · 2024-08-14T06:31:16Z

libs/community/langchain_community/chains/pebblo_retrieval/utilities.py

+                "Request: method %s, url %s, body %s len %s response status %s body %s",
+                method,
+                response.request.url,
+                str(response.request.body),


I think we should not be logging body here as it might print some info which is PII

Not sure why this was logged. This line was in all the API calls, including prompt/governance, so I left it as is. I'll remove it if it's not needed. Since it's a debug log, it shouldn't be an issue.

This should be removed from both the places if not in this then next iteration.. The issue I see is if someone is trying to debug the RAG app, this info might get absorbed in the centralized log and people who may not have access to the data might get access to the info in the log.

gr8nishan · 2024-08-14T06:41:51Z

libs/community/langchain_community/chains/pebblo_retrieval/utilities.py

+                    resp.get("retrieval_data", {}).get("prompt", {})
+                )
+                payload["prompt"].pop("data")
+                context = payload["context"]


We should use get here in payload["response"], payload["prompt"], payload["context"]

…in-ai#25605) Complete missing arguments in api doc of `PineconeVectorStore`.

…ader (langchain-ai#25593) - **Description:** Updating metadata for sharepoint loader with full path i.e., webUrl - **Issue:** NA - **Dependencies:** NA - **Tests:** NA - **Docs** NA Co-authored-by: dristy.cd <[email protected]> Co-authored-by: ccurme <[email protected]>

Otherwise I've got KeyError from `fastembeds`

…-ai#25619) Mistral 7B occasionally fails tool-calling tests. Updating to Mixtral appears to improve this.

Requires langsmith 0.1.101 release

and update langsmtih example selector docs

This ensures version 0.15.7+ is pulled. This version of unstructured uses a version of NLTK >= 3.8.2 that has a fix for a critical CVE: GHSA-cgvx-9447-vcch

Thank you for contributing to LangChain! **Description:** Adding `BoxRetriever` for langchain_box. This retriever handles two use cases: * Retrieve all documents that match a full-text search * Retrieve the answer to a Box AI prompt as a Document **Twitter handle:** @BoxPlatform - [x] **Add tests and docs**: If you're adding a new integration, please include 1. a test for the integration, preferably unit tests that do not rely on network access, 2. an example notebook showing its use. It lives in `docs/docs/integrations` directory. - [x] **Lint and test**: Run `make format`, `make lint` and `make test` from the root of the package(s) you've modified. See contribution guidelines for more: https://python.langchain.com/docs/contributing/ Additional guidelines: - Make sure optional dependencies are imported within a function. - Please do not add dependencies to pyproject.toml files (even optional ones) unless they are required for unit tests. - Most PRs should not touch more than one package. - Changes should be backwards compatible. - If you are adding something to community, do not re-import it in langchain. If no one reviews your PR within a few days, please @-mention one of baskaryan, efriis, eyurtsev, ccurme, vbarda, hwchase17. --------- Co-authored-by: Erick Friis <[email protected]>

@efriis

…tation (langchain-ai#25430) ### Summary Create `langchain-databricks` as a new partner packages. This PR does not migrate all existing Databricks integration, but the package will eventually contain: * `ChatDatabricks` (implemented in this PR) * `DatabricksVectorSearch` * `DatabricksEmbeddings` * ~`UCFunctionToolkit`~ (will be done after UC SDK work which drastically simplify implementation) Also, this PR does not add integration tests yet. This will be added once the Databricks test workspace is ready. Tagging @efriis as POC ### Tracker [✍️] Create a package and imgrate ChatDatabricks [ ] Migrate DatabricksVectorSearch, DatabricksEmbeddings, and their docs ~[ ] Migrate UCFunctionToolkit and its doc~ [ ] Add provider document and update README.md [ ] Add integration tests and set up secrets (after moved to an external package) [ ] Add deprecation note to the community implementations. --------- Signed-off-by: B-Step62 <[email protected]> Co-authored-by: Erick Friis <[email protected]>

Raj725 requested review from srics and gr8nishan August 14, 2024 05:49

gr8nishan reviewed Aug 14, 2024

View reviewed changes

Raj725 force-pushed the refactor/PebbloRetrievalQA branch 4 times, most recently from 49e0549 to 330d10e Compare August 21, 2024 04:47

daziz and others added 22 commits August 21, 2024 08:49

docs:Correcting spelling mistakes (langchain-ai#25612)

e7d6b25

Improvement[Doc] Improve api doc in of PineconeVectorStore (langcha…

34d0417

…in-ai#25605) Complete missing arguments in api doc of `PineconeVectorStore`.

docs: Update qdrant.ipynb "BM25".lower() (langchain-ai#25616)

d457d7d

Otherwise I've got KeyError from `fastembeds`

together[patch]: use mixtral in standard integration tests (langchain…

10a2ce2

…-ai#25619) Mistral 7B occasionally fails tool-calling tests. Updating to Mixtral appears to improve this.

docs: how to use langsmith few shot (langchain-ai#25601)

a78843b

Requires langsmith 0.1.101 release

core[patch]: support oai dicts as messages (langchain-ai#25621)

0bc3845

and update langsmtih example selector docs

core[patch]: Release 0.2.34 (langchain-ai#25622)

628574b

docs: langsmith few shot prereq (langchain-ai#25623)

71c2ec6

docs: ls how to link (langchain-ai#25624)

4feda41

infra: test convert_message (langchain-ai#25632)

39c4481

[unstructured][security] Bump unstructured version (langchain-ai#25364)

b71ae52

This ensures version 0.15.7+ is pulled. This version of unstructured uses a version of NLTK >= 3.8.2 that has a fix for a critical CVE: GHSA-cgvx-9447-vcch

docs: add prereq commas (langchain-ai#25626)

f4b3c90

partners/unstructured: release 0.1.2 (langchain-ai#25637)

c8be0a9

docs: fix openai api ref (langchain-ai#25639)

9daff60

chroma: add back fastapi optional dep (langchain-ai#25641)

766b650

docs: ls similar examples header (langchain-ai#25642)

f535e8a

Use 1.101 instead of 1.100 bc 1.100 was yanked (langchain-ai#25638)

4591bc0

docs: link Versions in intro (langchain-ai#25640)

4f347cb

pqa v1

b98a9d8

Raj725 added 5 commits August 22, 2024 09:10

temp

a6fd57e

qa chain: model rename

5ea6c34

Move prompt validity check into PebbloAPIWrapper

73f24fb

Model updates, Code & Log cleanup, Fix lint errors

72e62bf

Rename PebbloAPIWrapper to PebbloRetrievalAPIWrapper

1d2942a

Raj725 force-pushed the refactor/PebbloRetrievalQA branch from 7aa9b62 to 1d2942a Compare August 22, 2024 03:41

Raj725 closed this Aug 22, 2024

Raj725 deleted the refactor/PebbloRetrievalQA branch August 22, 2024 16:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[community] Refactor PebbloRetrievalQA #84

[community] Refactor PebbloRetrievalQA #84

Raj725 commented Aug 14, 2024

gr8nishan Aug 14, 2024 •

edited

Loading

Raj725 Aug 14, 2024

gr8nishan Aug 14, 2024

Raj725 Aug 14, 2024

gr8nishan Aug 14, 2024

Raj725 Aug 14, 2024

gr8nishan Aug 14, 2024

gr8nishan Aug 14, 2024

[community] Refactor PebbloRetrievalQA #84

[community] Refactor PebbloRetrievalQA #84

Conversation

Raj725 commented Aug 14, 2024

gr8nishan Aug 14, 2024 • edited Loading

Choose a reason for hiding this comment

Raj725 Aug 14, 2024

Choose a reason for hiding this comment

gr8nishan Aug 14, 2024

Choose a reason for hiding this comment

Raj725 Aug 14, 2024

Choose a reason for hiding this comment

gr8nishan Aug 14, 2024

Choose a reason for hiding this comment

Raj725 Aug 14, 2024

Choose a reason for hiding this comment

gr8nishan Aug 14, 2024

Choose a reason for hiding this comment

gr8nishan Aug 14, 2024

Choose a reason for hiding this comment

gr8nishan Aug 14, 2024 •

edited

Loading