Skip to content

preflight: ja nPk permutation phrase - #13

Closed
yasumorishima wants to merge 1 commit into
jafrom
ja-permutation
Closed

yasumorishima wants to merge 1 commit into
jafrom
ja-permutation

Conversation

@yasumorishima

@yasumorishima yasumorishima commented Sep 3, 2026 •

Copy link
Copy Markdown
Owner

Preflight only (CI + on-demand CodeRabbit). Not for merge here.

Summary by CodeRabbit

  • Localization

    • Updated Japanese permutation notation speech to follow the displayed argument order and use clearer nPk phrasing.
  • Bug Fixes

    • Corrected Japanese pronunciation of permutation expressions in both ClearSpeak and SimpleSpeak modes.
  • Tests

    • Added coverage confirming notation such as P₂⁵ is read as “5 個から 2 個取る順列.”

The pochhammer speech rule (reached from the nPk P-notation intents) copied
the en argument reversal and glued 順列の between the arguments, so 5 P 2 was
spoken as 2 順列の 5 — word salad in Japanese. en says "2 permutations of 5"
after Wikipedia's "k-permutations of n"; nb and sv rebuilt the phrase
entirely ("antalet permutationer av 2 element ur 5"). Japanese has a
standard school phrase that reads the notation in display order, so the
reversal is simply not needed: 5 個から 2 個取る順列, "permutations taking
2 out of 5".

The rule is marked audit-ignore the way nb's is, since the x:/T: sequence no
longer mirrors en. A speech test covers the msubsup form in ClearSpeak and
SimpleSpeak. audit-translations: untranslated text 3674 -> 3673, rule
differences 28 -> 28, missing/extra rules 0 -> 0.

This is the item daisy#731's description left alone on purpose (順列の — "the
right Japanese wording is a decision rather than a swap").

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016JCoREgn1pJzcbdnUx4iuh
@yasumorishima

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 3, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Sep 3, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 2f601ff9-0289-4875-971c-b13319ef7969

📥 Commits

Reviewing files that changed from the base of the PR and between 316b7d3 and c2a250e.

📒 Files selected for processing (2)
  • Rules/Languages/ja/SharedRules/general.yaml
  • tests/Languages/ja/ja.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The Japanese permutation rule now reads nPk arguments in display order with updated Japanese phrases. A test verifies the output for both ClearSpeak and SimpleSpeak.

Changes

Japanese permutation notation

Layer / File(s) Summary
Permutation speech rule and validation
Rules/Languages/ja/SharedRules/general.yaml, tests/Languages/ja/ja.rs
The rule now reads arguments as “個から” and “個取る順列”. The new test verifies “5 個から 2 個取る順列” for ClearSpeak and SimpleSpeak.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to c2a25

Japanese nPk notation now speaks the school-style display-order phrase, with coverage for both supported speech styles. No current merge-readiness risk remains.

Suggested reviewers: moritz-gross

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the Japanese nPk permutation phrase change. It is concise and related to the main changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (1 skipped: 1 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ja-permutation

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@yasumorishima

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 3, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@yasumorishima

Copy link
Copy Markdown
Owner Author

Preflight done: CI all green (new test matches engine output exactly), CodeRabbit no actionable comments. Submitted upstream as daisy#747.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant