Skip to content

fix(libSceCesCs): map the GB 18030 profiles as the console does - #10739

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
rooklz:fix/cescs-gb18030-console
Oct 27, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
rooklz:fix/cescs-gb18030-console

Conversation

@rooklz

@rooklz rooklz commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

What

The GB 18030 tables followed ICU, and the sources disagree on 26 sequences, so the debt entry called them unmeasured. I measured both profiles on a PS5 with homebrew payloads that load the console's libSceCesCs.sprx: sceCesMbcToUtf32 over every one- and two-byte sequence, every four-byte sequence below 0x8431A530, every 0x81 0x30-led sequence and the supplementary range (sampled, plus its edges), and sceCesUtf32ToMbc over every value from U+0000 to U+110001. On the console:

  • GB18030-2000 matches ICU's gb-18030-2000.ucm in the BMP, as here, but maps no supplementary character: 0x90308130-0xE3329A35 answer 0x805C0020 with length 4, and U+10000 and up do not encode.
  • GB18030-2005 is the 2000 table with 0xA8BC and 0x8135F437 swapped (U+1E3F and U+E7C7), the one difference between the two profiles' sweeps. It keeps the 24 PUA cells that gb-18030-2005.ucm remaps to U+FE10-U+FE19, U+9FB4-U+9FBB and six supplementary ideographs, so those code points take their four-byte forms as in 2000. Above the BMP it maps plane 2 only, U+20000-U+2FFFF (0x95328236-0x9A348431), both ways.

The 2005 profile now uses the 2000 tables with the swap, and Gb18030.hpp drops the 2005 tables. Both profiles' encode and decode sweeps match the console apart from two topics covered elsewhere: the surrogate error code (#9175) and the lengths of rejected characters.

Tested

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The console-measured table consolidation and per-profile tests are substantial. The required conventions check is failing because the PR title is the placeholder fix(libSceCesCs). Please update it to identify the behavior (for example, fix(libSceCesCs): map the GB 18030 profiles as the console does) and let the check rerun. I have not approved while that required check is red.

@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
PR 10056 core/libs/tests/GuestCesChinese.cpp L224-225, L227-228, L230-234, L236-239

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 9912 core/libs/prx/libSceCesCs/Export.cpp
docs/dev/debt/libSceCesCs.md
PR 11614 core/libs/prx/libSceCesCs/Export.cpp
docs/dev/debt/libSceCesCs.md

@MarcZX10

Copy link
Copy Markdown
Contributor

I reviewed exact head cb534c3d82ea5d970f6b5ecd00d9a354ce7253c3. The checkout passes tools/check_conventions.py against current main using the PR title/body, and git diff --check is clean. The hosted conventions job fails for one concrete reason: the title fix(libSceCesCs) is not Conventional Commits syntax. Please rename it, e.g. fix(libSceCesCs): map GB 18030 profiles as measured on PS5, then rerun the check.

There is also a real merge-order dependency with #10056. That PR explicitly depends on #10186, which in turn depends on #10185; these are not all independent updates. I compared the exact trees: #10056 and this PR both change Export.cpp, Gb18030.hpp, GuestCesChinese.cpp, and the CES debt entry, and the Chinese test changes are substantial and overlapping. Please add #10056 to the dependency/merge-order list and confirm the 2005/2000 profile expectations still hold after composing those commits. #10183/#10185/#10186 already list #10739 as same-line merge-order overlap, so this is partly acknowledged; the #10056 chain is the missing dependency.

Current checks: eligibility, intake, labels and report pass; the six host/product jobs are skipped. I did not run the guest CES test locally or independently verify the console measurements. The PR reports a current-host CES test run on its fork, which is useful evidence once the branches are composed.

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hosted conventions check fails on this exact head because the title fix(libSceCesCs) is not Conventional Commits syntax, as reported in my earlier comment. Please rename the PR with a typed scope and description (for example, fix(libSceCesCs): map GB 18030 profiles as measured on PS5) and rerun the conventions check. The merge-order dependency with #10056 also remains to be recorded: it overlaps this PR in the GB18030 tables and Chinese test, and it carries the #10186/#10185 stack. Eligibility, intake, labels and report pass; product test jobs are skipped. I have not run the guest CES test locally.

@rooklz rooklz changed the title fix(libSceCesCs) fix(libSceCesCs): map the GB 18030 profiles as the console does Oct 24, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 25, 2026
@rooklz
rooklz force-pushed the fix/cescs-gb18030-console branch from cb534c3 to 0641e03 Compare October 25, 2026 01:45
@rooklz

rooklz commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

@MarcZX10 thanks. The title is fixed (fix(libSceCesCs): map the GB 18030 profiles as the console does). Rebased onto current main after #10184, #10185 and #10186 landed (head 0641e0355a). The only overlap was the short-buffer check in GuestCesChinese.cpp, which now uses #10185's separate output array. On that head, merged onto main, all 14 guest_ces* tests pass, including the 2000/2005 expectations. #10056 still conflicts with main and carries those three PRs' commits. The description now lists it in the merge order: whichever of #10056 and this PR lands second rebases onto the other and reruns guest_ces_chinese.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 25, 2026

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed updated head 0641e0355a67aac313c30563ffa9fdc569c6caaa. The title now passes conventions, the description records the real overlap/merge order with #10056, and the code and tests reflect the revised 2000/2005 supplementary mappings. The author reports all 14 guest_ces* tests passed on this head after applying it to main; the current hosted run confirms conventions, eligibility, labels, and report, while all product build jobs are skipped. I have not independently run the CES suite or verified the PS5 sweep. I found no new code blocker in the diff, but am leaving this as a comment review rather than treating the author-reported console/test evidence as independently verified.

@rooklz

rooklz commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

@MarcZX10 thanks for the re-review. Both items from your change request are done (the title and the #10056 merge order), so if nothing else is outstanding, could that request be dismissed or replaced with your current review? It's the only thing the merge queue is still waiting on.

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head 0641e0355a67aac313c30563ffa9fdc569c6caaa. The title now meets the repository convention, and the description records the overlap/order with #10056 and its dependent stack; since either PR can land first and the second rebases, Depends on: none is consistent. The GB18030 table change and revised boundary expectations match the reported console behavior. The author reports all 14 guest_ces* tests passing on this head and provides the PS5 sweep details; I did not independently rerun the CES suite or console measurement. Hosted conventions, eligibility, labels, and report checks pass; product jobs are skipped by workflow policy. No remaining blocker found.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 25, 2026
@rooklz
rooklz force-pushed the fix/cescs-gb18030-console branch from 0641e03 to edcc451 Compare October 25, 2026 13:10
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 25, 2026
@rooklz
rooklz force-pushed the fix/cescs-gb18030-console branch from edcc451 to 0641e03 Compare October 26, 2026 00:37
@rooklz
rooklz force-pushed the fix/cescs-gb18030-console branch from 0641e03 to 9a914b7 Compare October 26, 2026 03:02
@rooklz

rooklz commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main at head 9a914b76aa. Independently re-ran the supplementary-plane sweep this PR documents (sceCesMbcToUtf32/sceCesUtf32ToGb on U+10000, U+20000, U+2FFFF, U+10FFFF against GB18030-2000, GB18030-2005, GBK and EUC-CN on the real console) and it corroborates exactly what's in the What section: 2000 maps no supplementary character, 2005 maps plane 2 only (U+20000-U+2FFFF).

Main grew two things since this PR's last head that needed reconciling on rebase, both now folded in and covered by guest_ces_chinese (79/79):

  • The rejected-length convention from fix(libSceCesCs): report the length of a rejected MBCS character as the console does #10722: a 4-byte unassigned GB18030 sequence now reports consumed=4 (was unchanged) to match that measured PR.
  • A couple of test assertions that assumed consumed/produced stay byte-for-byte unchanged on error actually get overwritten in specific paths (rejected<>'s length!=0 branch, and scalarToProfiled's *targetSize=size on DST_BUFFER_END); fixed to match what the implementation actually does.

No change to the 2000/2005 mapping tables or the core "what" of this PR.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 26, 2026
@oneandonlydean oneandonlydean self-assigned this Oct 26, 2026
@babyhuey

Copy link
Copy Markdown
Contributor

Windows run of this head 9a914b76aa7, merged onto main 204c6747d0a (clean merge, local only), with the selection CI's skipped Windows product job uses: Windows 11, Ryzen 7 5800X, RTX 3080, WinLibs GCC 15.2, Release, BUILD_TESTING=ON. The tree was configured fresh after the merge, then select_tests.py select / ninja -f select.ninja selected / --target libs / select_tests.py ctest --parallel 4 --timeout 120, with check_skips.py --job windows (all skips expected).

  • Selected 398 tests: 391 pass, 1 fails, pr_overlap_encoding, which also fails on 204c6747d0a itself on this machine, 6 skip. No failures from this PR.

AI-assisted: yes (Claude Code)

@oneandonlydean
oneandonlydean merged commit 84a4b12 into boykopovar:main Oct 27, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants