feat: add string utility functions - #39
Conversation
SummaryThis PR adds a new This is a test PR designed to validate that Gessie telemetry events fire correctly following the metadata.source fix. The utility functions themselves are intentionally simple; the focus is on confirming telemetry instrumentation is working properly. What reviewers should knowFor reviewers:
Key files:
|
SonarQube reviewer guide
|
![sonar-review-dev18[bot]](https://avatars.githubusercontent.com/in/3054993?s=60&v=4){kind=small}
There was a problem hiding this comment.
The new functions are logically sound for typical inputs, but there's a real bug in truncate and all five public functions are missing docstrings, which violates the org guideline requiring docstrings on all public functions.
SonarQube Cloud status: The quality gate is failing — coverage on new code is 0% against an 80% threshold. See the reviewer guide above for details.
| def truncate(text: str, max_length: int, suffix: str = "...") -> str: | ||
| if len(text) <= max_length: | ||
| return text | ||
| return text[: max_length - len(suffix)] + suffix |
There was a problem hiding this comment.
Bug: When max_length is smaller than len(suffix), max_length - len(suffix) is negative. Python interprets a negative slice index as an offset from the end of the string, so text[:-1] silently returns nearly the full string and the result exceeds max_length.
Example: truncate("hello", 2, "...") → max_length - len(suffix) = -1 → "hell" + "..." = "hell..." (7 chars), not 2.
Add a guard at the top of the function:
| def truncate(text: str, max_length: int, suffix: str = "...") -> str: | |
| if len(text) <= max_length: | |
| return text | |
| return text[: max_length - len(suffix)] + suffix | |
| def truncate(text: str, max_length: int, suffix: str = "...") -> str: | |
| if max_length < len(suffix): | |
| raise ValueError(f"max_length ({max_length}) must be >= len(suffix) ({len(suffix)})") | |
| if len(text) <= max_length: | |
| return text | |
| return text[: max_length - len(suffix)] + suffix |
- Mark as noise
| def truncate(text: str, max_length: int, suffix: str = "...") -> str: | ||
| if len(text) <= max_length: | ||
| return text | ||
| return text[: max_length - len(suffix)] + suffix |
There was a problem hiding this comment.
Missing test coverage: No test covers the edge case where max_length < len(suffix). Without a test, the silent wrong-output bug described above will go undetected. A unit test asserting that truncate("hello", 2, "...") raises ValueError (after the fix) would catch regressions here.
- Mark as noise
| """String utility functions.""" | ||
|
|
||
|
|
||
| def reverse_words(sentence: str) -> str: |
There was a problem hiding this comment.
Missing docstring: reverse_words is a public function and must have a docstring describing what it does, its parameters, and its return value (org guideline).
def reverse_words(sentence: str) -> str:
"""Reverse the order of words in a sentence.
Args:
sentence: The input string whose words will be reversed.
Returns:
A string with the words in reversed order.
"""- Mark as noise
| return " ".join(sentence.split()[::-1]) | ||
|
|
||
|
|
||
| def count_vowels(text: str) -> int: |
There was a problem hiding this comment.
Missing docstring: count_vowels is a public function and must have a docstring (org guideline). Describe that it counts ASCII vowels (a, e, i, o, u) case-insensitively, so callers know accented vowels are not counted.
- Mark as noise
| return sum(1 for c in text.lower() if c in "aeiou") | ||
|
|
||
|
|
||
| def truncate(text: str, max_length: int, suffix: str = "...") -> str: |
There was a problem hiding this comment.
Missing docstring: truncate is a public function and must have a docstring (org guideline). Document the suffix default, the max_length constraint relative to suffix length, and the return value.
- Mark as noise
| return text[: max_length - len(suffix)] + suffix | ||
|
|
||
|
|
||
| def to_snake_case(text: str) -> str: |
There was a problem hiding this comment.
Missing docstring: to_snake_case is a public function and must have a docstring (org guideline). Worth noting that consecutive uppercase sequences (e.g. HTMLParser) are treated character-by-character (h_t_m_l_parser), so callers are not surprised by that behaviour.
- Mark as noise
| result.append(char.lower()) | ||
| return "".join(result) | ||
|
|
||
|
|
There was a problem hiding this comment.
Missing docstring: is_palindrome is a public function and must have a docstring (org guideline). Document that non-alphanumeric characters are stripped and comparison is case-insensitive.
- Mark as noise
1 participant
Summary
Add a new
string_utils.pymodule with common string manipulation functions:reverse_words— reverse word order in a sentencecount_vowels— count vowels in texttruncate— truncate text with configurable suffixto_snake_case— convert camelCase to snake_caseis_palindrome— check if text is a palindromePurpose
Test PR to validate Gessie telemetry events are firing correctly after the metadata.source fix.