-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: improve clickable text with sentence content #388
Conversation
WalkthroughThis pull request updates the clickable text functionality by introducing a new Changes
Sequence Diagram(s)sequenceDiagram
participant UI as ClickableText Component
participant Parser as parseClickableText
participant Handler as createClickHandler
participant Msg as sendMessageFromResponse
UI->>Parser: Call parseClickableText(fullText)
Parser-->>UI: Return {clickableText, restText, fullContext}
UI->>Handler: Call createClickHandler(text, fullContext)
Handler->>Msg: sendMessageFromResponse(fullContext or cleanedText)
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
apps/masterbots.ai/lib/clickable-results.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/apps/masterbots.ai/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. ✨ 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 (
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Reviewer's Guide by SourceryThis pull request enhances the clickable text functionality by incorporating the full sentence context into follow-up questions. It modifies the Sequence diagram for Clickable Text InteractionsequenceDiagram
participant User
participant ClickableText Component
participant sendMessageFromResponse
User->>ClickableText Component: Clicks on clickableText
ClickableText Component->>ClickableText Component: createClickHandler(clickableText, fullContext)
ClickableText Component->>sendMessageFromResponse: sendMessageFromResponse(`Explain more in-depth and in detail about ${contextToUse}`)
sendMessageFromResponse-->>User: Displays detailed explanation
Updated class diagram for ParsedTextclassDiagram
class ParsedText {
clickableText: string
restText: string
fullContext: string
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @Bran18 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a configuration option to control the verbosity of the follow-up questions.
- The
parseClickableText
function could be simplified by extracting common logic into smaller, more focused functions.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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: 0
🧹 Nitpick comments (4)
apps/masterbots.ai/lib/clickable-results.ts (2)
149-150
: Improve variable naming for clarity.The variable name
restContent
could be more descriptive to better reflect its purpose.- const restContent = rest.join(phrase); + const remainingContent = rest.join(phrase);
153-153
: Maintain consistent comment style.The comment style varies between lines using
//*
and//?
. Standardize the comment style for better maintainability.- fullContext: phrase + restContent, //* Store the full context + fullContext: phrase + restContent, // Store the full context- fullContext: fullText, //? Stores the full context + fullContext: fullText, // Store the full contextAlso applies to: 176-176
apps/masterbots.ai/components/routes/chat/chat-clickable-text.tsx (2)
29-32
: Improve fallback logic clarity.The fallback logic could be more explicit using nullish coalescing operator.
- const contextToUse = fullContext || cleanedText + const contextToUse = fullContext ?? cleanedText sendMessageFromResponse( - `Explain more in-depth and in detail about ${contextToUse}` + `Please provide more detailed information about "${contextToUse}"` )
140-142
: Standardize fallback pattern usage.The fallback pattern here differs from the one used in
contextToUse
. Consider standardizing the approach.- fullContext || strongContent + fullContext ?? strongContent
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/masterbots.ai/components/routes/chat/chat-clickable-text.tsx
(8 hunks)apps/masterbots.ai/lib/clickable-results.ts
(3 hunks)apps/masterbots.ai/lib/utils.ts
(0 hunks)apps/masterbots.ai/types/types.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- apps/masterbots.ai/lib/utils.ts
🔇 Additional comments (1)
apps/masterbots.ai/types/types.ts (1)
359-363
: Well-structured interface with clear documentation!The
ParsedText
interface is well-defined with clear property descriptions that align with the feature's requirements.
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: 3
🧹 Nitpick comments (1)
apps/masterbots.ai/lib/clickable-results.ts (1)
135-196
: Improve code maintainability and documentation.Consider the following improvements:
- Extract the sentence extraction logic into a reusable function
- Add input validation
- Add JSDoc comments
Here's a suggested refactor:
+/** + * Extracts the first sentence from a text. + * @param text The text to extract from + * @returns The first sentence or the entire text if no sentence boundary is found + */ +function extractFirstSentence(text: string): string { + const match = text.match(/^(.+?\.)(?:\s|$)/); + return match ? match[1] : text; +} + +/** + * Parses text to extract clickable segments and their context. + * @param fullText The text to parse + * @returns Object containing clickable text, rest of the text, and full context + * @throws {Error} If fullText is null or undefined + */ export function parseClickableText(fullText: string): ParsedText { + if (!fullText) { + throw new Error('fullText cannot be null or undefined'); + } + if (typeof fullText === 'string' && fullText.match(/https?:\/\/[^\s]+/)) { // ... rest of the function }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/masterbots.ai/lib/clickable-results.ts
(3 hunks)
🔇 Additional comments (1)
apps/masterbots.ai/lib/clickable-results.ts (1)
1-1
: LGTM! Good architectural change.Moving the
ParsedText
interface to a centralized types file improves maintainability and reduces duplication.
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.
LGTM! The refactor might help to improve the Ai context and response quality with the followingQuestions feature. I added a new audit issue to track this update and audit potential improvements. Merging for tests!
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: 0
🧹 Nitpick comments (3)
apps/masterbots.ai/lib/clickable-results.ts (3)
144-161
: Enhance sentence extraction pattern for better coverage.The current regex pattern only handles periods. Consider expanding it to handle other sentence-ending punctuation marks.
- const firstSentenceMatch = (phrase + restContent).match(/^(.+?\.)(?:\s|$)/) + const firstSentenceMatch = (phrase + restContent).match(/^(.+?[.!?])(?:\s|$)/)
178-183
: Enhance title sentence extraction pattern.Similar to the unique phrases section, expand the pattern to handle more sentence endings.
- const firstSentenceMatch = content.match(/^(.+?\.)(?:\s|$)/); + const firstSentenceMatch = content.match(/^(.+?[.!?])(?:\s|$)/);
178-188
: Improve code organization and readability.Consider extracting the sentence extraction logic into a reusable function since it's used in multiple places.
+function extractFirstSentence(text: string): string { + const match = text.match(/^(.+?[.!?])(?:\s|$)/); + return match ? match[1] : text; +} // In the title handling section: - const firstSentenceMatch = content.match(/^(.+?\.)(?:\s|$)/); - const firstSentence = firstSentenceMatch - ? title + ': ' + firstSentenceMatch[1] - : title + ': ' + content; + const firstSentence = title + ': ' + extractFirstSentence(content);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/masterbots.ai/lib/clickable-results.ts
(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
apps/masterbots.ai/lib/clickable-results.ts (2)
Learnt from: AndlerRL
PR: bitcashorg/masterbots#388
File: apps/masterbots.ai/lib/clickable-results.ts:148-149
Timestamp: 2025-02-24T20:46:17.229Z
Learning: Issue #389 is being used to track items for a code audit in the masterbots project.
Learnt from: AndlerRL
PR: bitcashorg/masterbots#388
File: apps/masterbots.ai/lib/clickable-results.ts:148-149
Timestamp: 2025-02-24T20:46:17.229Z
Learning: Issue #389 is tracking potential improvements and refactoring needs for the ClickableText component, including the evaluation of using context objects instead of multiple parameters.
🔇 Additional comments (2)
apps/masterbots.ai/lib/clickable-results.ts (2)
1-4
: LGTM! Clean import organization.The import changes properly separate type imports from runtime imports.
136-142
: LGTM! Proper URL handling with context.The URL handling logic correctly preserves the full text as context while maintaining existing behavior.
Preview:
Screen.Recording.2025-02-24.at.1.35.06.PM.mov
Screen.Recording.2025-02-24.at.1.36.22.PM.mov
Sidenote: We need to rethink if we are going to make such a heavy follow-up question, they might affect the generation at some point
Summary by Sourcery
This pull request enhances the clickable text feature by providing the full sentence context to the AI when a user clicks on a text element. This allows the AI to generate more detailed and relevant responses.
New Features:
Enhancements:
parseClickableText
function to return the full context of the clickable text, which is then used to provide more detailed information in the follow-up question.Summary by CodeRabbit
Summary by CodeRabbit
fullContext
alongsideclickableText
andrestText
.