-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Adding a new prop to pre-process transcripts for human-readability. #14698
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThe pull request introduces enhancements to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
🧹 Outside diff range and nitpick comments (2)
components/gong/actions/retrieve-transcripts-of-calls/retrieve-transcripts-of-calls.mjs (2)
54-60
: Add input validation for millisToTimestamp.The method should validate the input to handle edge cases gracefully.
Consider adding input validation:
millisToTimestamp(millis) { + if (typeof millis !== 'number' || millis < 0) { + throw new Error('Invalid milliseconds value'); + } const totalSeconds = Math.floor(millis / 1000); const minutes = Math.floor(totalSeconds / 60); const seconds = totalSeconds % 60; return `[${minutes.toString().padStart(2, "0")}:${seconds.toString().padStart(2, "0")}]`; }
62-126
: Well-structured transcript simplification implementation.The implementation effectively achieves the PR's objective of enhancing transcript readability through:
- Clear speaker identification
- Chronological ordering with timestamps
- Topic change demarcation
- Maintaining original data when simplified format isn't requested
This provides a good balance between maintaining the original detailed format and offering a more human-readable alternative.
🧰 Tools
🪛 Biome
[error] 81-81: Avoid the use of spread (
...
) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce
) because it causes a time complexity ofO(n^2)
.
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
components/gong/actions/retrieve-transcripts-of-calls/retrieve-transcripts-of-calls.mjs
(3 hunks)components/gong/package.json
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/gong/package.json
🧰 Additional context used
🪛 Biome
components/gong/actions/retrieve-transcripts-of-calls/retrieve-transcripts-of-calls.mjs
[error] 81-81: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
🔇 Additional comments (3)
components/gong/actions/retrieve-transcripts-of-calls/retrieve-transcripts-of-calls.mjs (3)
8-8
: LGTM! Version bump is appropriate.
The version increment reflects the addition of the new transcript formatting feature.
38-44
: LGTM! Well-defined property with clear documentation.
The new property is appropriately defined as optional with a default value, ensuring backward compatibility.
129-149
: LGTM! Clean implementation of the simplified transcript feature.
The async/await usage and control flow are well-implemented.
components/gong/actions/retrieve-transcripts-of-calls/retrieve-transcripts-of-calls.mjs
Outdated
Show resolved
Hide resolved
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
🧹 Outside diff range and nitpick comments (1)
components/gong/actions/retrieve-transcripts-of-calls/retrieve-transcripts-of-calls.mjs (1)
54-60
: Consider handling longer durations.The timestamp conversion works well for minutes and seconds but might need to handle hours for longer recordings.
millisToTimestamp(millis) { const totalSeconds = Math.floor(millis / 1000); - const minutes = Math.floor(totalSeconds / 60); + const hours = Math.floor(totalSeconds / 3600); + const minutes = Math.floor((totalSeconds % 3600) / 60); const seconds = totalSeconds % 60; - - return `[${minutes.toString().padStart(2, "0")}:${seconds.toString().padStart(2, "0")}]`; + + return hours > 0 + ? `[${hours}:${minutes.toString().padStart(2, "0")}:${seconds.toString().padStart(2, "0")}]` + : `[${minutes.toString().padStart(2, "0")}:${seconds.toString().padStart(2, "0")}]`; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
components/gong/actions/retrieve-transcripts-of-calls/retrieve-transcripts-of-calls.mjs
(3 hunks)
🔇 Additional comments (3)
components/gong/actions/retrieve-transcripts-of-calls/retrieve-transcripts-of-calls.mjs (3)
8-8
: LGTM! Version bump is appropriate.
The version increment from 0.0.2 to 0.0.3 aligns with semantic versioning for feature additions.
38-44
: LGTM! Well-defined prop with clear documentation.
The new prop is well-structured with appropriate type, default value, and clear description. Making it optional ensures backward compatibility.
128-148
: LGTM! Clean implementation of the new feature.
The async conversion and conditional processing based on the feature flag are well implemented. The code maintains good separation of concerns.
components/gong/actions/retrieve-transcripts-of-calls/retrieve-transcripts-of-calls.mjs
Show resolved
Hide resolved
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.
Hi @malexanderlim lgtm! Ready for QA!
Hi everyone, all test cases are passed! Ready for release! |
Added a new optional boolean prop returnSimplifiedTranscript to the Gong Retrieve Transcripts action that transforms the raw transcript data into a human-readable format. When enabled, the output presents the conversation in a clean, chronological format with simplified speaker identification (e.g., "Speaker 1"), timestamps in [MM:SS] format, and topic markers where available. The formatted transcript only shows speaker changes when they occur and maintains proper spacing and sectioning, making it much easier to read and analyze conversation flow. This enhancement simplifies downstream use cases where transcript readability is important, while still maintaining the original API response structure when the option is disabled.
Before:
After:
Summary by CodeRabbit
New Features
returnSimplifiedTranscript
for requesting simplified call transcripts.Improvements
run
method to support asynchronous operations and improved control flow for transcript retrieval.Version Update
0.1.1
to0.1.2
.