Add Prompt dataclass with initial methods #562
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @adivate2021, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a comprehensive system for managing prompts within the application. It establishes a new Prompt dataclass for structured prompt handling, extends the API with dedicated endpoints for prompt insertion and retrieval, and provides corresponding client-side methods and data models. This enhancement streamlines the process of defining, storing, and utilizing dynamic prompt templates.
Highlights
- New Prompt Dataclass: Introduced a new
Promptdataclass insrc/judgeval/prompts/prompt.pyto encapsulate prompt definition, storage, and compilation logic. This class includescreateandgetclass methods for interacting with the API, and acompilemethod for variable substitution. - API Endpoint Additions: Added new API endpoints
/prompts/insert/and/prompts/fetch/to handle the creation and retrieval of prompts. These endpoints are integrated into the API generation scripts. - API Client Functionality: Implemented
prompts_insertandprompts_fetchmethods within both the synchronous and asynchronous API clients (JudgmentSyncClientandJudgmentAsyncClient) to facilitate interaction with the new prompt management endpoints. - Data Model Definitions: Defined new
TypedDictand PydanticBaseModelclasses (PromptInsertRequest,PromptInsertResponse,PromptFetchResponse) to standardize the data structures for prompt-related API requests and responses.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a Prompt dataclass along with methods to create, fetch, and compile prompts, and integrates them into the API. The changes are generally well-structured. However, I've identified a couple of critical issues in src/judgeval/prompts/prompt.py related to unsafe dictionary access for optional fields, which could lead to KeyError exceptions. Additionally, there are a few medium-severity issues concerning code style, a missing type hint, and a potential regression in error handling in prompt_scorer.py that could affect user experience and code consistency. I've provided detailed comments and suggestions for each of these points.
| r = client.prompts_insert( | ||
| payload={"name": name, "prompt": prompt, "tags": tags} | ||
| ) | ||
| return r["commit_id"], r["parent_commit_id"] |
There was a problem hiding this comment.
The parent_commit_id key is not guaranteed to be in the response dictionary r as it is marked as NotRequired in the PromptInsertResponse type definition. Accessing it directly with r["parent_commit_id"] will raise a KeyError if the key is absent. You should use r.get("parent_commit_id") for safe access.
| return r["commit_id"], r["parent_commit_id"] | |
| return r["commit_id"], r.get("parent_commit_id") |
| prompt=prompt_config["prompt"], | ||
| tags=prompt_config["tags"], | ||
| commit_id=prompt_config["commit_id"], | ||
| parent_commit_id=prompt_config["parent_commit_id"], |
There was a problem hiding this comment.
The parent_commit_id key is not guaranteed to be in the prompt_config dictionary as it is marked as NotRequired in the PromptFetchResponse type definition. Accessing it directly with prompt_config["parent_commit_id"] will raise a KeyError if the key is absent. You should use prompt_config.get("parent_commit_id") for safe access.
| parent_commit_id=prompt_config["parent_commit_id"], | |
| parent_commit_id=prompt_config.get("parent_commit_id"), |
| query_params = {} | ||
| query_params["name"] = name | ||
| if commit_id is not None: | ||
| query_params["commit_id"] = commit_id | ||
| if tag is not None: | ||
| query_params["tag"] = tag |
There was a problem hiding this comment.
The construction of query_params can be made slightly more concise. While the current implementation is correct, initializing the dictionary with the required name parameter and then conditionally adding optional parameters can improve readability.
| query_params = {} | |
| query_params["name"] = name | |
| if commit_id is not None: | |
| query_params["commit_id"] = commit_id | |
| if tag is not None: | |
| query_params["tag"] = tag | |
| query_params = {"name": name} | |
| if commit_id is not None: | |
| query_params["commit_id"] = commit_id | |
| if tag is not None: | |
| query_params["tag"] = tag |
| query_params = {} | ||
| query_params["name"] = name | ||
| if commit_id is not None: | ||
| query_params["commit_id"] = commit_id | ||
| if tag is not None: | ||
| query_params["tag"] = tag |
There was a problem hiding this comment.
Similar to the synchronous version, the construction of query_params here can be made more concise for better readability.
| query_params = {} | |
| query_params["name"] = name | |
| if commit_id is not None: | |
| query_params["commit_id"] = commit_id | |
| if tag is not None: | |
| query_params["tag"] = tag | |
| query_params = {"name": name} | |
| if commit_id is not None: | |
| query_params["commit_id"] = commit_id | |
| if tag is not None: | |
| query_params["tag"] = tag |
| tag: Optional[str] = None, | ||
| judgment_api_key: str = os.getenv("JUDGMENT_API_KEY") or "", | ||
| organization_id: str = os.getenv("JUDGMENT_ORG_ID") or "", | ||
| ): |
There was a problem hiding this comment.
The function fetch_prompt is missing a return type hint. Based on its usage and the prompts_fetch method it calls, the return type should be PromptFetchResponse. Adding type hints improves code clarity and allows for better static analysis. You will also need to import PromptFetchResponse from judgeval.api.api_types.
| ): | |
| ) -> "PromptFetchResponse": |
| if not tags: | ||
| tags = [] |
There was a problem hiding this comment.
While if not tags: works, it's more idiomatic and explicit in Python to check for None with if tags is None:. This avoids potential confusion if an empty list is passed intentionally and you wanted to treat it differently from None (though in this case the outcome is the same).
| if not tags: | |
| tags = [] | |
| if tags is None: | |
| tags = [] |
Backend PR:
https://github.com/JudgmentLabs/judgment/pull/778