Comfy-Org / Comfy-Org/ComfyUI_frontend

i18n: the 4-way split of #15178 silently reverted five reviewed fixes

Open
#15,648 1 comment 0 reactions 1 assignee Claimed by @benceruleanlu View on GitHub
area:i18n
Dominant language
TypeScript
Stars
2k
Forks
704
Avg merge
1d 8h
Merged PRs (30d)
512

Description

## Problem

https://github.com/Comfy-Org/ComfyUI_frontend/pull/15178 was closed and split into
#15228 → #15229 → #15230 → #15236. Diffing the closed PR's tree against the stack tip
(#15236, which is cumulative since each PR is based on the previous head) shows five
changes that were in the reviewed version and are **not** in the stack.

## The reverts

**1. `parseBatchResponse` silently drops non-string values again** — `scripts/i18n/translate.ts`

```diff
- const invalidIds: string[] = []
for (const [key, value] of Object.entries(parsed)) {
- if (typeof value === 'string') {
- record[key] = value
- } else {
- invalidIds.push(key)
- }
- }
- if (invalidIds.length > 0) {
- throw new Error(
- `translation response has non-string values for ids: ${invalidIds.join(', ')}`
- )
+ if (typeof value === 'string') record[key] = value
```

This is the exact defect reported in
https://github.com/Comfy-Org/ComfyUI_frontend/pull/15062#pullrequestreview-4922889854
and answered with "Fixed in the pending local diff: the parser now rejects every
non-string response value and reports all offending ids." A dropped id is later
reported as `no translation returned`, which is a different and misleading diagnosis.

**2. The plural instruction in `buildSystemPrompt` regressed to the wrong rule**

```diff
- - The | character separates plural forms. Use the target locale's appropriate number of forms and translate each form.
+ - The | character separates plural forms. Keep the same number of forms and translate each form.
```

This is the instruction that was corrected after auditing the corpus and finding that
35 of 46 English `|` strings already have a different form count in at least one
shipped locale. The stack now tells the model to do the thing the audit established is
wrong, and it contradicts #15230's own plural-arity handling.

**3. `maxMalformedResponseRetries` regressed 3 → 1** — `scripts/i18n/translate.ts:21`

**4. `pruneCountFloor` regressed 5 → 25** — `scripts/i18n/config.ts:61`. The floor was
lowered to 5 precisely because 25 is over 10% of `settings.json` (234 leaves) and a
fifth of `commands.json` (124 leaves), which makes the guard meaningless for small files.

**5. The explanatory comment on `chunkItems` was dropped** — the note that the character
threshold is an initial heuristic with recursive splitting as the safety net. Without
it the constant reads as a measured bound.

## Reproduce

```
git fetch origin pull/15178/head:a pull/15236/head:b
for f in scripts/i18n/translate.ts scripts/i18n/config.ts; do
diff <(git show a:$f) <(git show b:$f)
done
```

## Note

The split also introduced genuine improvements that are not in #15178 — `strayLinkPattern`,
`hasEmptyPluralForm` comparing source and target, `printCapped`, and the audit skip-set
derived from `pendingLeaves`. This issue is only about the five items that went backwards.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.