Safety for Fluent string dependencies

@jessegreenberg

I noted in slack to @jessegreenberg that fluent strings being able to depend on others can create a problem for common strings (or even... sim-specific strings between release branches):

Case 1:

Sim Y is published (common code repo has string A)
Common code introduces string B
Sim X is published (common code repo has string A and string B)
Translator translates sim X (having string A refer to string B, since they are in the same repo and both available in sim X) - that sim works.
Something causes rebuild of sim Y --- it breaks! (string A refers to string B, which doesn't exist in its release branch). Presumably hard build failure.

Case 2:

Sim Y is published (strings A and B exist, but sim Y ONLY uses string A)
Sim X is published (uses both strings)
Translator translates sim X (having string A refer to string B, since they are in the same repo and both available in sim X) - that sim works.
Something causes rebuild of sim Y --- it potentially builds but without string B in the runtime, meaning it gets deployed, and now sim Y is runtime-broken because string A refers to string B which wouldn't be included.

As part of scenerystack/scenerystack#55, I'm not yet handling the string dependencies, but it would be similar.

I believe essentially each build would need to run fluent dependency tracking for every locale (instead of just English, as is done now), to create the set of strings that will be included.

Once this is a bit more figured out, I'll need to adjust SceneryStack to run this (and to include dependencies in every FluentConstant/FluentPattern).

It also came up that because we are wrapping non-a11y strings in FluentConstant (when a legacy pattern isn't detected), these strings can:

Have references added in translations, e.g. { someOtherString }
or more seriously, have escaping issues if the English string doesn't have braces, but a translation does. NOTE this could work for a translator (when translating a sim that is older and doesn't use FluentConstant), but then would BREAK sims that are newer and wrap it in FluentConstant.

So to me, wrapping those strings in FluentConstant is a breaking change, and I'm nervous about release branches going out with SceneryPhetFluent in its current state. Marking as blocks-publication.

Even though #1626 (comment) is unlikely, I think we should avoid using FluentConstant and FluentPattern outside of the a11y key. That will make sure that #1626 (comment) is impossible to encounter. These problems will be something to work on when we support translation of these patterns.

Would it be safer to avoid pruning common code dependency strings, and just let rosetta decide which to show?

I am not sure! But I think this is the band aid we need to fix the issue sim-side.

Details

Subject: [PATCH] Add isLocaleChanging so that we can opt out of work while many LocalizedStringProperties change at once, see https://github.com/phetsims/chipper/issues/1611
---
Index: js/grunt/modulify/generateFluentTypes.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/grunt/modulify/generateFluentTypes.ts b/js/grunt/modulify/generateFluentTypes.ts
--- a/js/grunt/modulify/generateFluentTypes.ts	(revision f87199dcae4abd0c76a23f57b8df79cf9f027dee)
+++ b/js/grunt/modulify/generateFluentTypes.ts	(date 1756420690131)
@@ -309,7 +309,19 @@
 
       const accessor = createAccessor( [ ...pathArr, key ] );
       const getter = `_.get( ${pascalCaseRepo}Strings, '${accessor}' )`;
-      if ( ChipperStringUtils.isLegacyStringPattern( val ) ) {
+
+      // Do not pass the following strings through Fluent:
+      //
+      // - Strings that live outside the `a11y` namespace. They are not currently
+      //   translated; treating them as Fluent may introduce runtime errors if a
+      //   translator inserts Fluent syntax or references another key.
+      //
+      // - Strings that use legacy placeholder patterns such as `{0}` or `{{value}}`.
+      //   These are formatted at runtime with `StringUtils.format` or `StringUtils.fillIn`
+      //   and are therefore incompatible with FluentPattern.
+      //
+      // TODO: https://github.com/phetsims/rosetta/issues/221, Revisit when full Fluent support is available for non-a11y content.
+      if ( ChipperStringUtils.isLegacyStringPattern( val ) || !accessor.startsWith( 'a11y.' ) ) {
 
         // This is a legacy string and is meant to be used with StringUtils.format or
         // StringUtils.fillIn. It should use the LocalizedStringProperty directly.

only a11y (non-translatable) strings use Fluent, see #1626

@samreid

@samreid and I met to discuss. We had several questions about how rosetta and the build packages strings. But we thought that committing #1626 (comment) would give us safety for now.

To see how the patch helps us I did the following test:

Add someUnusedWithUndefinedReference: '{undefinedVariable}' to membrane transport yaml.
Commented out this line in FluentLibrary: // throw new Error( `These messages are not defined: [ ${undefinedMessagesFormatted} ]` );
Run grunt modulify.

This produced the following error when running the sim:

Assertion failed: Fluent errors found when formatting message: ReferenceError: Unknown message: undefinedVariable

After applying the patch in #1626 (comment) the problem went away. I think this demonstrates how a translator could break the sim.

@jonathanolson @samreid does this unblock publications? Should we maintenance release any simulations? The long term approach should be to find a solution for #1626 (comment). What else do we need for the short-medium term?

Run modulify after phetsims/chipper#1626 (comment), see phetsims/chip…

Sounds good, I added a meeting for next week to discuss next steps, if any.

@samreid

Discussion with @samreid and @jonathanolson today.

My understanding is that this is caused by two main issues:

The first is caused by how rosetta works. Translations are submitted to babel. Simulations take strings from babel main, not a branch. But "used" strings are counted only based on references in the simulation SHAs. This is how it is possible for fluent references in babel to reference strings that may not exist in published versions.

The second is that Fluent references and used strings are only computed from the English contents. So fluent dependencies added in non-english languages may be broken because the dependency string can be stripped out as it is not used in the english.

We don't know how to solve these problems yet.

But the patch is sufficient for now. Even though we think it is unlikely that a translator will encounter a problem right now, these changes are all recent and there are only a few sims. It should be easy to maintenance release this fix and that may save us headaches in the future.

only a11y (non-translatable) strings use Fluent, see #1626

Run modulify after phetsims/chipper#1626 (comment), see phetsims/chip…

only a11y (non-translatable) strings use Fluent, see #1626

use TReadOnlyProperty<string> instead of FluentConstant after phetsim…

only a11y (non-translatable) strings use Fluent, see #1626

The batch MR for the short-medium term described in #1626 (comment) was continued in #1630. So the next steps for this issue are the long-term questions about i18n for accessible content and fluent strings described in the original comment.

Navigation Menu

Repository navigation

Safety for Fluent string dependencies #1626

8 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Safety for Fluent string dependencies #1626

Description

Activity

jonathanolson commented on Aug 28, 2025

jessegreenberg commented on Aug 28, 2025

samreid commented on Aug 28, 2025

jessegreenberg commented on Aug 28, 2025

jessegreenberg commented on Aug 29, 2025

8 remaining items

samreid commented on Sep 3, 2025

jessegreenberg commented on Sep 10, 2025

jessegreenberg commented on Sep 12, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions