Feature/gsk 2334 talk to my model mvp #1889

AbSsEnT · 2024-04-12T14:18:29Z

Description

Related Issue

Type of Change

📚 Examples / docs / tutorials / dependencies update
🔧 Bug fix (non-breaking change which fixes an issue)
🥂 Improvement (non-breaking change which improves an existing feature)
🚀 New feature (non-breaking change which adds functionality)
💥 Breaking change (fix or feature that would cause existing functionality to change)
🔐 Security fix

Checklist

I've read the CODE_OF_CONDUCT.md document.
I've read the CONTRIBUTING.md guide.
I've written tests for all new methods and classes that I created.
I've written the docstring in Google format for all the methods and classes that I used.
I've updated the pdm.lock running pdm update-lock (only applicable when pyproject.toml has been
modified)

…-2334-talk-to-my-model-mvp

…ich returns a prediction from the row of the dataset.

… is called. Replaced placeholders and dummy variables to the real objects.

…-2334-talk-to-my-model-mvp

…skard-AI/giskard into feature/gsk-2335-query-prediction-for-the-row-from-the-dataset

…f 'tools' API.

…flow. Performed prompt engineering for the tool description and LLM instruction.

…-2334-talk-to-my-model-mvp

…-2335-query-prediction-for-the-row-from-the-dataset

…skard-AI/giskard into feature/gsk-2335-query-prediction-for-the-row-from-the-dataset

…ction-for-the-row-from-the-dataset Implementation of the "PredictFromDatasetTool"

…-2334-talk-to-my-model-mvp

…skard-AI/giskard into feature/gsk-2336-query-shap-prediction-explanation

…-to-tools' of github.com:Giskard-AI/giskard into feature/gsk-2336-query-shap-prediction-explanation

…-to-tools' of github.com:Giskard-AI/giskard into feature/gsk-2334-talk-to-my-model-mvp

…skard-AI/giskard into feature/gsk-2336-query-shap-prediction-explanation

…prediction-explanation Feature/gsk 2336 query shap prediction explanation

…common for all child Tools classes.

…-to-tools' of github.com:Giskard-AI/giskard into feature/gsk-2419-adapt-workflow-to-the-tools-api

giskard/llm/client/copilot.py

giskard/models/base/model.py

pierlj · 2024-04-17T10:29:05Z

giskard/models/base/model.py

+        }
+
+    @staticmethod
+    def _gather_context(message_list: list) -> str:


Could be nice to check that the resulting string is not too long for the context windows of the LLM

…b.com/Giskard-AI/giskard into feature/gsk-2334-talk-to-my-model-mvp

pierlj · 2024-04-18T07:21:51Z

giskard/llm/talk/config.py

+def get_talk_llm_model() -> str:
+    return os.getenv("GSK_TALK_LLM_MODEL", "gpt-4-0125-preview")


This is unused I think, we can remove it

andreybavt · 2024-04-18T07:57:55Z

giskard/llm/client/copilot.py

+class ToolChatMessage(ChatMessage):
+    name: Optional[str] = None
+    tool_call_id: Optional[str] = None
+    tool_calls: Optional[list] = None


nitpick, could be more precise: Optional[list[ChatCompletionMessageToolCall]]

I'll add it, I forget if adding a typing based on a conditional imports create troubles (even if put under quotes), I'll try it.

andreybavt · 2024-04-18T08:03:23Z

giskard/llm/client/copilot.py

+    tool_calls: Optional[list] = None
+
+
+def _format_message(msg: ChatMessage) -> dict:


should be ToolChatMessage type

actually for this one, it's ChatMessage, but what GiskardCopilotClient.complete returns is ToolChatMessage

andreybavt · 2024-04-18T08:30:30Z

giskard/models/base/model.py

+            **TALK_CLIENT_CONFIG,
+        )
+
+        if content := response.content:


I don't get the use of walrus here

yeah good catch, it would've been useful if these where properties with lazy loading of sorts, but no both of these are just simple attributes. I fixed it.

andreybavt

Generally LGTM, just minor comments

sonarqubecloud · 2024-04-18T18:16:59Z

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
79.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

AbSsEnT added 30 commits December 12, 2023 16:55

Initial commit for the MVP of "Talk to my model" functionality.

bfc87e5

Merge branch 'main' of github.com:Giskard-AI/giskard into feature/gsk…

e8a3b6e

…-2334-talk-to-my-model-mvp

Merge branch 'main' of github.com:Giskard-AI/giskard into feature/gsk…

3316bd4

…-2334-talk-to-my-model-mvp

Defined the basic pipeline of the 'talk' function.

4452fd2

Defined the Tool interface and the boilerplate for the first tool, wh…

847e8b8

…ich returns a prediction from the row of the dataset.

small addition

7ca2416

Added method to initialise tools objects, each time the method 'talk'…

b44386b

… is called. Replaced placeholders and dummy variables to the real objects.

Initial implementation of the "__call__" method.

b727b77

Merge branch 'main' of github.com:Giskard-AI/giskard into feature/gsk…

84318d1

…-2334-talk-to-my-model-mvp

Merge branch 'feature/gsk-2334-talk-to-my-model-mvp' of github.com:Gi…

71a08b5

…skard-AI/giskard into feature/gsk-2335-query-prediction-for-the-row-from-the-dataset

Bug fixes. Adapted flow to currently use legacy 'functions' instead o…

549c916

…f 'tools' API.

Debugged "predict from dataset" tool workflow. Debugged the tool work…

66673a3

…flow. Performed prompt engineering for the tool description and LLM instruction.

Merge branch 'main' of github.com:Giskard-AI/giskard into feature/gsk…

4deb3fc

…-2334-talk-to-my-model-mvp

Merge branch 'main' of github.com:Giskard-AI/giskard into feature/gsk…

0e98572

…-2335-query-prediction-for-the-row-from-the-dataset

Merge branch 'feature/gsk-2334-talk-to-my-model-mvp' of github.com:Gi…

e2781aa

…skard-AI/giskard into feature/gsk-2335-query-prediction-for-the-row-from-the-dataset

Merge pull request #1687 from Giskard-AI/feature/gsk-2335-query-predi…

66f7429

…ction-for-the-row-from-the-dataset Implementation of the "PredictFromDatasetTool"

Initial implementation of the 'SHAPExplanationTool'.

b46fd57

Merge branch 'main' of github.com:Giskard-AI/giskard into feature/gsk…

a836a1e

…-2334-talk-to-my-model-mvp

Merge branch 'feature/gsk-2334-talk-to-my-model-mvp' of github.com:Gi…

4dcd490

…skard-AI/giskard into feature/gsk-2336-query-shap-prediction-explanation

Added handling an errors, while calling tools.

777011a

Merge branch 'feature/gsk-2367-migrate-openai-api-call-from-functions…

19488bd

…-to-tools' of github.com:Giskard-AI/giskard into feature/gsk-2336-query-shap-prediction-explanation

Merge branch 'feature/gsk-2367-migrate-openai-api-call-from-functions…

cd2bdf3

…-to-tools' of github.com:Giskard-AI/giskard into feature/gsk-2334-talk-to-my-model-mvp

Merge branch 'feature/gsk-2334-talk-to-my-model-mvp' of github.com:Gi…

0a9315a

…skard-AI/giskard into feature/gsk-2336-query-shap-prediction-explanation

Merge pull request #1696 from Giskard-AI/feature/gsk-2336-query-shap-…

4db4482

…prediction-explanation Feature/gsk 2336 query shap prediction explanation

Moved more attributes and properties to the BaseTool, since they are …

6b86d48

…common for all child Tools classes.

Changed PredictFromDataset tool's specification.

bbcf347

Adapting model.py to the use of tools API.

e248aab

Merge branch 'feature/gsk-2367-migrate-openai-api-call-from-functions…

394b2bb

…-to-tools' of github.com:Giskard-AI/giskard into feature/gsk-2419-adapt-workflow-to-the-tools-api

Fully changed 'talk' method workflow to use tools API.

dc155ee

Added multiple toll calling for the SHAP explanation tool

9a70228

Rabah Abdul Khalek added 3 commits April 17, 2024 11:40

propagagated ToolChatMessage

1d68c53

fixing imports in model.py

bf9acb0

more work on imports

69b1307

pierlj reviewed Apr 17, 2024

View reviewed changes

giskard/llm/client/copilot.py Outdated Show resolved Hide resolved

Rabah Abdul Khalek and others added 2 commits April 17, 2024 12:03

switching from llmimporterr to giskardinstallationerr

569ce2c

Merge branch 'main' into feature/gsk-2334-talk-to-my-model-mvp

68ded26

pierlj reviewed Apr 17, 2024

View reviewed changes

giskard/models/base/model.py Outdated Show resolved Hide resolved

pierlj reviewed Apr 17, 2024

View reviewed changes

Rabah Abdul Khalek and others added 4 commits April 17, 2024 12:38

simplifying the client calling for copilot

81d9e68

Merge branch 'feature/gsk-2334-talk-to-my-model-mvp' of https://githu…

6de2f80

…b.com/Giskard-AI/giskard into feature/gsk-2334-talk-to-my-model-mvp

restored client init and updated imports

becf3df

Merge branch 'main' into feature/gsk-2334-talk-to-my-model-mvp

0431f94

pierlj reviewed Apr 18, 2024

View reviewed changes

removed unused function

74f5351

rabah-khalek approved these changes Apr 18, 2024

View reviewed changes

rabah-khalek enabled auto-merge (squash) April 18, 2024 07:40

andreybavt reviewed Apr 18, 2024

View reviewed changes

rabah-khalek disabled auto-merge April 18, 2024 08:02

andreybavt reviewed Apr 18, 2024

View reviewed changes

This comment was marked as resolved.

Sign in to view

andreybavt reviewed Apr 18, 2024

View reviewed changes

Rabah Abdul Khalek and others added 2 commits April 18, 2024 11:56

implemented AA's feedback

945addd

Merge branch 'main' into feature/gsk-2334-talk-to-my-model-mvp

15747f5

andreybavt approved these changes Apr 18, 2024

View reviewed changes

rabah-khalek enabled auto-merge (squash) April 18, 2024 10:03

fixing tests

0b5190c

rabah-khalek merged commit 960a93c into main Apr 18, 2024

rabah-khalek deleted the feature/gsk-2334-talk-to-my-model-mvp branch April 18, 2024 18:19

		def get_talk_llm_model() -> str:
		return os.getenv("GSK_TALK_LLM_MODEL", "gpt-4-0125-preview")

		tool_calls: Optional[list] = None


		def _format_message(msg: ChatMessage) -> dict:

Uh oh!

Feature/gsk 2334 talk to my model mvp #1889

Feature/gsk 2334 talk to my model mvp #1889

Uh oh!

Conversation

AbSsEnT commented Apr 12, 2024

Description

Related Issue

Type of Change

Checklist

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

andreybavt left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Apr 18, 2024

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

6 participants