-
Notifications
You must be signed in to change notification settings - Fork 107
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
Add a voice agent to the documentation #1631
base: canary
Are you sure you want to change the base?
Conversation
@ananddtyagi is attempting to deploy a commit to the Gloo Team on Vercel. A member of the Team first needs to authorize it. |
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.
❌ Changes requested. Reviewed everything up to 8d80ea6 in 2 minutes and 11 seconds
More details
- Looked at
14
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. fern/docs.yml:12
- Draft comment:
Double ampersand ('&&') might be a typo. - Reason this comment was not posted:
Marked as duplicate.
2. fern/docs.yml:13
- Draft comment:
Exposing API key in URL may be unsafe. - 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 = 50%
This appears to be a frontend JavaScript file being loaded with an API key parameter. Since this is in a public docs file and the URL is publicly accessible, the API key is intentionally public. Frontend JavaScript files commonly include public API keys that are meant to be visible to users. This is likely a public/publishable key rather than a secret key.
I could be wrong about whether this key needs to be public - some API keys should never be exposed even if they're for frontend use. I don't have enough context about the omnivista service to be certain.
However, since this is in a documentation site configuration file and loads a frontend JavaScript widget, it's very likely this is an intentionally public API key meant for frontend use. The fact that it was committed to the repository suggests it's not meant to be secret.
The comment should be deleted as it raises a speculative security concern without strong evidence that exposing this key is actually problematic.
3. fern/docs.yml:13
- Draft comment:
Typo: In the JS URL (line 13), there is a double ampersand ('&&') between the 'name=BAML Support' and 'baseColor=purple' query parameters. Consider reviewing if this is intentional or if it should be a single ampersand. - 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 = 50%
Double ampersands in URL query strings are actually valid and commonly used, especially in JavaScript contexts. The URL appears to be for a JavaScript file inclusion, so the double ampersand could be intentional. Without knowing the specific requirements of the voice-agent service being called, we can't be certain this is actually an error. The comment is speculative and asks for "reviewing if this is intentional" which violates our rules about not asking authors to verify things.
The double ampersand could potentially cause issues with URL parsing in some contexts. Maybe there's a specific reason it needs to be a single ampersand for this service.
Without clear evidence that a single ampersand is required by this specific service, we should not assume the double ampersand is a mistake. The comment is speculative and asks for verification, which violates our rules.
Delete the comment because it is speculative and asks for verification without clear evidence of an actual issue.
Workflow ID: wflow_moITFgggJbQt4Qek
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Add a voice agent from Omnivista to the BAML documentation.