Skip to content
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 tweet truncation issue by truncating at complete sentences #388

Merged
merged 2 commits into from
Nov 18, 2024

Conversation

boyaloxer
Copy link
Contributor

Original Code (Before Fix)

The original code was truncating tweets incorrectly, often cutting them off mid-sentence. The logic did not ensure that tweets ended at a complete sentence or whitespace, leading to awkward, incomplete posts.
Problematic Section

const contentLength = 240;

let content = slice.slice(0, contentLength);
// if its bigger than 280, delete the last line
if (content.length > 280) {
content = content.slice(0, content.lastIndexOf("\n"));
}
if (content.length > contentLength) {
// slice at the last period
content = content.slice(0, content.lastIndexOf("."));
}

// if it's still too long, get the period before the last period
if (content.length > contentLength) {
content = content.slice(0, content.lastIndexOf("."));
}

Issues:

It arbitrarily truncated content at contentLength (240 characters) without ensuring logical sentence breaks.
The logic to truncate at the last period (.) or newline (\n) was insufficient for tweets lacking such characters.
The fallback mechanism could still produce incomplete sentences.

Updated Code (After Fix)

We introduced a new helper function, truncateToCompleteSentence, to properly truncate the text. This function ensures that:

Tweets end at a logical sentence break (e.g., a period).
If no periods are found, tweets truncate at the nearest whitespace and append an ellipsis (...).
As a last resort, it hard-truncates the text while adding an ellipsis for readability.

Fixed Section

const MAX_TWEET_LENGTH = 280;

/**

  • Truncate text to fit within the Twitter character limit, ensuring it ends at a complete sentence.
    */
    function truncateToCompleteSentence(text: string): string {
    if (text.length <= MAX_TWEET_LENGTH) {
    return text;
    }

    // Attempt to truncate at the last period within the limit
    const truncatedAtPeriod = text.slice(0, text.lastIndexOf(".", MAX_TWEET_LENGTH) + 1);
    if (truncatedAtPeriod.trim().length > 0) {
    return truncatedAtPeriod.trim();
    }

    // If no period is found, truncate to the nearest whitespace
    const truncatedAtSpace = text.slice(0, text.lastIndexOf(" ", MAX_TWEET_LENGTH));
    if (truncatedAtSpace.trim().length > 0) {
    return truncatedAtSpace.trim() + "...";
    }

    // Fallback: Hard truncate and add ellipsis
    return text.slice(0, MAX_TWEET_LENGTH - 3).trim() + "...";
    }

// Inside the generateNewTweet function:
const formattedTweet = newTweetContent.replaceAll(/\n/g, "\n").trim();
const content = truncateToCompleteSentence(formattedTweet);

Key Improvements

Helper Function: The truncateToCompleteSentence function centralizes truncation logic and ensures consistent handling.
Logical Truncation:
    First tries to truncate at a period (.).
    Falls back to whitespace if no period is found.
    Finally hard-truncates with an ellipsis if necessary.
Replaced Inline Logic: The old logic was replaced with a single call to the helper function, ensuring clean and maintainable code.

@boyaloxer boyaloxer closed this Nov 18, 2024
@boyaloxer boyaloxer changed the title Fix tweet truncation issue by truncating at complete sentences Fix tweet truncation issue by truncating at complete sentences (Under Repair) Nov 18, 2024
@boyaloxer boyaloxer changed the title Fix tweet truncation issue by truncating at complete sentences (Under Repair) Fix tweet truncation issue by truncating at complete sentences Nov 18, 2024
@boyaloxer boyaloxer reopened this Nov 18, 2024
@boyaloxer
Copy link
Contributor Author

Bah. It was working before I removed conflicts.

@boyaloxer boyaloxer closed this Nov 18, 2024
@boyaloxer boyaloxer reopened this Nov 18, 2024
@boyaloxer
Copy link
Contributor Author

Actually might work. Think I had an unrelated error.

@boyaloxer
Copy link
Contributor Author

Yeah think it's good now.

@lalalune lalalune merged commit f1f65ad into elizaOS:main Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants