sweep/add docstrings and comments to agentsapi #21
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Diwank Singh Tomer <[email protected]>
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me!
- Reviewed the entire pull request up to 9b2c58f
- Looked at
375lines of code in5files - Took 49 seconds to review
More info
- Skipped
0files when reviewing. - Skipped posting
1additional comments because they didn't meet confidence threshold of50%.
1. /agents-api/agents_api/web.py:133:
- Assessed confidence :
0% - Comment:
Good job on adding docstrings and comments to the code. They are clear, concise, and informative. This greatly improves the readability and maintainability of the code. - Reasoning:
The PR is adding docstrings and comments to the code, which is a good practice for code readability and maintainability. The comments and docstrings added are clear, concise, and informative. They follow the standard Python docstring format and provide useful information about the purpose and functionality of the code. There are no clear violations of best practices, logical bugs, performance bugs, or security bugs in the changes made in this PR. The changes made in this PR are beneficial to the project.
Workflow ID: wflow_UNTqLuQstgMfa0fb
Not what you expected? You can customize the content of the reviews using rules. Learn more here.
This was referenced Apr 9, 2024
Merged
Merged
Merged
Merged
whiterabbit1983
added a commit
that referenced
this pull request
Apr 10, 2024
* refactor: Rename stuff (#20) Signed-off-by: Diwank Singh Tomer <[email protected]> * refactor: Sweep auto comments (#21) Signed-off-by: Diwank Singh Tomer <[email protected]> * fix: Fix names of services inside docker compose files (#26) Signed-off-by: Diwank Singh Tomer <[email protected]> * f/patch methods (#22) * feat: PATCH methods Signed-off-by: Diwank Singh Tomer <[email protected]> * fix: Use old instructions format * chore: Apply formatting --------- Signed-off-by: Diwank Singh Tomer <[email protected]> Co-authored-by: Dmitry Paramonov <[email protected]> * fix: Fix small problems related to the openapi spec Signed-off-by: Diwank Singh Tomer <[email protected]> --------- Signed-off-by: Diwank Singh Tomer <[email protected]> Co-authored-by: Dmitry Paramonov <[email protected]>
whiterabbit1983
added a commit
that referenced
this pull request
Apr 10, 2024
* feat: Simplify instructions schema Signed-off-by: Diwank Singh Tomer <[email protected]> * feat: memory tests (#23) Co-authored-by: Philip <[email protected]> * fix: Fix small problems related to the openapi spec (#29) * refactor: Rename stuff (#20) Signed-off-by: Diwank Singh Tomer <[email protected]> * refactor: Sweep auto comments (#21) Signed-off-by: Diwank Singh Tomer <[email protected]> * fix: Fix names of services inside docker compose files (#26) Signed-off-by: Diwank Singh Tomer <[email protected]> * f/patch methods (#22) * feat: PATCH methods Signed-off-by: Diwank Singh Tomer <[email protected]> * fix: Use old instructions format * chore: Apply formatting --------- Signed-off-by: Diwank Singh Tomer <[email protected]> Co-authored-by: Dmitry Paramonov <[email protected]> * fix: Fix small problems related to the openapi spec Signed-off-by: Diwank Singh Tomer <[email protected]> --------- Signed-off-by: Diwank Singh Tomer <[email protected]> Co-authored-by: Dmitry Paramonov <[email protected]> * feat: Simplify agent instructions --------- Signed-off-by: Diwank Singh Tomer <[email protected]> Co-authored-by: Diwank Singh Tomer <[email protected]> Co-authored-by: Philip <[email protected]>
whiterabbit1983
added a commit
that referenced
this pull request
Apr 10, 2024
* refactor: Rename stuff (#20) Signed-off-by: Diwank Singh Tomer <[email protected]> * refactor: Sweep auto comments (#21) Signed-off-by: Diwank Singh Tomer <[email protected]> * fix: Fix names of services inside docker compose files (#26) Signed-off-by: Diwank Singh Tomer <[email protected]> * f/patch methods (#22) * feat: PATCH methods Signed-off-by: Diwank Singh Tomer <[email protected]> * fix: Use old instructions format * chore: Apply formatting --------- Signed-off-by: Diwank Singh Tomer <[email protected]> Co-authored-by: Dmitry Paramonov <[email protected]> * fix: Fix small problems related to the openapi spec Signed-off-by: Diwank Singh Tomer <[email protected]> --------- Signed-off-by: Diwank Singh Tomer <[email protected]> Co-authored-by: Dmitry Paramonov <[email protected]>
whiterabbit1983
added a commit
that referenced
this pull request
Apr 10, 2024
* fix: Fix small problems related to the openapi spec (#29) * refactor: Rename stuff (#20) Signed-off-by: Diwank Singh Tomer <[email protected]> * refactor: Sweep auto comments (#21) Signed-off-by: Diwank Singh Tomer <[email protected]> * fix: Fix names of services inside docker compose files (#26) Signed-off-by: Diwank Singh Tomer <[email protected]> * f/patch methods (#22) * feat: PATCH methods Signed-off-by: Diwank Singh Tomer <[email protected]> * fix: Use old instructions format * chore: Apply formatting --------- Signed-off-by: Diwank Singh Tomer <[email protected]> Co-authored-by: Dmitry Paramonov <[email protected]> * fix: Fix small problems related to the openapi spec Signed-off-by: Diwank Singh Tomer <[email protected]> --------- Signed-off-by: Diwank Singh Tomer <[email protected]> Co-authored-by: Dmitry Paramonov <[email protected]> * fix: Remove old patch_agent function --------- Signed-off-by: Diwank Singh Tomer <[email protected]> Co-authored-by: Dmitry Paramonov <[email protected]>
creatorrr
added a commit
that referenced
this pull request
Apr 11, 2024
* aider: Refactored the `Entry` model to support `content` as either a string or a dictionary.
# Aider chat conversation:
USER: I need to refactor the Entry pydantic model because it was originally designed to only handle `content` string but openai api returns content as a dictionary whenever there is a tool_call returned. I dont think it's being currently handled right now. Can you please go through each file one by one and analyze this? Write down your notes please so I can follow along
ASSISTANT: Let's start by analyzing the `Entry` model in the `agents-api/agents_api/common/protocol/entries.py` file. The current implementation assumes that the `content` field is a string. However, as per the new requirements, the OpenAI API can return the `content` as a dictionary when there is a tool call. This means we need to refactor the `Entry` model to support a `content` field that can either be a string or a dictionary.
Here are my notes on the changes that need to be made:
1. Update the `Entry` model to support a `content` field that can be either a string or a dictionary.
2. Adjust the `token_count` property to handle the `content` field when it's a dictionary.
3. Review the usage of the `Entry` model throughout the codebase to ensure that any code that interacts with the `content` field can handle both strings and dictionaries.
4. Update any serialization/deserialization logic that assumes `content` is a string to handle it as a potentially more complex type.
Let's start by refactoring the `Entry` model. Here's the diff for the changes:
```diff
--- agents-api/agents_api/common/protocol/entries.py
+++ agents-api/agents_api/common/protocol/entries.py
@@ ... @@
from typing import Literal, Union
from uuid import UUID, uuid4
from pydantic import BaseModel, Field, computed_field
from agents_api.autogen.openapi_model import Role
EntrySource = Literal["api_request", "api_response", "internal", "summarizer"]
Tokenizer = Literal["character_count"]
+Content = Union[str, dict]
class Entry(BaseModel):
id: UUID = Field(alias="entry_id", default_factory=uuid4)
session_id: UUID
source: EntrySource = Field(default="api_request")
role: Role
- name: str | None = None
- content: str
+ name: Optional[str] = None
+ content: Content
tokenizer: str = Field(default="character_count")
created_at: float = Field(default_factory=lambda: datetime.utcnow().timestamp())
timestamp: float = Field(default_factory=lambda: datetime.utcnow().timestamp())
@computed_field
@Property
def token_count(self) -> int:
if self.tokenizer == "character_count":
- return int(len(self.content) // 3.5)
+ content_length = len(self.content) if isinstance(self.content, str) else len(json.dumps(self.content))
+ return int(content_length // 3.5)
raise NotImplementedError(f"Unknown tokenizer: {self.tokenizer}")
class Config:
use_enum_values = True
```
Next, we will need to review and update the rest of the codebase where the `Entry` model is used to ensure compatibility with the new `content` field type. This will involve checking the `add_entries_query` function in `agents-api/agents_api/models/entry/add_entries.py`, the `session.py` in `agents-api/agents_api/routers/sessions/`, and any other places where `Entry` is used.
Let's proceed with the analysis and make the necessary changes.
* aider: Ensured `content` field is serialized to a JSON string in `add_entries_query` and removed unnecessary `_aux_content` function.
# Aider chat conversation:
USER: sounds good, can you please review the `models/entry/add_entries.py` file? this one preps the query for inserting entries into the database so the content field must be converted to json string either there or as a method on the pydantic model itself
ASSISTANT: To handle the `content` field properly in the `add_entries_query` function, we need to ensure that the content is serialized to a JSON string regardless of whether it's a string or a dictionary. The `json.dumps` function from the `json` module can be used for this purpose. We will also remove the `_aux_content` function since it's no longer necessary.
Here's the diff for the changes:
```diff
--- agents-api/agents_api/models/entry/add_entries.py
+++ agents-api/agents_api/models/entry/add_entries.py
@@ ... @@
from ...common.utils import json
from ...common.protocol.entries import Entry
from ...common.utils.datetime import utcnow
def add_entries_query(entries: list[Entry]) -> str:
- def _aux_content(e: Entry):
- return e.content.replace('"', "'")
-
entries_lst = []
for e in entries:
ts = utcnow().timestamp()
source = json.dumps(e.source)
role = json.dumps(e.role)
name = json.dumps(e.name)
- content = json.dumps(_aux_content(e))
+ content = json.dumps(e.content)
tokenizer = json.dumps(e.tokenizer)
if e.content:
entries_lst.append(
f'[to_uuid("{e.id}"), to_uuid("{e.session_id}"), {source}, {role}, {name}, {content}, {e.token_count}, {tokenizer}, {ts}, {ts}]'
)
if not len(entries_lst):
return "?[] <- [[]]"
entries_query = ",\n".join(entries_lst)
query = f"""
{{
?[entry_id, session_id, source, role, name, content, token_count, tokenizer, created_at, timestamp] <- [
{entries_query}
]
:insert entries {{
entry_id,
session_id,
source,
role,
name, =>
content,
token_count,
tokenizer,
created_at,
timestamp,
}}
:returning
}}
"""
return query
```
This change ensures that the `content` field is properly serialized to a JSON string when constructing the query for inserting entries into the database.
* feat: Simplify instructions schema
Signed-off-by: Diwank Singh Tomer <[email protected]>
* feat: memory tests (#23)
Co-authored-by: Philip <[email protected]>
* fix: Fix small problems related to the openapi spec (#29)
* refactor: Rename stuff (#20)
Signed-off-by: Diwank Singh Tomer <[email protected]>
* refactor: Sweep auto comments (#21)
Signed-off-by: Diwank Singh Tomer <[email protected]>
* fix: Fix names of services inside docker compose files (#26)
Signed-off-by: Diwank Singh Tomer <[email protected]>
* f/patch methods (#22)
* feat: PATCH methods
Signed-off-by: Diwank Singh Tomer <[email protected]>
* fix: Use old instructions format
* chore: Apply formatting
---------
Signed-off-by: Diwank Singh Tomer <[email protected]>
Co-authored-by: Dmitry Paramonov <[email protected]>
* fix: Fix small problems related to the openapi spec
Signed-off-by: Diwank Singh Tomer <[email protected]>
---------
Signed-off-by: Diwank Singh Tomer <[email protected]>
Co-authored-by: Dmitry Paramonov <[email protected]>
* feat: Simplify agent instructions
* feat(agents-api): Move query execution inside models
Signed-off-by: Diwank Singh Tomer <[email protected]>
* feat: Simplify agent instructions
* fix(agents-api): Fix instruction schema mix up
Signed-off-by: Diwank Singh Tomer <[email protected]>
* fix(agents-api): Fix routers for using models correctly
Signed-off-by: Diwank Singh Tomer <[email protected]>
---------
Signed-off-by: Diwank Singh Tomer <[email protected]>
Co-authored-by: Philip <[email protected]>
Co-authored-by: Dmitry Paramonov <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
This PR adds a changelog and license, simplifies the README, and improves code documentation in the
agents-apidirectory.Key points:
CHANGELOG.mdwith documentation of the addition of the v0.2 agents platform.LICENSEfile containing the Apache License.README.mdto only the title 'julep.ai'.env.pyandweb.pyinagents-apiwith docstrings and comments for better code understanding.Generated with ❤️ by ellipsis.dev