Conversation
- Added Portuguese (pt) language with full number-to-words support - Added 34 server API tests with supertest - Updated README to document 9 languages and REST API - Fixed server to return 400 for invalid inputs (not 500) - Added Russian to server /languages endpoint Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6dcfe3e152
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
index.js
Outdated
| import { english, spanish, french, german, danish, chinese, hindi, russian, portuguese, LANGUAGES } from './languages/index.js'; | ||
|
|
||
| // Re-export language functions | ||
| export { spanish, french, german, danish, chinese, hindi, russian }; |
There was a problem hiding this comment.
The new portuguese function is imported but not re-exported alongside the other language helpers, so import { portuguese } from 'numberstring' will fail even though the README and API imply it exists. This is a user-facing API regression specific to the new language: only toWords(..., { lang: 'pt' }) works, but the named export does not.
Useful? React with 👍 / 👎.
languages/pt.js
Outdated
| // Use "e" (and) for connecting in Portuguese | ||
| const prevPart = parts[i - 1]; | ||
| const currPart = parts[i]; | ||
| // Add "e" when the current part is less than 100 or ends with 00 | ||
| if (currPart && !currPart.includes('mil') && !currPart.includes('ilhão') && !currPart.includes('ilhões')) { |
There was a problem hiding this comment.
Portuguese uses “e” between a higher-scale group and the thousands group (e.g., 1,001,000 → “um milhão e mil”). The join logic explicitly skips inserting “e” when the current part contains mil, which yields outputs like “um milhão mil” for any number where the thousands group is non-zero and a higher group exists. This produces incorrect wording for common inputs such as 1,001,000 or 2,005,000.
Useful? React with 👍 / 👎.
CI runs tests from the root, so server test dependencies need to be in the main package.json. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add portuguese to named exports in index.js (Codex review) - Fix Portuguese join logic to correctly use "e" between higher scale groups (milhão/bilhão) and thousands (mil) - Add comprehensive Portuguese test suite with 8 test cases - Add toWords test for Portuguese language Fixes the issue where "um milhão mil" was output instead of the correct "um milhão e mil" for 1,001,000. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add PR review workflow section with guidance on checking comments - Add supported languages list (9 languages now supported) - Add features section documenting full capabilities - Emphasize running lint/tests before commits as first step Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Both Codex suggestions have been addressed in commit
Thanks @codex for the helpful review! |
|
To use Codex here, create an environment for this repo. |
1 participant
Summary
Test plan
🤖 Generated with Claude Code