feat(memory-store,agents-api): Add special foreign fields to usage table#1480
Draft
feat(memory-store,agents-api): Add special foreign fields to usage table#1480
Conversation
Signed-off-by: Diwank Singh Tomer <diwank.singh@gmail.com>
Contributor
CI Feedback 🧐(Feedback updated until commit 832f644)A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
Ahmad-mtos
suggested changes
Jun 10, 2025
| "user": str(developer.id), | ||
| "tags": developer.tags, | ||
| "custom_api_key": x_custom_api_key, | ||
| "session_id": session_id, |
Contributor
There was a problem hiding this comment.
entry_id isn't being passed.
Comment on lines
+9
to
+13
| ADD COLUMN session_id UUID NULL, | ||
| ADD COLUMN execution_id UUID NULL, | ||
| ADD COLUMN transition_id UUID NULL, | ||
| ADD COLUMN entry_id UUID NULL, | ||
| ADD COLUMN provider TEXT NULL; |
Contributor
There was a problem hiding this comment.
entry_id and transition_id would be enough, though it's not that bad to add them as fields as well.
Comment on lines
139
to
+146
| custom_api_used: bool = False, | ||
| estimated: bool = False, | ||
| metadata: dict[str, Any] | None = None, | ||
| session_id: UUID | None = None, | ||
| execution_id: UUID | None = None, | ||
| transition_id: UUID | None = None, | ||
| entry_id: UUID | None = None, | ||
| provider: str | None = None, |
Contributor
There was a problem hiding this comment.
a lot of variables, better to create a model in typespec to clean up the parameters.
Comment on lines
+134
to
+141
| "input_messages": messages, | ||
| "output_content": [ | ||
| choice.message.content | ||
| for choice in response.choices | ||
| if hasattr(choice, "message") | ||
| and choice.message | ||
| and hasattr(choice.message, "content") | ||
| ] |
Contributor
There was a problem hiding this comment.
adding all messages to the metadata adds a huge unnecessary memory overhead to the usage table.
Comment on lines
+15
to
+23
| def extract_provider_from_model(model: str) -> str | None: | ||
| """ | ||
| Extract the provider from a model name. | ||
|
|
||
| Args: | ||
| model (str): The model name (e.g., "gpt-4", "claude-3-sonnet", "openai/gpt-4") | ||
|
|
||
| Returns: | ||
| str | None: The provider name (e.g., "openai", "anthropic") or None if unknown |
Contributor
There was a problem hiding this comment.
not sure about the functionality of this function. Litellm in general with a self-hosted proxy acts very weirdly towards providers. A better way I think is to determine the provider by the api key provided in the litellm-config.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Signed-off-by: Diwank Singh Tomer diwank.singh@gmail.com