fix(runtime,i18n): field labels read the bundle shape producers write — one shared derivation (#3833)

.changeset/i18n-field-labels-shared-nested-derivation.md

@@ -0,0 +1,62 @@
+---
+"@objectstack/spec": minor
+"@objectstack/runtime": patch
+"@objectstack/service-i18n": patch
+---
+
+fix(runtime,i18n): the dispatcher's field-labels route reads the bundle shape
+producers actually write — one shared derivation (#3833)
+
+`GET /i18n/labels/:object/:locale` served through the dispatcher returned
+`{ labels: {} }` for every provider. Its derivation scanned for flat
+`o.<object>.fields.<field>` keys:
+
+```ts
+const prefix = `o.${objectName}.fields.`;
+for (const [key, value] of Object.entries(translations)) { … }
+```
+
+That dialect was retired by #3778 — no producer has ever written it, and a real
+bundle's top-level keys are the `TranslationData` groups (`objects`, `apps`,
+`messages`, …), so the prefix could not match anything. 4cca74c fixed the
+identical derivation in `service-i18n` and did not reach the dispatcher's copy.
+
+This is not a rare fallback. `getFieldLabels` is optional on `II18nService` and
+**nothing implements it** — not `memory-i18n`, not `file-i18n-adapter` — so the
+dedicated-method branch both surfaces check first is dead in production and this
+derivation is the only path there is. Any stack served by the dispatcher (the
+AppPlugin in-memory provider auto-registered for stacks declaring translation
+bundles) got an empty map, indistinguishable from "this object has no translated
+labels": nothing errored, nothing warned.
+
+Worse than the class it was found next to. #3676, which prompted the check,
+ignored a declared filter and returned the full bundle — a correct superset. This
+returned nothing and said it was fine.
+
+The derivation now lives once, as `resolveObjectFieldLabels` in
+`packages/spec/src/system/i18n-resolver.ts`, alongside the other resolvers that
+read `TranslationData`. Both surfaces call it. Keeping a copy each is precisely
+how one got fixed and the other did not; the next bundle-shape change now has one
+place to land. Fields carrying no non-empty `label` stay omitted rather than
+emitted blank — partial translation is the normal state, and callers merge this
+map over their source labels, where a `''` would erase them.
+
+### The tests were fiction on both sides
+
+The dispatcher's fallback test fed flat `o.contact.fields.first_name` keys and
+asserted labels came back, so it passed on data that cannot occur while
+production returned `{}` — the same failure mode as the client test retired in
+#3676, which asserted a query string was built that no server read. It now feeds
+the nested shape, and was confirmed to fail against the pre-fix code (`expected
+{} to deeply equal { first_name: 'First Name', … }`) rather than merely passing
+after it. The shared helper carries its own unit tests, including one pinning
+that the retired flat dialect resolves to `{}`.
+
+The same suite's mock also declared a `getFieldLabels` no shipped provider has,
+and returned flat-dialect data from `getTranslations`; both now reflect what a
+real provider does, with the divergence noted where it remains deliberate.
+
+Not addressed here, filed separately: `GetFieldLabelsResponseSchema` declares
+`labels` as `Record<string, { label, help?, options? }>`, but both surfaces emit
+`Record<string, string>` — a third declared ≠ enforced gap in the same endpoint,
+and a wire-shape change too breaking to fold into a correctness fix.

packages/runtime/src/domains/i18n.ts

@@ -17,7 +17,8 @@
  */
 
 import { resolveLocale } from '@objectstack/core';
-import { CoreServiceName } from '@objectstack/spec/system';
+import { CoreServiceName, resolveObjectFieldLabels } from '@objectstack/spec/system';
+import type { TranslationData } from '@objectstack/spec/system';
 import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js';
 import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js';
 
@@ -89,15 +90,16 @@ export async function handleI18nRequest(
             const labels = i18nService.getFieldLabels(objectName, locale);
             return { handled: true, response: deps.success({ object: objectName, locale, labels }) };
         }
-        // Fallback: derive field labels from full translation bundle
-        const translations = i18nService.getTranslations(locale);
-        const prefix = `o.${objectName}.fields.`;
-        const labels: Record<string, string> = {};
-        for (const [key, value] of Object.entries(translations)) {
-            if (key.startsWith(prefix)) {
-                labels[key.substring(prefix.length)] = value as string;
-            }
-        }
+        // Fallback: derive field labels from the locale's translation bundle.
+        // This is not really a fallback — `getFieldLabels` is optional on
+        // `II18nService` and nothing implements it, so this is the path every
+        // provider takes. Shared with service-i18n's identical derivation so
+        // the next bundle-shape change cannot fix one copy and miss the other,
+        // which is how this one went on scanning the retired flat
+        // `o.<object>.fields.<field>` dialect after #3778 and returned `{}`
+        // for every provider (#3833).
+        const translations = i18nService.getTranslations(locale) as TranslationData | undefined;
+        const labels = resolveObjectFieldLabels(translations, objectName);
         return { handled: true, response: deps.success({ object: objectName, locale, labels }) };
     }
 

packages/runtime/src/http-dispatcher.test.ts

@@ -1801,7 +1801,16 @@ describe('HttpDispatcher', () => {
         beforeEach(() => {
             mockI18nService = {
                 getLocales: vi.fn().mockReturnValue(['en', 'zh-CN', 'ja']),
-                getTranslations: vi.fn().mockReturnValue({ 'o.account.label': '客户', 'o.account.fields.name': '名称' }),
+                // The nested shape every producer writes and #3778 converged
+                // on. This used to be flat `o.account.label` keys — a dialect
+                // no bundle has ever carried.
+                getTranslations: vi.fn().mockReturnValue({
+                    objects: { account: { label: '客户', fields: { name: { label: '名称' } } } },
+                }),
+                // Declared optional on `II18nService` and implemented by NO
+                // shipped provider — the tests below that assert it is called
+                // cover the dispatcher's handling of a provider that supplies
+                // it, not a path any current stack takes. See #3833.
                 getFieldLabels: vi.fn().mockReturnValue({ name: '名称', industry: '行业' }),
             };
 
@@ -1824,7 +1833,9 @@ describe('HttpDispatcher', () => {
             expect(result.handled).toBe(true);
             expect(result.response?.status).toBe(200);
             expect(result.response?.body?.data?.locale).toBe('zh-CN');
-            expect(result.response?.body?.data?.translations).toEqual({ 'o.account.label': '客户', 'o.account.fields.name': '名称' });
+            expect(result.response?.body?.data?.translations).toEqual({
+                objects: { account: { label: '客户', fields: { name: { label: '名称' } } } },
+            });
             expect(mockI18nService.getTranslations).toHaveBeenCalledWith('zh-CN');
         });
 
@@ -1897,12 +1908,38 @@ describe('HttpDispatcher', () => {
             expect((body.error as { code: unknown }).code).toBe(400);
         });
 
-        it('should fallback to deriving labels from translations when getFieldLabels is missing', async () => {
+        /**
+         * This is the path EVERY provider takes, not an edge case:
+         * `getFieldLabels` is optional on `II18nService` and nothing implements
+         * it — not `memory-i18n`, not `file-i18n-adapter` — so the dedicated-
+         * method branch above is dead in production and this derivation always
+         * runs.
+         *
+         * Its predecessor fed flat `o.contact.fields.first_name` keys and
+         * asserted labels came back. That dialect was retired by #3778 (no
+         * producer ever wrote it), so the test passed on data that cannot
+         * occur while the real path — scanning for an `o.` prefix in a bundle
+         * whose top-level keys are `objects`/`apps`/`messages` — returned `{}`
+         * for every caller. Feeding the shape real bundles actually have is
+         * the whole point of the test (#3833).
+         */
+        it('derives labels from the NESTED bundle shape every producer writes (#3833)', async () => {
             delete mockI18nService.getFieldLabels;
             mockI18nService.getTranslations.mockReturnValue({
-                'o.contact.fields.first_name': 'First Name',
-                'o.contact.fields.email': 'Email',
-                'o.contact.label': 'Contact',
+                objects: {
+                    contact: {
+                        label: 'Contact',
+                        fields: {
+                            first_name: { label: 'First Name' },
+                            email: { label: 'Email', help: 'Primary address' },
+                            // No label — partial translation is the normal
+                            // state, and a blank entry would overwrite the
+                            // caller's source label with an empty string.
+                            phone: { help: 'Mobile preferred' },
+                        },
+                    },
+                },
+                messages: { save: 'Save' },
             });
 
             const result = await dispatcher.handleI18n('/labels/contact/en', 'GET', {}, { request: {} });
@@ -1914,6 +1951,18 @@ describe('HttpDispatcher', () => {
             });
         });
 
+        it('returns {} for an object the locale does not translate, without throwing', async () => {
+            delete mockI18nService.getFieldLabels;
+            mockI18nService.getTranslations.mockReturnValue({
+                objects: { contact: { fields: { email: { label: 'Email' } } } },
+            });
+
+            const result = await dispatcher.handleI18n('/labels/account/en', 'GET', {}, { request: {} });
+            expect(result.handled).toBe(true);
+            expect(result.response?.status).toBe(200);
+            expect(result.response?.body?.data?.labels).toEqual({});
+        });
+
         it('should return 501 when i18n service is not available', async () => {
             (kernel as any).getService = vi.fn().mockResolvedValue(null);
             (kernel as any).services = new Map();

packages/services/service-i18n/src/i18n-service-plugin.ts

@@ -5,6 +5,7 @@ import { wireAuthoredTranslationSync } from '@objectstack/core';
 import type { IHttpServer, IHttpRequest, IHttpResponse } from '@objectstack/spec/contracts';
 import type { II18nService } from '@objectstack/spec/contracts';
 import type { TranslationData } from '@objectstack/spec/system';
+import { resolveObjectFieldLabels } from '@objectstack/spec/system';
 import { FileI18nAdapter } from './file-i18n-adapter.js';
 import type { FileI18nAdapterOptions } from './file-i18n-adapter.js';
 
@@ -234,14 +235,11 @@ export class I18nServicePlugin implements Plugin {
           // That data is NESTED (`objects.<obj>.fields.<field>.label`) — the
           // flat dotted `o.<obj>.fields.<field>` keys this used to scan were a
           // third translation dialect that no producer ever wrote, so the
-          // fallback always returned `{}` (#3778).
+          // fallback always returned `{}` (#3778). The derivation now lives in
+          // `resolveObjectFieldLabels` so the dispatcher's copy of it cannot
+          // drift out of shape again the way it did after that fix (#3833).
           const data = i18n.getTranslations(locale) as TranslationData | undefined;
-          const fields = data?.objects?.[objectName]?.fields ?? {};
-          const labels: Record<string, string> = {};
-          for (const [fieldName, field] of Object.entries(fields)) {
-            const label = field?.label;
-            if (typeof label === 'string' && label.length > 0) labels[fieldName] = label;
-          }
+          const labels = resolveObjectFieldLabels(data, objectName);
           res.json({ success: true, data: { object: objectName, locale, labels } });
         }
       } catch (error: any) {

packages/spec/api-surface.json

@@ -1300,6 +1300,7 @@
     "resolveMetadataFormLabels (function)",
     "resolveMetadataTypeDescription (function)",
     "resolveMetadataTypeLabel (function)",
+    "resolveObjectFieldLabels (function)",
     "resolveSettingsActionConfirm (function)",
     "resolveSettingsActionLabel (function)",
     "resolveSettingsActionSuccess (function)",

packages/spec/src/system/i18n-resolver.test.ts

@@ -11,6 +11,7 @@ import {
   resolveActionResultDialog,
   translateAction,
   translateMetadataDocument,
+  resolveObjectFieldLabels,
 } from './i18n-resolver';
 
 describe('ObjectTranslationDataSchema (_views/_actions extensions)', () => {
@@ -1038,3 +1039,53 @@ describe('translateObject inline actions (objectstack#3370)', () => {
     expect(Object.hasOwn(out, 'actions')).toBe(false);
   });
 });
+
+// ==========================================
+// resolveObjectFieldLabels — the `/i18n/labels/:object/:locale` body
+// ==========================================
+
+describe('resolveObjectFieldLabels (objectstack#3833)', () => {
+  const data = TranslationDataSchema.parse({
+    objects: {
+      contact: {
+        label: 'Contact',
+        fields: {
+          first_name: { label: 'First Name' },
+          email: { label: 'Email', help: 'Primary address' },
+          phone: { help: 'Mobile preferred' },
+        },
+      },
+    },
+    messages: { save: 'Save' },
+  });
+
+  it('enumerates the labels a locale actually translates', () => {
+    expect(resolveObjectFieldLabels(data, 'contact')).toEqual({
+      first_name: 'First Name',
+      email: 'Email',
+    });
+  });
+
+  it('omits fields carrying no label rather than emitting a blank one', () => {
+    // Partial translation is the normal state (see ObjectTranslationDataSchema),
+    // and callers merge this over their source labels — a '' would erase them.
+    expect(resolveObjectFieldLabels(data, 'contact')).not.toHaveProperty('phone');
+  });
+
+  it('returns {} for an untranslated object, a bundle with no objects, and no bundle', () => {
+    expect(resolveObjectFieldLabels(data, 'account')).toEqual({});
+    expect(resolveObjectFieldLabels({ messages: { save: 'Save' } }, 'contact')).toEqual({});
+    expect(resolveObjectFieldLabels(undefined, 'contact')).toEqual({});
+  });
+
+  it('never matches the retired flat `o.<object>.fields.<field>` dialect', () => {
+    // The shape the dispatcher scanned for until #3833. It is not a bundle
+    // any producer writes, and reading it as one is what returned {} in
+    // production while a test built on the same fiction stayed green.
+    const flat = {
+      'o.contact.fields.first_name': 'First Name',
+      'o.contact.label': 'Contact',
+    } as unknown as Parameters<typeof resolveObjectFieldLabels>[0];
+    expect(resolveObjectFieldLabels(flat, 'contact')).toEqual({});
+  });
+});

packages/spec/src/system/i18n-resolver.ts

@@ -724,6 +724,44 @@ function lookupObjectFieldAttr(
   return undefined;
 }
 
+/**
+ * Enumerate an object's translated field labels out of ONE locale's
+ * `TranslationData` — the `GET /i18n/labels/:object/:locale` body.
+ *
+ * Distinct from `lookupObjectFieldAttr` above, which answers "what is this
+ * one field called" against a whole bundle and a locale chain. This answers
+ * "which fields does this locale translate at all", which is what the
+ * endpoint returns and what no per-field lookup can produce.
+ *
+ * Shared deliberately. Both serving surfaces derive this map — the dispatcher
+ * domain body and service-i18n's autonomous route — and they are the only
+ * path there is: `getFieldLabels` is optional on `II18nService` and NOTHING
+ * implements it (neither `memory-i18n` nor `file-i18n-adapter`), so the
+ * "dedicated method" branch both surfaces check first is dead and this
+ * derivation always runs. Keeping one copy each is how the dispatcher was
+ * left scanning the retired flat `o.<object>.fields.<field>` dialect after
+ * #3778 converged the tree on nested `objects.<object>.fields.<field>.label`
+ * and fixed only service-i18n's copy — a scan that cannot match a real bundle,
+ * so that route returned `{}` for every provider (#3833).
+ *
+ * Fields with no non-empty `label` are omitted rather than emitted blank:
+ * partial translation is the normal state (see `ObjectTranslationDataSchema`),
+ * and a caller merges what comes back over its source labels.
+ */
+export function resolveObjectFieldLabels(
+  data: TranslationData | undefined,
+  objectName: string,
+): Record<string, string> {
+  const fields = data?.objects?.[objectName]?.fields;
+  const labels: Record<string, string> = {};
+  if (!fields) return labels;
+  for (const [fieldName, field] of Object.entries(fields)) {
+    const label = field?.label;
+    if (typeof label === 'string' && label.length > 0) labels[fieldName] = label;
+  }
+  return labels;
+}
+
 function lookupObjectFieldOption(
   bundle: TranslationBundle | undefined,
   objectName: string,