Skip to content

feat: add string utility helpers - #36

Open
CristianAmbrosini wants to merge 1 commit into
mainfrom
cristian/test-telemetry-1777037612
Open

feat: add string utility helpers#36
CristianAmbrosini wants to merge 1 commit into
mainfrom
cristian/test-telemetry-1777037612

Conversation

@CristianAmbrosini

Copy link
Copy Markdown
Owner

Test PR to verify Gessie telemetry events (organizationUuid fix)

@sonar-review-dev18

sonar-review-dev18 Bot commented Apr 24, 2026

Copy link
Copy Markdown

Summary

This PR adds a new string_utils.py module with five string utility helper functions:

  • truncate() — safely truncates strings with max_length validation
  • capitalize_words() — capitalizes each word in a string
  • count_vowels() — counts vowels (case-insensitive)
  • reverse_words() — reverses word order
  • is_palindrome() — checks palindrome status, ignoring non-alphanumeric characters

Note: The PR description mentions verifying telemetry events and an organizationUuid fix, but the actual changes are limited to adding these utility helpers. Clarify whether this is the complete scope or if telemetry-related changes are pending.

What reviewers should know

What to review:

  • String handling correctness: edge cases like empty strings, single characters, special characters
  • The is_palindrome() function's cleaning logic removes non-alphanumeric chars—verify this is the intended behavior
  • capitalize_words() handles multiple spaces correctly (splits on whitespace, rejoins with single space)
  • Error handling: only truncate() validates input; others accept any input

To understand context:

  • Check if this module is already imported/used elsewhere, or if it's newly standalone
  • Clarify the relationship to the mentioned telemetry/organizationUuid fix—appears to be a test fixture or utility for broader changes

  • Generate Walkthrough
  • Generate Diagram
Diagram
%%{init: {'flowchart': {'curve': 'catmullRom'}}}%%
flowchart TD
    input([Input string s])

    input --> truncate
    input --> capitalize_words
    input --> count_vowels
    input --> reverse_words
    input --> is_palindrome

    truncate["truncate(s, max_length)\nValidates max_length ≥ 0\nReturns s[:max_length]"]
    capitalize_words["capitalize_words(s)\nSplits on whitespace\nCapitalizes each word\nRejoins with single space"]
    count_vowels["count_vowels(s)\nLowercases s\nCounts chars in 'aeiou'"]
    reverse_words["reverse_words(s)\nSplits on whitespace\nReverses word list\nRejoins with single space"]
    is_palindrome["is_palindrome(s)\nStrips non-alphanumeric\nLowercases\nCompares string to its reverse"]

    truncate --> strOut([str])
    capitalize_words --> strOut
    count_vowels --> intOut([int])
    reverse_words --> strOut
    is_palindrome --> boolOut([bool])
Loading

🗣️ Give feedback

@sonarqube-cloud-dev18

Copy link
Copy Markdown

SonarQube reviewer guide

Review in SonarQube

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@sonar-review-dev18 sonar-review-dev18 Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The utility functions look correct, but the SonarQube Cloud quality gate is failing — coverage on new code is 0% against an 80% threshold. See the reviewer guide above for details.

🗣️ Give feedback

Comment thread string_utils.py

def truncate(s: str, max_length: int) -> str:
if max_length < 0:
raise ValueError("max_length must be non-negative")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test coverage: The ValueError path for negative max_length is untested. A unit test passing -1 should assert this raises ValueError. Without it, any future refactor of this guard could silently drop the validation.

  • Mark as noise

Comment thread string_utils.py
Comment on lines +22 to +24
def is_palindrome(s: str) -> bool:
cleaned = ''.join(c.lower() for c in s if c.isalnum())
return cleaned == cleaned[::-1]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test coverage: The cleaning step (strip non-alphanumeric, lowercase) is non-trivial and has at least two untested edge cases:

  • Empty string input ("") — returns True (vacuously a palindrome); confirm this is intentional.
  • Mixed-case input with punctuation (e.g. "A man, a plan, a canal: Panama") — exercises both the isalnum() filter and the .lower() call together.

A unit test covering each of these would catch regressions in the cleaning logic independently of the palindrome check itself.

  • Mark as noise

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.

1 participant