-
Notifications
You must be signed in to change notification settings - Fork 35
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
fix: keyterms to keyterm #281
Conversation
WalkthroughThe changes across the codebase update the configuration and API options for transcription by renaming the field from Changes
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (5)
examples/speech-to-text/rest/topic/main.go (1)
36-36
: Consider updating documentation to clarify usage.The consistent renaming from
Keyterms
toKeyterm
across all examples while maintaining the[]string
type suggests a potential change in the API's intended usage. Consider:
- Adding a comment in the examples clarifying whether multiple terms are still supported
- Updating the API documentation to reflect this change
examples/speech-to-text/rest/file/main.go (1)
1-1
: Verify migration strategy for breaking changes.The rename from
Keyterms
toKeyterm
is consistently applied across all files. However, as this is a breaking change affecting both the Go API and JSON payload structure, please ensure:
- The major version is bumped (e.g., from v1.x.x to v2.0.0) following semver
- A migration guide is provided for users
- The changelog clearly documents this breaking change
- Consider providing a temporary backward compatibility layer if feasible
pkg/client/interfaces/v1/types-stream.go (1)
28-28
: Breaking Change: Field renamed fromKeyterms
toKeyterm
.This change modifies a public API field name which could break existing client code. Consider:
- Adding a deprecation notice for the old field name
- Supporting both field names temporarily for backward compatibility
- Documenting this change in the changelog
pkg/client/listen/v1/websocket/new_using_chan.go (1)
73-76
: Update error message to match new field name.The error message uses plural "Keyterms" while the field name is singular "Keyterm". Consider updating for consistency:
- klog.V(1).Info("Keyterms are only supported with nova-3 models.") + klog.V(1).Info("Keyterm is only supported with nova-3 models.")pkg/client/agent/v1/websocket/new_using_chan.go (1)
73-76
: Update error message to match new field name.The error message uses plural "Keyterms" while the field name is singular "Keyterm". Consider updating for consistency:
- klog.V(1).Info("Keyterms are only supported with nova-3 models.") + klog.V(1).Info("Keyterm is only supported with nova-3 models.")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
examples/agent/websocket/simple/main.go
(1 hunks)examples/speech-to-text/rest/callback/callback/main.go
(1 hunks)examples/speech-to-text/rest/file/main.go
(1 hunks)examples/speech-to-text/rest/intent/main.go
(1 hunks)examples/speech-to-text/rest/sentiment/main.go
(1 hunks)examples/speech-to-text/rest/stream/main.go
(1 hunks)examples/speech-to-text/rest/summary/main.go
(1 hunks)examples/speech-to-text/rest/topic/main.go
(1 hunks)examples/speech-to-text/rest/url/main.go
(1 hunks)examples/speech-to-text/websocket/http_callback/main.go
(1 hunks)examples/speech-to-text/websocket/http_channel/main.go
(1 hunks)examples/speech-to-text/websocket/microphone_callback/main.go
(1 hunks)examples/speech-to-text/websocket/microphone_channel/main.go
(1 hunks)examples/speech-to-text/websocket/replay/main.go
(1 hunks)pkg/client/agent/v1/websocket/new_using_chan.go
(1 hunks)pkg/client/interfaces/v1/types-agent.go
(1 hunks)pkg/client/interfaces/v1/types-prerecorded.go
(1 hunks)pkg/client/interfaces/v1/types-stream.go
(1 hunks)pkg/client/listen/v1/rest/client.go
(2 hunks)pkg/client/listen/v1/websocket/new_using_callbacks.go
(1 hunks)pkg/client/listen/v1/websocket/new_using_chan.go
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Go Tests
🔇 Additional comments (16)
examples/speech-to-text/rest/callback/callback/main.go (1)
42-42
:❓ Verification inconclusive
Verify API design intention for Keyterm field.
The field type remains
[]string
but the name change fromKeyterms
toKeyterm
suggests single value usage. This could be a breaking change for clients using multiple terms.
🌐 Web query:
Does the Deepgram API support multiple key terms or is it limited to a single term?
💡 Result:
The Deepgram API supports multiple key terms through both its
keywords
andsearch
features. Here's how they work:
1. Using Multiple Keywords
- Purpose: Improve transcription accuracy for specific words by directing the model to prioritize them.
- Implementation: Pass an array of keywords in your request:
This ensures Deepgram prioritizes transcribing these terms correctly[1][10].transcript = await deepgram.prerecorded.transcribe( { url: "http://your_url.com" }, { keywords: ['keyword1', 'keyword2', 'keyword3'] } )
2. Using Multiple Search Terms
- Purpose: Locate exact phrases or words in transcripts and retrieve their timestamps.
- Implementation:
- While some interfaces (like Make.com’s Deepgram module) may initially appear limited to a single search term[4], you can use URL encoding to pass multiple terms:
&search=term1&search=term2
- Example workaround using HTTP requests:
This generates separate queries for each term[4].encodeURIComponent('term1') + '&search=' + encodeURIComponent('term2')
Key Differences
Feature Use Case Format keywords
Boost transcription accuracy Array of strings search
Find exact terms in transcripts URL-encoded strings
For most use cases,
keywords
is ideal for ensuring critical terms are transcribed correctly, whilesearch
efficiently locates specific phrases post-transcription[10]. Both support multiple entries via their respective methods[1][4][10].Citations:
- 1: https://dev.to/gulshanaggarwal/comment/1n73j
- 2: https://deepgram.com
- 3: https://reverieinc.com/blog/top-deepgram-alternatives-for-speech-to-text-api/
- 4: https://community.make.com/t/use-deepgram-with-multiple-search-terms/37692
- 5: https://github.com/orgs/deepgram/discussions/751
- 6: https://www.transcribetube.com/blog/speech-to-text-api
- 7: https://github.com/orgs/deepgram/discussions/937
- 8: https://developers.deepgram.com/docs/model
- 9: https://siteefy.com/ai-tools/deepgram/
- 10: https://developers.deepgram.com/docs/keywords-vs-search
Clarify Field Naming for Multiple Key Terms
The current implementation at line 42 in
examples/speech-to-text/rest/callback/callback/main.go
uses an array ([]string{"deepgram"}
) for theKeyterm
field. Our web query confirms that the Deepgram API supports multiple key terms when supplied as an array. However, the singular nameKeyterm
may mislead users into thinking only a single term is allowed.
- Action Required: Confirm if the renaming from
Keyterms
toKeyterm
is intentional. If the intent is to support multiple keywords, consider renaming the field (e.g., toKeywords
) or update the documentation and example usage to reflect that multiple entries can be provided.examples/speech-to-text/rest/summary/main.go (1)
36-36
: Implementation is consistent with other examples.examples/speech-to-text/rest/intent/main.go (1)
36-36
: Implementation is consistent with other examples.examples/speech-to-text/rest/sentiment/main.go (1)
36-36
:❓ Verification inconclusive
LGTM! Consider updating documentation.
The change from
Keyterms
toKeyterm
is consistent with the API terminology update. The field type remains[]string
, maintaining backward compatibility.Let's check if there are any documentation files that need to be updated:
🏁 Script executed:
#!/bin/bash # Search for any documentation files containing "keyterms" rg -i "keyterms" --type mdLength of output: 26
LGTM – Code Change Verified, but Please Manually Verify Documentation
The change from
Keyterms
toKeyterm
inexamples/speech-to-text/rest/sentiment/main.go
at line 36 is consistent with the API update, and the field’s type remains[]string
, so there’s no functional change. However, our automated search usingrg -i "keyterms" --type md
did not return any results, which might indicate that documentation updates may not be needed—but please verify manually to ensure no related documentation references have been missed.
- Code Change: Replacement from
Keyterms
toKeyterm
is correct.- Documentation: Verify if any markdown or other documentation files reference the old field name.
examples/speech-to-text/rest/url/main.go (1)
34-34
: LGTM!The change from
Keyterms
toKeyterm
is consistent with the API terminology update while maintaining the same functionality.examples/speech-to-text/rest/stream/main.go (1)
36-36
: LGTM!The change from
Keyterms
toKeyterm
is consistent with the API terminology update while maintaining the same functionality.examples/speech-to-text/websocket/http_callback/main.go (1)
36-36
:❓ Verification inconclusive
Breaking Change: Update affects both REST and WebSocket interfaces.
The change from
Keyterms
toKeyterm
is consistent with the API terminology update. However, since this change affects both REST and WebSocket interfaces, it's important to:
- Document this change in the changelog
- Consider bumping the major version if following semantic versioning
- Update the migration guide for API consumers
Let's check for version and changelog files:
🏁 Script executed:
#!/bin/bash # Search for version and changelog files fd -i "changelog|version"Length of output: 316
Breaking Change Alert: Update Affects Both REST and WebSocket Interfaces
The renaming of the field from
Keyterms
toKeyterm
(while still using a string slice) is consistent with the updated API terminology. However, because this change impacts both REST and WebSocket interfaces (specifically withinLiveTranscriptionOptions
), please ensure the following:
Changelog/Migration Documentation:
Document this breaking change. Although no explicit changelog file was located in the search output, API versioning is managed in files underpkg/api/version/
. If a separate changelog is maintained elsewhere, add the appropriate entry; otherwise, consider updating the version files as needed.Version Bump:
Revisit the versioning strategy (e.g., bump the major version if you’re following semantic versioning) to clearly signal API-breaking changes.Migration Guide:
Update the migration guide to help API consumers understand the change and adjust their implementations accordingly.examples/speech-to-text/websocket/replay/main.go (1)
33-33
: Approve the change but flag as breaking.The rename from
Keyterms
toKeyterm
is consistent with the PR objectives. However, this is a breaking change that will require users to update their code.Please ensure that this breaking change is:
- Documented in the changelog
- Communicated to users
- Released with an appropriate version bump (major version)
examples/speech-to-text/websocket/http_channel/main.go (1)
40-40
: LGTM! Change is consistent.The rename from
Keyterms
toKeyterm
matches the changes in other files.pkg/client/interfaces/v1/types-agent.go (1)
38-40
: Approve the change but note API impact.The rename from
Keyterms
toKeyterm
includes updating the JSON tag from"keyterms"
to"keyterm"
. This change affects both the Go API and the JSON payload structure.Please ensure that:
- The API documentation is updated to reflect this change
- Any example JSON payloads in the documentation are updated
- Integration tests verify the new JSON structure works correctly
examples/speech-to-text/rest/file/main.go (1)
36-36
: LGTM! Change is consistent.The rename from
Keyterms
toKeyterm
matches the changes across both WebSocket and REST implementations.pkg/client/interfaces/v1/types-prerecorded.go (1)
34-34
: LGTM! Field rename consistently applied.The field rename from
Keyterms
toKeyterm
is consistently applied here, maintaining the same type and tags.examples/speech-to-text/websocket/microphone_callback/main.go (1)
129-129
: LGTM! Field renamed from Keyterms to Keyterm.The change is consistent with the PR objective and maintains the same functionality, using the nova-3 model which supports key terms.
pkg/client/listen/v1/websocket/new_using_callbacks.go (1)
73-73
: LGTM! Updated model compatibility check.The field rename maintains the same validation logic, ensuring key terms are only used with nova-3 models.
pkg/client/listen/v1/rest/client.go (1)
79-79
: LGTM! Updated model compatibility checks in REST endpoints.The field rename maintains consistent validation logic for key terms across both DoFile and DoStream methods.
Also applies to: 125-125
examples/speech-to-text/websocket/microphone_channel/main.go (1)
228-228
: LGTM! Field renamed from Keyterms to Keyterm.The change is consistent with the PR objective and maintains the same functionality, using the nova-3 model which supports key terms.
@@ -394,7 +394,7 @@ func main() { | |||
tOptions.Agent.Think.Model = "gpt-4o-mini" | |||
tOptions.Agent.Think.Instructions = "You are a helpful AI assistant." | |||
tOptions.Agent.Listen.Model = "nova-3" | |||
tOptions.Agent.Listen.Keyterms = []string{"Bueller"} | |||
tOptions.Agent.Listen.Keyterm = []string{"Bueller"} |
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.
💡 Verification agent
🧩 Analysis chain
LGTM! Terminology update is consistent.
The change from Keyterms
to Keyterm
aligns with the PR objectives and maintains the same functionality.
Let's verify this change is consistent across the SDK:
🏁 Script executed:
#!/bin/bash
# Description: Check for any remaining instances of "Keyterms" in the codebase
# to ensure the terminology update is complete.
echo "Searching for remaining instances of 'Keyterms'..."
rg -i "keyterms" --type go
Length of output: 706
Action Needed: Inconsistent Terminology Detected
- The change in
examples/agent/websocket/simple/main.go
(line 397) to useKeyterm
is correctly implemented and aligns with the PR objectives. - However, a search of the codebase shows that several log messages still reference
"Keyterms"
:pkg/client/agent/v1/websocket/new_using_chan.go
pkg/client/listen/v1/websocket/new_using_chan.go
pkg/client/listen/v1/websocket/new_using_callbacks.go
pkg/client/listen/v1/rest/client.go
(2 occurrences)
Please verify whether these references should also be updated to Keyterm
for consistency across the SDK, or if they are intentionally retained for backward compatibility. Adjust accordingly.
Proposed changes
Types of changes
What types of changes does your code introduce to the community Go SDK?
Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
Summary by CodeRabbit