feat: add x-title and http-referer header to all openai providers#1300
Merged
mrubens merged 1 commit intoRooCodeInc:mainfrom Mar 5, 2025
Merged
Conversation
🦋 Changeset detectedLatest commit: 589387b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
8d2ba3c to
23fcd69
Compare
Contributor
Author
|
@ellipsis review please |
Contributor
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 23fcd69d6ffbb08d67dde338f12cad4aeb89a5f2 in 2 minutes and 4 seconds
More details
- Looked at
107lines of code in5files - Skipped
0files when reviewing. - Skipped posting
17drafted comments based on config settings.
1. .changeset/wise-pears-join.md:5
- Draft comment:
Ensure your commit message matches our commit message standards (e.g., usingfeat:for features). - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 70%
The comment references our commit message standards which is part of the Development Standards. However, this is a changeset file, not a commit message. Changesets have their own format and don't follow commit message conventions. The type of change is already specified as "patch" in the YAML front matter.
Maybe changeset messages should follow similar conventions to commit messages for consistency? The change does add new functionality which could warrant a 'feat' designation.
Changesets are different from commits and have their own established format. The change type is already properly specified as "patch" in the YAML metadata.
The comment should be deleted as it incorrectly applies commit message standards to a changeset file which has its own format.
2. src/api/providers/__tests__/openai.test.ts:94
- Draft comment:
In the test 'should set default headers correctly', consider using jest.mocked() for improved type-safety instead of jest.requireMock. - Reason this comment was not posted:
Confidence changes required:66%<= threshold70%
None
3. src/api/providers/openai.ts:43
- Draft comment:
UsingURL.canParseis non-standard. Consider wrappingnew URL()in try/catch to validate the base URL. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. src/api/providers/openrouter.ts:158
- Draft comment:
Consider implementing an exponential backoff strategy instead of a fixed delay loop for retrying generation details. - Reason this comment was not posted:
Confidence changes required:66%<= threshold70%
None
5. src/api/providers/requesty.ts:15
- Draft comment:
Ensure the removal of duplicated defaultHeaders aligns with our modular convention; sharing defaultHeaders from openai.ts is a good practice. - Reason this comment was not posted:
Confidence changes required:33%<= threshold70%
None
6. .changeset/wise-pears-join.md:1
- Draft comment:
The changeset description is clear. Please ensure that the header naming (e.g. 'HTTP-Referer' and 'X-Title') align with what's mentioned in the changeset text (lowercase variants) for consistency, even though header names are case‐insensitive. - Reason this comment was not posted:
Confidence changes required:70%<= threshold70%
None
7. src/api/providers/__tests__/openai.test.ts:94
- Draft comment:
The new test for default headers correctly checks that the OpenAI client is initialized with the expected headers. Consider also asserting that baseURL and apiKey are passed correctly if needed for full coverage. - Reason this comment was not posted:
Confidence changes required:70%<= threshold70%
None
8. src/api/providers/openai.ts:18
- Draft comment:
Exporting the defaultHeaders constant improves modularity. For extra safety, consider marking it as immutable (e.g. with Object.freeze) to prevent accidental modifications. - Reason this comment was not posted:
Confidence changes required:70%<= threshold70%
None
9. src/api/providers/openai.ts:23
- Draft comment:
Updating the OpenAiHandlerOptions interface to remove the defaultHeaders property ensures that all providers use the centralized defaultHeaders, reducing redundancy. - Reason this comment was not posted:
Confidence changes required:70%<= threshold70%
None
10. src/api/providers/openrouter.ts:14
- Draft comment:
Importing defaultHeaders from the OpenAI provider promotes consistency across providers. Additionally, if the commented-out usage metrics code (lines 146–152) is no longer needed, consider removing it to keep the codebase clean. - Reason this comment was not posted:
Confidence changes required:70%<= threshold70%
None
11. src/api/providers/requesty.ts:13
- Draft comment:
The removal of the explicit defaultHeaders in the RequestyHandler constructor and relying on the base OpenAiHandler's defaults is a good move for consistency and reducing redundancy. - Reason this comment was not posted:
Confidence changes required:70%<= threshold70%
None
12. .changeset/wise-pears-join.md:2
- Draft comment:
Typographical note: The key "roo-cline" might be a typo. If you intended to refer to a package or component like "roo-client", please verify and update the spelling. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 70%
Without more context about the codebase, I cannot be certain whether "roo-cline" is actually a typo or if it's a legitimate package name. The comment is speculative and asks for verification, which violates our rules. Additionally, if this was a real typo, it would likely cause build failures when trying to publish the package.
The package name could actually be "roo-cline" - perhaps it's a CLI tool (command line). I shouldn't assume it's a typo just because it's similar to another name.
The comment violates multiple rules: it's speculative, asks for verification, and would be caught by the build system if incorrect.
Delete this comment as it's speculative, asks for verification, and any real package name issues would be caught by the build system.
13. .changeset/wise-pears-join.md:5
- Draft comment:
Consider capitalizing 'openai' to 'OpenAI' in the description if it refers to the OpenAI APIs, to maintain consistent naming for proper nouns. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 70%
The comment is about documentation formatting, not code. It's suggesting a minor text change that doesn't affect functionality. Our rules focus on code quality and don't mention documentation formatting standards. The change is purely cosmetic and doesn't impact the clarity of what the changeset describes.
While proper capitalization of company names is generally good practice, this might be overthinking a simple changeset description that's mainly for internal tracking.
The comment doesn't address code quality, security, or any of our priority concerns. It's a trivial documentation suggestion.
Delete the comment as it's purely about documentation formatting and doesn't relate to our key review priorities or code quality standards.
14. src/api/providers/__tests__/openai.test.ts:102
- Draft comment:
It appears that 'Roo-Cline' in the HTTP-Referer header might be a typographical error. Considering the repository name is 'Roo-Code' and the X-Title header is set to 'Roo Code', you might want to double-check if 'Roo-Cline' should instead be 'Roo-Code' for consistency. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 70%
The comment points out an apparent inconsistency between header values. However, there's no clear evidence that this is actually a mistake. The different names could be intentional - Roo-Cline could be a different repository or system component. Without access to the actual configuration or broader system context, we can't determine if this is truly an error.
I might be overlooking a legitimate inconsistency in the codebase's naming conventions. The different names could cause confusion for future developers.
While consistency is important, we don't have enough context to determine if this is actually an error. The different names could be intentionally different for valid architectural reasons.
Delete the comment since we don't have strong evidence that this is actually a mistake. Making assumptions about naming without full context could lead to incorrect changes.
15. src/api/providers/openai.ts:19
- Draft comment:
Typographical error: The header URL currently reads 'https://github.com/RooVetGit/Roo-Cline'. Please verify if this should be 'Roo-Code' to be consistent with the repository name. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 70%
The header value could be intentionally set to "Roo-Cline" - we don't have enough context to know if this is actually wrong. The comment is speculative and asks for verification rather than pointing out a definite issue. Headers like this are often used for analytics or identification purposes, and the exact value may be intentional.
I might be wrong if there's a company standard that all HTTP-Referer headers should match the actual repository name. Also, "Roo-Cline" could be a typo that should be fixed.
Without clear evidence that this is actually wrong or documentation specifying the correct value, we shouldn't ask for verification. The comment is speculative and asks the author to verify something that may be intentional.
Delete this comment as it's speculative and asks for verification without clear evidence of an actual issue.
16. src/api/providers/openai.ts:115
- Draft comment:
Typographical note: In the comment on line 115, 'doesnt' should be corrected to 'doesn't' for proper punctuation. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
17. src/api/providers/requesty.ts:62
- Draft comment:
Typographical error: The sample model description contains 'Excells in coding' which should be corrected to 'excels in coding'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_lGlFrPbGQbYhtnmm
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
- Provides the ability for Open AI compatible gateways, such as LiteLLM, Open Router, Requesty to determine originating app. - Uses standard set by Open Router.
23fcd69 to
589387b
Compare
mrubens
approved these changes
Mar 5, 2025
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
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.
Context
See https://discord.com/channels/1332146336664915968/1332148077263458385/1344252789077053463 for discussion.
Implementation
Screenshots
How to Test
Get in Touch
Important
Adds
x-titleandhttp-refererheaders to OpenAI providers for improved observability and standardization.x-titleandhttp-refererheaders to all OpenAI providers inopenai.ts.openrouter.tsandrequesty.ts.openai.test.tsto verify default headers are set correctly..changeset/wise-pears-join.mdto document the change.This description was created by
for 23fcd69d6ffbb08d67dde338f12cad4aeb89a5f2. It will automatically update as commits are pushed.