Preserve camelCase brand names in capitalize()#72
Merged
max-rosenblattl merged 2 commits intoMay 5, 2026
Conversation
Contributor
📝 WalkthroughWalkthroughThe ChangesUtility Function Enhancement
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
util.py (1)
16-26: ⚡ Quick winNo automated tests added for the new branching logic
The PR description notes there are no existing tests covering
capitalize(). Given that this function now has four distinct code paths (already uppercase, camelCase guard, capitalise, all non-alpha), at minimum a parametrised unit test covering the documented examples (including the correctedtvOScase) should accompany this change.🧪 Suggested test skeleton
import pytest from util import capitalize `@pytest.mark.parametrize`("text, expected", [ ("iPhone distance values had a mean", "iPhone distance values had a mean"), ("eBay-like", "eBay-like"), ("iOS device", "iOS device"), ("tvOS platform", "tvOS platform"), # currently broken without the fix above ("the watch recorded", "The watch recorded"), ("iphone case", "Iphone case"), ("Already capitalized", "Already capitalized"), ("123abc", "123Abc"), ("", ""), ("a", "A"), ]) def test_capitalize(text, expected): assert capitalize(text) == expected🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@util.py` around lines 16 - 26, The new branching logic in capitalize() lacks automated tests; add a parametrized unit test for the capitalize function that covers all four paths (already uppercase, camelCase guard like "iPhone"/"eBay"/"iOS"/"tvOS", normal capitalization, and all-non-alpha or leading-digits cases). Create a pytest test (e.g., test_capitalize) using pytest.mark.parametrize with cases from the PR comment ("iPhone distance values had a mean", "eBay-like", "iOS device", "tvOS platform", "the watch recorded", "iphone case", "Already capitalized", "123abc", "", "a") and assert capitalize(text) == expected for each to ensure the corrected tvOS behavior and other branches remain covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@util.py`:
- Around line 22-25: The camelCase-preservation guard currently only checks
text[i + 1] and misses multi-lowercase-prefixes like "tvOS"; update the logic in
the block that uses variables text, i, and c to scan forward from i+1 until a
non-alphabetic character or end-of-string and if any character in that scanned
segment is uppercase return text unchanged, otherwise proceed to return text[:i]
+ c.upper() + text[i + 1:]; ensure the scan stops on non-alpha characters and
uses the same variables (text, i, c) so other cases like "iPhone" and "eBay"
keep working.
---
Nitpick comments:
In `@util.py`:
- Around line 16-26: The new branching logic in capitalize() lacks automated
tests; add a parametrized unit test for the capitalize function that covers all
four paths (already uppercase, camelCase guard like
"iPhone"/"eBay"/"iOS"/"tvOS", normal capitalization, and all-non-alpha or
leading-digits cases). Create a pytest test (e.g., test_capitalize) using
pytest.mark.parametrize with cases from the PR comment ("iPhone distance values
had a mean", "eBay-like", "iOS device", "tvOS platform", "the watch recorded",
"iphone case", "Already capitalized", "123abc", "", "a") and assert
capitalize(text) == expected for each to ensure the corrected tvOS behavior and
other branches remain covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+22
to
+25
| # Preserve intentional lowercase-first names like iPhone, eBay, iOS. | ||
| if i + 1 < len(text) and text[i + 1].isupper(): | ||
| return text | ||
| return text[:i] + c.upper() + text[i + 1:] |
Contributor
There was a problem hiding this comment.
camelCase detection misses multi-lowercase-prefix brands (e.g., tvOS)
The guard on Line 23 only checks text[i + 1] — the single character immediately following the first alphabetic character. This correctly preserves iPhone, eBay, and iOS, but fails for tvOS: the first alpha is 't', the next character is 'v' (lowercase), so the guard is skipped and the result is "TvOS".
The PR description explicitly lists tvOS (and by extension any xy…Z prefix) as a target, so this is a functional gap.
Fix: scan forward through the word (stopping at non-alpha characters or end of string) and return early if any character in that word segment is uppercase.
🐛 Proposed fix
- # Preserve intentional lowercase-first names like iPhone, eBay, iOS.
- if i + 1 < len(text) and text[i + 1].isupper():
- return text
- return text[:i] + c.upper() + text[i + 1:]
+ # Preserve intentional lowercase-first names like iPhone, eBay, iOS, tvOS.
+ # Scan the rest of the word for any uppercase letter (camelCase signal).
+ j = i + 1
+ while j < len(text) and text[j].isalpha():
+ if text[j].isupper():
+ return text
+ j += 1
+ return text[:i] + c.upper() + text[i + 1:]Behavior with this fix:
| Input | Before fix | After fix |
|---|---|---|
"tvOS device" |
"TvOS device" ❌ |
"tvOS device" ✅ |
"iPhone data" |
"iPhone data" ✅ |
"iPhone data" ✅ |
"eBay-like" |
"eBay-like" ✅ |
"eBay-like" ✅ |
"the watch" |
"The watch" ✅ |
"The watch" ✅ |
"iphone case" |
"Iphone case" ✅ |
"Iphone case" ✅ |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@util.py` around lines 22 - 25, The camelCase-preservation guard currently
only checks text[i + 1] and misses multi-lowercase-prefixes like "tvOS"; update
the logic in the block that uses variables text, i, and c to scan forward from
i+1 until a non-alphabetic character or end-of-string and if any character in
that scanned segment is uppercase return text unchanged, otherwise proceed to
return text[:i] + c.upper() + text[i + 1:]; ensure the scan stops on non-alpha
characters and uses the same variables (text, i, c) so other cases like "iPhone"
and "eBay" keep working.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Current situation
capitalize()(applied to every generated caption) blindly upper-cases the first alphabetic character, which mangles brand-style names whose first letter is intentionally lowercase. With a channel display name likeiPhone distance, captions render as:Fix: when the first alphabetic character is lowercase and the next character is uppercase (camelCase prefix —
iPhone,eBay,iOS,tvOS, …), leave the text unchanged. Sentence-start lowercase words (the watch …,a stretch …) still get capitalized as before.Verified against:
iPhone distance values had a mean …iPhone distance values had a mean …(unchanged)eBay-likeeBay-like(unchanged)iOS deviceiOS device(unchanged)the watch recordedThe watch recordediphone case(no second-uppercase signal)Iphone caseAlready capitalizedAlready capitalizedRelease Notes
iPhoneintoIPhone.Documentation
No documentation changes — pure behavior fix in
util.capitalize.Testing
Manual: ran
util.capitalizeover the cases above; all match expected output. No existing test coveredcapitalizedirectly.Code of Conduct & Contributing Guidelines