Skip to content
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.

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

CogniSwitch Integration #8466

Merged
merged 14 commits into from
Nov 8, 2023
Merged

Conversation

CogniJT
Copy link
Contributor

@CogniJT CogniJT commented Oct 25, 2023

Added Components

  • QueryEngine
  • Tests
  • Example Notebook
  • Inits

Description

This provides a connector to the CogniSwitch platform within LlamaIndex.

What is CogniSwitch?

CogniSwitch enhances the reliability of Generative AI applications for enterprises. It achieves this by leveraging AI to gather and organize knowledge from documented sources, eliminating biases and hallucinations in AI responses. The platform allows experts to curate and visualize this knowledge before it is published. The CogniSwitch API enables Gen AI applications to access this knowledge as needed, ensuring reliability. It seamlessly integrates with top Generative AI technologies and offers customized solutions for different business functions within an enterprise.

Why CogniSwitch?

Use CogniSwitch to build production ready applications that can consume, organize and retrieve knowledge flawlessly. Using the framework of your choice, in this case Llama-Index, CogniSwitch helps alleviate the stress of decision making when it comes to, choosing the right storage and retrieval formats. It also eradicates reliability issues and hallucinations when it comes to responses that are generated. Get started by interacting with your knowledge in just three simple steps

Type of Change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Added new unit/integration tests
  • Added new notebook (that tests end-to-end)
  • I stared at the code and made sure it makes sense

Suggested Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I ran make format; make lint to appease the lint gods

- QueryEngine
- ToolSpec
- Tests
- Example Notebook
- Inits
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Collaborator

@jerryjliu jerryjliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! let's keep the cogniswitch query engine part here, for the tool spec could you submit to llama-hub instead? we're centralizing all integrations there https://github.com/run-llama/llama-hub

@CogniJT
Copy link
Contributor Author

CogniJT commented Oct 25, 2023

Thanks for the response & guidance @jerryjliu

Will remove the tool spec & update the pull request accordingly.

@CogniJT CogniJT marked this pull request as draft October 26, 2023 03:08
@CogniJT
Copy link
Contributor Author

CogniJT commented Oct 26, 2023

Hi @jerryjliu, created a related PR in LlamaHub. That would be a dependency in order for this to work. Could you streamline that please?

@CogniJT
Copy link
Contributor Author

CogniJT commented Oct 26, 2023

run-llama/llama-hub#604

@CogniJT CogniJT requested a review from jerryjliu October 30, 2023 02:52
@CogniJT CogniJT marked this pull request as ready for review October 30, 2023 02:53
@CogniJT
Copy link
Contributor Author

CogniJT commented Oct 30, 2023

Hi @jerryjliu, made the changes as you requested. We are now merged with LlamaHub & using that import over here. Do take a look & let us know if anything else is required. Thanks!

@Disiok
Copy link
Collaborator

Disiok commented Oct 30, 2023

Hi @jerryjliu, made the changes as you requested. We are now merged with LlamaHub & using that import over here. Do take a look & let us know if anything else is required. Thanks!

Thanks for the great work! Taking a look to merge this.

@Disiok
Copy link
Collaborator

Disiok commented Oct 30, 2023

Hey @CogniJT could you allow maintainers to push to the branch? Just want to push some cleanup fixes before I merge the PR.

@saiCogniswitch
Copy link
Contributor

hey @Disiok, can you let us know if there is any bug in the code?

@CogniJT
Copy link
Contributor Author

CogniJT commented Nov 1, 2023

Hi @Disiok, as far I can see there does not seem to be any restrictions in settings. Perhaps you're talking about the llama_index repo access? We don't have permissions to change that. As part of your review, could you leave your comments for changes & we can follow through on those. Thanks!

@CogniJT
Copy link
Contributor Author

CogniJT commented Nov 1, 2023

@jerryjliu could you please clarify what's required to move ahead? Thank you.

@CogniJT
Copy link
Contributor Author

CogniJT commented Nov 1, 2023

Just a heads up that we've updated the BaseQueryEngine to its new form subclassing PromptMixin

@saiCogniswitch
Copy link
Contributor

hi @jerryjliu I was checking the above checks where linting failed, but in the codespaces from my side the lint had passed successfully, can you please let us know how to resolve this and move forward. Thanks!
have a good day.

@Disiok
Copy link
Collaborator

Disiok commented Nov 3, 2023

Hey folks, looks like lint is still failing. could you pull the latest main, and make sure you have the pre-commit hooks installed? Thanks!

Also it'd be really helpful to allow us to make changes to your branch, so we can quickly fix stuff up on our side. Take a look at https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork to see if this is possible.

@CogniJT
Copy link
Contributor Author

CogniJT commented Nov 4, 2023

Thanks @Disiok, the link clarifies what you were asking for. However I'm unable to find the below option under this Pull Request

