[fix] Automatically fix malformed node def translations

[christian-byrne]

Adds cleanup step to i18n workflow to remove null entries that can occur when translation service encounters timing issues with node output changes.

┆Issue is synchronized with this Notion page by Unito

[webfiltered]

Is this generated? It's doing some weird stuff. Where are we going to get ,\s*null?

For any bash script using sed, verification is reasonably important. It looks like the middle script is the only one that's actually wanted.

[christian-byrne]

Yes this is generated. I'm currently testing by recreating conditions of issue detailed in #3967 and verifying it correctly solves the problem.

[christian-byrne]

More context:

  Where the null values come from:
  The null values appear in the translated locale files (not the English source) when the OpenAI
  translation service encounters timing issues. Specifically, when node outputs are removed/added between
  script runs, the translation service sometimes produces malformed arrays like:
  "outputs": [
    null,
    null,
    null,
    {
      "name": "normal"
    }
  ]

  Instead of the correct object format:
  "outputs": {
    "0": {
      "name": "imagen"
    },
    "1": {
      "name": "mask"
    }
  }

[christian-byrne]

See also: https://github.com/Comfy-Org/ComfyUI_frontend/pull/4019/files#r2119120075

[webfiltered]

Yeah fair enough! Must be doing something weird with arrays and having them serialise as objects. Originally couldn't find anything that matched when I searched.

[christian-byrne]

Changes:

• Convert from using sed inside workflow => using js script which also coerces malformed outputs to correct data structure, in addition to removing syntactically invalid null. Confirmed to work by testing with the malformed outputs generated here

[webfiltered]

Assuming you're verifying by diffing the output of test vs the actual expected output, this looks great.

If the actual file contents haven't been checked, they should be. Reason is it looks like it works by:

1. Removing nulls/undefined from the array and stuffing it back as an object
2. If this invalidates the entire array, it should replace all outputs 👍

However if it simply adds any missing values (like it does elsewhere in i18n), then the first output or three will have the (completely) wrong translation. 🙅

[christian-byrne]

This commit is entirely the result of running the script: 3bee3c5

[huchenlei]

I would expect claude write ts script under scripts/ and call it via tsx like other files under scripts/. Maybe add that instruction to CLAUDE.md?

[webfiltered]

@christian-byrne Reverting this for now. This script must be reviewed using the diff of a 100% valid, working run.

[christian-byrne]

What's expected/correct behavior that the script should do in that screenshot?

[webfiltered]

It's only setting "index 5" on an "array object" - which is a bit weird to begin with but we know about that. It's missing indices 0-4 (removed by script). So they'll be undefined instead of null.

So

{
  "0": {name:"n1"},
  "1": {name:"n2"},
  // ... 2, 3, and 4 go here
  "5": {name:"sanc_banana"}
}

[webfiltered]

Or if it really only has one output, not 6, then just {"0": {name:"sanc_banana"}}

Edit: Or if it's only translating input 6, then that's fine. But in this instance, it's not - the english section had all outputs listed. This is one of a few styles of broken things I noticed.

You can just get a working example (i18n had no timeouts) and a broken example of the same changes from another run, then diff. Is there a reason not to do this? It's basically the only thing worth verifying here.

[christian-byrne]

I need to start from beginning to better understand original error that Terry fixed and work from there.

[christian-byrne]

The immediately apparent solution is to fill the mistaken null values generated by lobe-i18n with the translations from a previous version if it exists (otherwise the untranslated name). It will require re-running the emulation and field extraction, as the order of outputs may have genuinely switched since last time the script ran. And it requires accessing the history of the translation file and parsing the relevant fields to determine if a previous translation can be used.

It is tempting to rewrite the translation automation process using better/maintained tooling. There are so many better tools for this, it doesn't make sense why we need to use a obscure open-source tool when we can easily make our own agent. But even in that case you may still want this type of validation process anyway.

Lowest effort solution is to accept (presumably) small margin of error with current tool and just add null check here:

ComfyUI_frontend/src/services/litegraphService.ts

Lines 203 to 220 in f7093f6

	#addOutputs(outputs: OutputSpec[]) {
	for (const output of outputs) {
	const { name, type, is_list } = output
	const shapeOptions = is_list ? { shape: LiteGraph.GRID_SHAPE } : {}
	const nameKey = `${this.#nodeKey}.outputs.${output.index}.name`
	const typeKey = `dataTypes.${normalizeI18nKey(type)}`
	const outputOptions = {
	...shapeOptions,
	// If the output name is different from the output type, use the output name.
	// e.g.
	// - type ("INT"); name ("Positive") => translate name
	// - type ("FLOAT"); name ("FLOAT") => translate type
	localized_name:
	type !== name ? st(nameKey, name) : st(typeKey, name)
	}
	this.addOutput(name, type, outputOptions)
	}
	}

in which case the lost translation is just simply lost but everything still works/renders otherwise.

[webfiltered]

Can't we just delete corrupt entries and rerun i18n?

[webfiltered]

Actually let's just break this down today quickly.

[christian-byrne]

The corruption is from lobe-cli itself