Fix incorrect singularization for words ending with -ves (arrives/drives/curves/behaves) and add tests#1704
Fix incorrect singularization for words ending with -ves (arrives/drives/curves/behaves) and add tests#1704jesi318 wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 864f993ce9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR fixes incorrect singularization of words ending in "-ves" by reordering regex rules in the default English vocabulary, adding specific irregular mappings for curve, valve, and safe, and introducing comprehensive test coverage for these edge cases. ChangesSingularization Rule Fix and Test Coverage
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 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 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 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates Humanizer’s inflection rules and tests to improve singularization/pluralization behavior for *ves word endings and correct a test data issue for “safe”.
Changes:
- Added test coverage for
*vessingularization edge cases (e.g., “arrives” → “arrive”, “knives” → “knife”) - Updated default singularization rules for
*vesendings and added irregular entries for “curve/curves” and “safe/safes” - Corrected plural test data from “safe/saves” to “safe/safes”
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tests/Humanizer.Tests/InflectorTests.cs | Adds *ves singularization edge-case tests and fixes “safe” plural test data |
| src/Humanizer/Inflections/Vocabularies.cs | Adjusts *ves singularization rules and adds irregular vocabulary entries |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _default.AddSingular("(.+)ves$", "$1ve"); | ||
| _default.AddSingular("(hive)s$", "$1"); | ||
| _default.AddSingular("(tive)s$", "$1"); | ||
| _default.AddSingular("([lr]|hoo|lea|loa|thie)ves$", "$1f"); | ||
| _default.AddSingular("([lr])ves$", "$1f"); |
| _default.AddIrregular("tie", "ties", matchEnding: false); | ||
| _default.AddIrregular("lens", "lenses"); | ||
| _default.AddIrregular("clove", "cloves"); | ||
| _default.AddIrregular("curve", "curves"); |
| _default.AddIrregular("clove", "cloves"); | ||
| _default.AddIrregular("curve", "curves"); | ||
| _default.AddIrregular("valve", "valves"); | ||
| _default.AddIrregular("safe", "safes"); |
| [InlineData("scarves", "scarf")] | ||
| [InlineData("hooves", "hoof")] | ||
| [InlineData("thieves", "thief")] | ||
| public void Singularize_Ves_EdgeCases(string plural, string expected) => |
| _default.AddSingular("(.+)ves$", "$1ve"); | ||
| _default.AddSingular("(hive)s$", "$1"); | ||
| _default.AddSingular("(tive)s$", "$1"); | ||
| _default.AddSingular("([lr]|hoo|lea|loa|thie)ves$", "$1f"); | ||
| _default.AddSingular("([lr])ves$", "$1f"); |
Here is a checklist you should tick through before submitting a pull request:
mainbranch (more info below)fixes #<the issue number>build.cmdorbuild.ps1and ensure there are no test failuresSummary
Fixes a bug where arrives.Singularize() → arrife (and similar: drives → drife, curves → curf, behaves → behafe etc) by adjusting the singularization rule set and ordering.
Adds unit tests that assert correct singular forms for common -ves cases and true -fe exceptions.
Why this change was made/ Root Cause
_default.AddSingular("([^f])ves$", "$1fe"); is a greedy rule which matched other strings mentioned in the issue above
What I changed
Reworked -ves singular rules in Vocabularies.Defaultso exceptions for nouns that map to -f/-fe (knife→knives, wife→wives, leaf→leaves, etc.) are handled before the general catch-all that converts -ves → -ve (verbs/adjectives like arrive/drive/curve/behave).
Replaced overly-generic pattern(s) that caused verbs to be mis-singularized with a combination of:
Explicit stem-based rules for known -f/-fe nouns, and
A general (.+)ves$ -> $1ve catch-all for remaining -ves cases.
Corrected the incorrect test yield return ["safe", "saves"]; to yield return ["safe", "safes"]; (which makes it grammattically correct according to me) and added a rule for it since it was an edge case . Please let me know if I'm wrong.
Added a focused unit test method Singularize_Ves_EdgeCases in InflectorTests.cs
([lr])ves$ → $1f was separated because this keeps this broad heuristic distinct from explicit stem-based rules, preventing it from interfering with more precise matches and making rule behavior easier to control and debug.