feat: Support decimal Arabic-kanji numerals - #21
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughREADME、正規表現、ユーティリティ、テストが更新され、小数点を含むアラビア数字と漢字の大位単位（例: 「8.5万」）の検出と変換が追加されました。 変更内容
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 分 詩
🚥 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 |
5787563 to
65bb9c0
Compare
![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: 65bb9c054b
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/index.ts (1)
62-70:⚠️ Potential issue | 🟠 Major純粋な小数（例:
8.5）まで抽出対象になっています。Line 62 の拡張に対し、Line 69 が整数のみ除外のままなので、
万/億/兆を含まない純粋な小数がfindKanjiNumbers()の結果に混入します。整数と同様に「純粋なアラビア小数」も除外する条件が必要です。修正案
- if ((! item.match(/^[0-9０-９]+$/)) && (item.length && '兆' !== item && '億' !== item && '万' !== item && '萬' !== item)) { + if ((! item.match(/^[0-9０-９]+([.][0-9０-９]+)?$/)) && (item.length && '兆' !== item && '億' !== item && '万' !== item && '萬' !== item)) { return true } else { return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.ts` around lines 62 - 70, The filter in findKanjiNumbers currently only rejects pure Arabic integers (uses /^[0-9０-９]+$/) so pure Arabic decimals like "8.5" slip through; update the filter condition that checks item against /^[0-9０-９]+$/ to also reject pure-decimal forms by using a regex that matches either integer or decimal (e.g. /^[0-9０-９]+(?:[.．][0-9０-９]+)?$/) when evaluating the item inside the match.filter callback (the block referencing regex, match and the anonymous filter function in src/index.ts).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/index.ts`:
- Around line 62-70: The filter in findKanjiNumbers currently only rejects pure
Arabic integers (uses /^[0-9０-９]+$/) so pure Arabic decimals like "8.5" slip
through; update the filter condition that checks item against /^[0-9０-９]+$/ to
also reject pure-decimal forms by using a regex that matches either integer or
decimal (e.g. /^[0-9０-９]+(?:[.．][0-9０-９]+)?$/) when evaluating the item inside
the match.filter callback (the block referencing regex, match and the anonymous
filter function in src/index.ts).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fe76f535-8896-4357-ba35-d0012b0a2ced
📒 Files selected for processing (4)
README.mdsrc/index.tssrc/utils.tstest/test.ts
…anji2number Handle formats like 8.5ä¸where a decimal number precedes a kanji unit character. Fixes geolonia#20
- Use Math.round after unit multiplication to avoid floating-point errors (e.g. 0.29億 producing 28999999.999999996 instead of 29000000) - Exclude standalone decimals without kanji units from findKanjiNumbers results (e.g. "8.5" in "値段は8.5です" should not be extracted)
65bb9c0 to
fc9739e
Compare
ymkjp
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc9739ed48
ℹ️ 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".
| if (unit % decimal.scale !== 0n) { | ||
| throw new TypeError('The attribute of kanji2number() must be a Japanese numeral as integer.') |
There was a problem hiding this comment.
The divisibility guard rejects valid coefficients that only differ by trailing zeros in the fractional part. For example, kanji2number("1.20000万") should evaluate to the same integer as "1.2万" (12000), but unit % decimal.scale !== 0n throws because scale is not reduced before checking. This makes parsing depend on numeric formatting rather than value and will fail on fixed-precision inputs commonly emitted by external systems.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/index.ts`:
- Around line 33-45: The code treats a falsy return from
parseDecimalCoefficient() the same as "no coefficient", which causes invalid
decimal coefficients (e.g. NaN or '' interpreted as missing) to be swallowed;
update the branch in kanji2number where it iterates over largeNumbers to use
raw[key] !== '' as the presence check, call parseDecimalCoefficient(raw[key])
only when raw[key] !== '', and if parseDecimalCoefficient returns null/invalid
for a non-empty raw value throw a TypeError (instead of falling back to treating
the unit as absent); reference parseDecimalCoefficient, largeNumbers, numbers,
raw, and the kanji2number loop to locate and fix the logic so integer
coefficients still work but invalid decimal strings raise an error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7a7f8143-1673-4220-aa4f-22af0b328196
📒 Files selected for processing (4)
README.mdsrc/index.tssrc/utils.tstest/test.ts
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- test/test.ts
| for (const key in largeNumbers) { | ||
| if (numbers[key]) { | ||
| const n = largeNumbers[key] * numbers[key] | ||
| number = number + n | ||
| const decimal = parseDecimalCoefficient(raw[key]) | ||
| if (decimal) { | ||
| const unit = BigInt(largeNumbers[key]) | ||
| if (unit % decimal.scale !== 0n) { | ||
| throw new TypeError('The attribute of kanji2number() must be a Japanese numeral as integer.') | ||
| } | ||
|
|
||
| number = number + Number(decimal.digits * (unit / decimal.scale)) | ||
| } else { | ||
| number = number + largeNumbers[key] * numbers[key] | ||
| } |
There was a problem hiding this comment.
不正な小数係数が 0 として握りつぶされます。
parseDecimalCoefficient() が解釈できない係数でも、Line 34 の truthy 判定だと NaN が「その単位は存在しない」と同じ扱いになります。1百2.5万 は findKanjiNumbers() で 1 トークンとして拾えますが、ここでは 万 の係数が無視されて最終結果が 0 になります。raw[key] !== '' を基準に分岐し、整数係数でも純粋な小数係数でもない値は TypeError にしたほうが安全です。
💡 修正案
for (const key in largeNumbers) {
- if (numbers[key]) {
+ if (raw[key] !== '') {
const decimal = parseDecimalCoefficient(raw[key])
if (decimal) {
const unit = BigInt(largeNumbers[key])
if (unit % decimal.scale !== 0n) {
throw new TypeError('The attribute of kanji2number() must be a Japanese numeral as integer.')
}
number = number + Number(decimal.digits * (unit / decimal.scale))
- } else {
+ } else if (Number.isFinite(numbers[key])) {
number = number + largeNumbers[key] * numbers[key]
+ } else {
+ throw new TypeError('The attribute of kanji2number() must be a Japanese numeral as integer.')
}
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/index.ts` around lines 33 - 45, The code treats a falsy return from
parseDecimalCoefficient() the same as "no coefficient", which causes invalid
decimal coefficients (e.g. NaN or '' interpreted as missing) to be swallowed;
update the branch in kanji2number where it iterates over largeNumbers to use
raw[key] !== '' as the presence check, call parseDecimalCoefficient(raw[key])
only when raw[key] !== '', and if parseDecimalCoefficient returns null/invalid
for a non-empty raw value throw a TypeError (instead of falling back to treating
the unit as absent); reference parseDecimalCoefficient, largeNumbers, numbers,
raw, and the kanji2number loop to locate and fix the logic so integer
coefficients still work but invalid decimal strings raise an error.
1 participant
Summary
findKanjiNumbersandkanji2numberto handle decimal Arabic-kanji numerals like8.5万,25.24億,1.2兆8.5万円for rent,1.2兆円for national budget)Changes
src/index.ts: Add decimal number pattern to thefindKanjiNumbersregex so tokens like8.5万are captured wholesrc/utils.ts: Add decimal number handling inkan2n()so8.5is correctly parsed as a coefficienttest/test.ts: Add test cases covering decimal + 万/億/兆 for bothfindKanjiNumbersandkanji2numberREADME.md: Add decimal examplesOut of scope
2,980万) — callers are expected to strip commas before passing to this library, as noted in findKanjiNumbers misparses mixed Arabic-kanji numerals (e.g. 14万2000, 1億8990万) #20Checklist (optional)
Fixes #20 (comment).
Summary by CodeRabbit
リリースノート