-
Notifications
You must be signed in to change notification settings - Fork 337
feat: support OPENROUTER_API_KEY for perplexity #828
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
Added support for using OPENROUTER_API_KEY as a fallback for Perplexity search functionality, addressing the broken search issue (#492).
Key changes:
_browser_perplexity.py: Implemented fallback logic to check forOPENROUTER_API_KEYwhenPERPLEXITY_API_KEYis not available, using the modelperplexity/sonar-provia OpenRouterhas_perplexity_key(): Updated to return true if either API key is available- API key precedence:
PERPLEXITY_API_KEY(env) →PERPLEXITY_API_KEY(config) →OPENROUTER_API_KEY(env/config) - Updated error messages and documentation to reflect the new OpenRouter option
- Added test coverage for both Perplexity and OpenRouter API keys
Issues found:
- Logical error in test assertions using
orinstead ofandwhich will cause incorrect test behavior
Confidence Score: 3/5
- This PR adds a useful fallback mechanism but contains a critical test logic bug that needs fixing
- The core implementation is solid and follows proper fallback precedence, but the test file contains logical errors in assertions (lines 66-68 and 76-78) that use
orinstead ofand. This will cause the tests to pass even when they should fail, potentially masking actual errors. The production code is safe, but the test bugs need to be fixed before merge. - tests/test_browser.py requires immediate attention to fix the logical errors in test assertions
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| gptme/tools/_browser_perplexity.py | 5/5 | Added OpenRouter API key support as fallback for Perplexity search - implementation is clean and follows proper precedence order |
| tests/test_browser.py | 2/5 | Added test for OpenRouter support but contains logical errors in assertions (using or instead of and) |
Sequence Diagram
sequenceDiagram
participant User
participant Browser
participant Perplexity as _browser_perplexity
participant Config as config.toml
participant PerplexityAPI as Perplexity API
participant OpenRouter as OpenRouter API
User->>Browser: search("query", "perplexity")
Browser->>Perplexity: search_perplexity(query)
alt PERPLEXITY_API_KEY in env
Perplexity->>PerplexityAPI: Use PERPLEXITY_API_KEY
PerplexityAPI-->>Perplexity: Search results
else PERPLEXITY_API_KEY in config
Perplexity->>Config: Load config.toml
Config-->>Perplexity: PERPLEXITY_API_KEY
Perplexity->>PerplexityAPI: Use PERPLEXITY_API_KEY
PerplexityAPI-->>Perplexity: Search results
else OPENROUTER_API_KEY in env/config
Perplexity->>Config: Load config.toml (if needed)
Config-->>Perplexity: OPENROUTER_API_KEY
Perplexity->>OpenRouter: Use OPENROUTER_API_KEY<br/>model: perplexity/sonar-pro
OpenRouter-->>Perplexity: Search results
else No API key found
Perplexity-->>Browser: Error message
Browser-->>User: Error: No API key found
end
Perplexity-->>Browser: Search results
Browser-->>User: Search results
4 files reviewed, 2 comments
| assert ( | ||
| "error" not in results.lower() or "Error" not in results | ||
| ), f"Got error: {results}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: logical error in assertion - using or instead of and means the assertion passes if either condition is true, which is incorrect
| assert ( | |
| "error" not in results.lower() or "Error" not in results | |
| ), f"Got error: {results}" | |
| assert "error" not in results.lower(), f"Got error: {results}" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/test_browser.py
Line: 66:68
Comment:
**logic:** logical error in assertion - using `or` instead of `and` means the assertion passes if either condition is true, which is incorrect
```suggestion
assert "error" not in results.lower(), f"Got error: {results}"
```
How can I resolve this? If you propose a fix, please make it concise.| assert ( | ||
| "error" not in results2.lower() or "Error" not in results2 | ||
| ), f"Got error: {results2}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: same logical error - using or instead of and
| assert ( | |
| "error" not in results2.lower() or "Error" not in results2 | |
| ), f"Got error: {results2}" | |
| assert "error" not in results2.lower(), f"Got error: {results2}" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/test_browser.py
Line: 76:78
Comment:
**logic:** same logical error - using `or` instead of `and`
```suggestion
assert "error" not in results2.lower(), f"Got error: {results2}"
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Changes requested ❌
Reviewed everything up to 9ba042e in 2 minutes and 13 seconds. Click for details.
- Reviewed
159lines of code in4files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. docs/lessons/tools/browser.md:51
- Draft comment:
Good documentation update. Consider adding a brief note on why different base URLs/models are used when using PERPLEXITY_API_KEY vs OPENROUTER_API_KEY for extra clarity. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. gptme/tools/_browser_perplexity.py:56
- Draft comment:
Fallback logic for using OPENROUTER_API_KEY is clear. Consider adding a log message here to indicate which key type is being used to aid debugging. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. gptme/tools/_browser_perplexity.py:117
- Draft comment:
When reading the config file, exceptions are silently ignored. Consider logging the exception to help with troubleshooting if the config file fails to load. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. gptme/tools/browser.py:170
- Draft comment:
The error message update now includes OPENROUTER_API_KEY. Ensure consistency with documentation and consider if additional guidance is needed for debugging when neither key is set. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_CGCxAMkAidtlEEcR
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
| results = search("what is gptme", "perplexity") | ||
| assert results, "Should get results from Perplexity" | ||
| assert ( | ||
| "error" not in results.lower() or "Error" not in results |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The assertion using 'or' may not reliably catch error messages. Replace 'or' with 'and' (or check just one case using lower-case) so that the test properly fails when an error is present.
| "error" not in results.lower() or "Error" not in results | |
| "error" not in results.lower() |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
Support OpenRouter keys for Perplexity as suggested in #492 (comment)
Should help us get a search tool working in gptme.ai
Important
Adds support for
OPENROUTER_API_KEYas a fallback for Perplexity searches and updates related documentation and tests.search_perplexity()in_browser_perplexity.pynow supportsOPENROUTER_API_KEYas a fallback ifPERPLEXITY_API_KEYis not set.search_perplexity()andsearch()inbrowser.pyto reflect new API key options.browser.mdto includeOPENROUTER_API_KEYas an option for Perplexity search.test_search_perplexity()intest_browser.pyto test Perplexity search with both API keys.This description was created by
for 9ba042e. You can customize this summary. It will automatically update as commits are pushed.