Clicking the "Color" field in the card editor causes the "Pinyin" field's coloring to incorrectly change

[Jacobinski]

Describe the bug

For certain words, such as 宾馆 and 可能, the extension will generate a correctly colored set of pinyin initially, but if the user clicks onto the "Color" field in the card (maybe other fields reproduce this too?), the pinyin coloring will incorrectly change. Please find two demonstration videos attached below.

Screen.Recording.2024-01-15.at.2.47.53.PM.mov

Screen.Recording.2024-01-15.at.2.45.30.PM.mov

Reproduction

1. Install the Chinese Support 3 add-on and disable all other add-ons. Perform the standard setup procedures from the README document in this GitHub repo to add a Chinese (Basic) card-type to your Anki.
2. Create a new Chinese (Basic) card, enable Chinese Support V3, then copy-and-paste 可能 into the Hanzi field. It should auto-populate the other card fields.
3. Expand the Pinyin HTML and verify that it is <span class="tone3">kě</span><span class="tone3">yǐ</span> <!-- ke yi -->
4. While having the Pinyin HTML expanded, click on the Color field and observe that the Pinyin HTML incorrectly changes to <span class="tone3">kěn</span><span class="tone2">éng</span> <!-- ken eng -->

Expected behavior

The Pinyin does not change into an incorrect version of itself.

Specs

• OS: macOS 14.2.1 (23C71)
• Anki Version: Version ⁨23.12.1 (1a1d4d54)⁩
• Chinese Support Version: v0.17.0

[Jacobinski]

There is an interesting unit test, called test_issue_78, in the code which seems to be checking for something similar to my issue. Here is the bug-report associated with the test: jdlorimer/chinese-support-redux#78.

anki-chinese-support-3/tests/test_behavior.py

Lines 37 to 48 in 42af602

	class FormatPinyin(Base):
	def test_issue_78(self):
	note = {'Hanzi': '壮观', 'Pinyin': 'zhuàngguān'}
	expected = (
	'<span class="tone4">zhuàng</span>'
	'<span class="tone1">guān</span> '
	'<!-- zhuang guan -->'
	)
	reformat_transcript(note, 'pinyin', 'pinyin')
	self.assertEqual(note['Pinyin'], expected)
	reformat_transcript(note, 'pinyin', 'pinyin')
	self.assertEqual(note['Pinyin'], expected)

It's not clear to me that this ever was "correctly" fixed. Running a Git Blame on the test shows that it was added in jdlorimer/chinese-support-redux@7a88d51 which seems to be a hack to workaround the issue.

[Jacobinski]

This issue can be reproduced by adding the following new unit test to the class FormatPinyin(Base) in test_behavior.py.

    def test_issue_55(self):
        note = {'Hanzi': '可能', 'Pinyin': 'kěnéng'}
        expected = (
            '<span class="tone3">kě</span>'
            '<span class="tone2">néng</span> '
            '<!-- ke neng -->'
        )
        reformat_transcript(note, 'pinyin', 'pinyin')
        self.assertEqual(note['Pinyin'], expected)
        reformat_transcript(note, 'pinyin', 'pinyin')
        self.assertEqual(note['Pinyin'], expected)

When tested using $ make test, the test fails with error:

======================================================== FAILURES =========================================================
_______________________________________________ FormatPinyin.test_issue_55 ________________________________________________

self = <tests.test_behavior.FormatPinyin testMethod=test_issue_55>

    def test_issue_55(self):
        note = {'Hanzi': '可能', 'Pinyin': 'kěnéng'}
        expected = (
            '<span class="tone3">kě</span>'
            '<span class="tone2">néng</span> '
            '<!-- ke neng -->'
        )
        reformat_transcript(note, 'pinyin', 'pinyin')
>       self.assertEqual(note['Pinyin'], expected)
E       AssertionError: '<span class="tone3">kěn</span><span class="tone2">éng</span> <!-- ken eng -->' != '<span class="tone3">kě</span><span class="tone2">néng</span> <!-- ke neng -->'
E       - <span class="tone3">kěn</span><span class="tone2">éng</span> <!-- ken eng -->
E       ?                       -                                              -
E       + <span class="tone3">kě</span><span class="tone2">néng</span> <!-- ke neng -->
E       ?                                                  +                  +

tests/test_behavior.py:60: AssertionError

[Jacobinski]

Another discovery:

If you go into test_transcribe.py and add a new test for 可能 in class Transcribe(Base) the test suite passes.

However, if you add a test for the same word into class SplitTranscript(Base) then the test suite fails

[Jacobinski]

Based on the above comment, if we look at the fact that the transcribe(['可能'], 'pinyin', 'simp') == ['kě néng'] test passes, and the split_transcript(' kěnéng', 'pinyin') == ['kě néng'] test fails, it seems to suggest that the issue lies in split_transcript.

anki-chinese-support-3/chinese/transcribe.py

Lines 200 to 242 in 42af602

	def split_transcript(transcript, target, grouped=True):
	assert isinstance(transcript, str)
	if target not in ['pinyin', 'pinyin_tw', 'jyutping']:
	raise NotImplementedError(target)
	def _split(pattern, s):
	if search(f'^{pattern}$', s, IGNORECASE):
	return s
	remainder = s.replace("'", '')
	done = []
	while True:
	found = False
	for i in range(len(remainder), 0, -1):
	if search(f'^{pattern}$', remainder[:i], IGNORECASE):
	done.append(remainder[:i])
	remainder = remainder[i:]
	found = True
	break
	if found and remainder:
	continue
	elif remainder:
	done.append(remainder)
	break
	else:
	break
	return ' '.join(done)
	separated = []
	for text in split(NOT_PINYIN_REGEX, transcript):
	if target in ['pinyin', 'pinyin_tw']:
	text = _split(PINYIN_REGEX, text)
	elif target == 'jyutping':
	text = _split(JYUTPING_REGEX, text)
	if grouped:
	separated.append(text)
	else:
	separated.extend(text.split())
	return list(filter(lambda s: s.strip(), separated))

It seems like what this function does it attempt to use regular expressions to split the pinyin word kěnéng into multiple pieces based on initials and finals. Unfortunately, kěn éng and kě néng are both valid splits of the word, so it's up to luck if the correct split is chosen for any given word.

Perhaps we can avoid attempting to split the pinyin based on a regular expression and instead always rely on the Hanzi field as a source of truth?

[Jacobinski]

This is actually kind of tricky to work around since we cannot always rely on the hanzi field as a source of truth. In many cases, there are multiple valid pinyins for a given hanzi (ex. 大都 has two translations: dàdōu means "for the most part"; dàdū: "metropolis") and if the extension chooses the wrong pinyin, the user should be able to overwrite it without the hanzi acting a source-of-truth. And if the user overwrites it, we can no longer rely on the hanzi field as a source-of-truth.

I think that the simplest way to work-around the issue for now it to treat the hanzi as a source-of-truth unless it is changed by the user. In this way, we will avoid having reformat_transcript() overwrite the translation, avoiding my reported bug. If the user changes the pinyin, the new value can act as a source-of-truth and we can run reformat_transcript() against it. This will still have issues for pinyin with multiple valid regular expression splits, but hopefully it will avoid the number of occurrences of this bug. Additionally, users should be able to manually put spaces in their pinyins to avoid this issue.

Please find my implementation of this fix in #56