On user-owned forks, if you want to allow anyone with push access to the upstream repository to make changes to your pull request, select Allow edits from maintainers.
Perhaps because this isn't user-owned?

@saiCogniswitch could you look into the linting issues

@saiCogniswitch
Copy link
Contributor

hi @Disiok , so I have run the lint with pre-commit hooks installed and the lint has run successfully in the codespaces, currently I have updated the changes accordingly. Let me know if if there is something else that can be done to check the lint thanks.
have a good day.

@Disiok
Copy link
Collaborator

Disiok commented Nov 6, 2023

Hey folks, here's the specific lint issue:

pre-commit hook(s) made changes.
If you are seeing this message in CI, reproduce locally with: `pre-commit run --all-files`.
To run `pre-commit` as part of git workflow, use `pre-commit install`.
All changes made by hooks:
diff --git a/llama_index/query_engine/cogniswitch_query_engine.py b/llama_index/query_engine/cogniswitch_query_engine.py
index a90d84b..baaa3[69](https://github.com/run-llama/llama_index/actions/runs/6753676283/job/18381073099?pr=8466#step:6:70) 100644
--- a/llama_index/query_engine/cogniswitch_query_engine.py
+++ b/llama_index/query_engine/cogniswitch_query_engine.py
@@ -4,7 +4,7 @@ import requests
 
 from llama_index.indices.query.base import BaseQueryEngine
 from llama_index.indices.query.schema import QueryBundle
-from llama_index.response.schema import RESPONSE_TYPE, Response
+from llama_index.response.schema import Response
 
 
 class CogniswitchQueryEngine(BaseQueryEngine):
diff --git a/tests/query_engine/test_cogniswitch_query_engine.py b/tests/query_engine/test_cogniswitch_query_engine.py
index 399f[79](https://github.com/run-llama/llama_index/actions/runs/6753676283/job/18381073099?pr=8466#step:6:80)3..[84](https://github.com/run-llama/llama_index/actions/runs/6753676283/job/18381073099?pr=8466#step:6:85)2f4d8 100644
--- a/tests/query_engine/test_cogniswitch_query_engine.py
+++ b/tests/query_engine/test_cogniswitch_query_engine.py
@@ -1,5 +1,5 @@
 from typing import Any
-from unittest.mock import MagicMock, patch
+from unittest.mock import patch
 
 import pytest
 from llama_index.query_engine.cogniswitch_query_engine import CogniswitchQueryEngine
make: *** [Makefile:11: lint] Error 1
Error: Process completed with exit code 2.

@saiCogniswitch
Copy link
Contributor

hi @Disiok , thanks for sharing the lint issue, the changes it is showing in the w.r.t the python file name, the change is same for the both "cogniswitch_query_engine.py" and "test_cogniswitch_query_engine.py". Do we have to do something w.r.t that?
I have also removed the unnecessary imports and updated it. Can you check and let us know if now is it ok.
Have a good day.

@saiCogniswitch
Copy link
Contributor

Hi @jerryjliu
Good Morning
just bumping this up for approval and merge, let us know if anything is required from us.
Thanks , have a good day

@Disiok
Copy link
Collaborator

Disiok commented Nov 8, 2023

Thanks for the updates! Looks like it's passing CI now.

@Disiok Disiok merged commit aa00a00 into run-llama:main Nov 8, 2023
@saiCogniswitch
Copy link
Contributor

Thanks @Disiok for the approval and merge.

@CogniJT
Copy link
Contributor Author

CogniJT commented Nov 8, 2023

Thanks @Disiok & @jerryjliu for all your help

@CogniJT
Copy link
Contributor Author

CogniJT commented Nov 8, 2023

@Disiok Would the examples we shared in this PR go into doc.llamaindex.ai?

ryanpeach pushed a commit to OnScale/llama_index that referenced this pull request Feb 7, 2024
* CogniSwitch Integration

- QueryEngine
- ToolSpec
- Tests
- Example Notebook
- Inits

* Delete llama_index/tools/tool_spec/cogniswitch directory

Removing ToolSpec as per Jerry's recommendation

* Add files via upload

* Update __init__.py

Removed CogniswitchToolSpec import & __all__

* Nits for CSQueryEngine

* updated the files after checking format, linting

* updated cogniswitch query engine and notebooks

* Update CogniswitchQueryEngine.ipynb

Nits, language

* Update CogniswitchToolSpec.ipynb

Nits, language

* updated the cogniswitch_query_engine file

* updated the test cogniswitch_query_engine filename

* removed unecessary imports from the code files

---------

Co-authored-by: JT <JoshuaT@aikonlabs.com>
Co-authored-by: saiCogniswitch <145636106+saiCogniswitch@users.noreply.github.com>
This was referenced Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants