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

chore: [testing] Mic/todos in moderators fix #56

Closed
wants to merge 8 commits into from

Conversation

michaelneale
Copy link
Collaborator

@michaelneale michaelneale commented Sep 24, 2024

adding in fixes from #54 to try to fix broken test on #52

Also, we can't default to contextsummarizer while it hard codes gpt4o #57

@michaelneale michaelneale changed the title Mic/todos in moderators fix chore: Mic/todos in moderators fix Sep 24, 2024
Copy link
Contributor

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

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

🚢

@codefromthecrypt
Copy link
Contributor

actually this has the wrong model thing again..

>               raise httpx.HTTPStatusError(f"{e}\n{response.text}", request=e.request, response=e.response)
E               httpx.HTTPStatusError: Client error '404 Not Found' for url 'http://localhost:11434/v1/chat/completions'
E               For more information check: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/404
E               {"error":{"message":"model \"gpt-4o-mini\" not found, try pulling it first","type":"api_error","param":null,"code":null}}

@michaelneale
Copy link
Collaborator Author

@codefromthecrypt hrm - yes same as last time. Is this same bug? I am totally confused

@@ -40,7 +40,7 @@ class Exchange:
provider: Provider
model: str
system: str
moderator: Moderator = field(default=ContextTruncate())
moderator: Moderator = field(default=ContextSummarizer())
Copy link
Contributor

Choose a reason for hiding this comment

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

let's remove this default so there's no chance of setting it wrong (e.g. better to error due to not inheriting model than using wrong one)

class ContextSummarizer(ContextTruncate):
    def __init__(
        self,
        model: str = "gpt-4o-mini",

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, or it needs to be fixed to take in the correct model

@michaelneale michaelneale changed the title chore: Mic/todos in moderators fix chore: [testing] Mic/todos in moderators fix Sep 25, 2024
@michaelneale michaelneale deleted the mic/todos-in-moderators-fix branch September 25, 2024 00:34
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.

3 participants