Skip to content

Instrument upload validation ignores the instrument's language, accepting untranslated bilingual instruments and rejecting language: ['en'] #1712

Open

Description

@joshunrau

$AnyInstrument, the schema that POST /v1/instruments and instrument-interpreter validate bundles against, checks every translatable field (title, description, labels, options, tags) without looking at the instrument's own language. With no language argument, $$InstrumentUIOption accepts either a bare value or an { en, fr } object, whatever language says. So validation goes wrong in both directions.

  • Rejects valid instruments. language: ['en'] is a legal InstrumentLanguage in runtime-core, and the specialized factory $$InstrumentUIOption(schema, ['en']) accepts { en: 'Title' }. $AnyInstrument rejects the same instrument with "expected string, received undefined" at details.title.fr, because its object branch always requires both en and fr.
  • Accepts broken instruments. A bilingual instrument (language: ['en', 'fr']) with an untranslated plain-string label or title passes. translateInstrument then reads 'Favorite Number'['en'], which is undefined, so the field renders with no label and the instrument card with no title. In the other direction, a unilingual language: 'en' instrument with a { en, fr } title passes and is returned untranslated, so an object reaches components that render it as text.

instrument-bundler does not type-check, so bundles from the playground's Upload Bundle dialog or a synced instrument repository reach this schema without the compiler having caught the mismatch.

Where

packages/schemas/src/instrument/instrument.base.ts:75-83:

} else if (typeof language === 'undefined') {
  resolvedSchema = z.union([
    $Schema,
    z.object({
      en: $Schema,
      fr: $Schema
    })
  ]);
}

$AnyInstrument = $$AnyInstrument() (instrument.core.ts:30) builds every field from this branch, and apps/api/src/instruments/instruments.service.ts:102 parses uploads with it. The reader that trusts the result is packages/instrument-utils/src/translate.ts:69 (label: field.label[language]) and :252 (title: details.title[language]).

Reproduce

  1. Take the bilingualFormInstrument stub, change details.title to 'Plain Title' and the favoriteNumber label to 'Favorite Number', then run $AnyInstrument.safeParse(...).
  2. Take the bilingualFileInstrument stub and set language: ['en'], details.title: { en: 't' }, details.description: { en: 'd' }, tags: { en: ['x'] }, and the file group's label: { en: 'File' }. Run $AnyInstrument.safeParse(...).

Actual: case 1 succeeds, and translateInstrument produces details.title === undefined and an unlabeled field. Case 2 fails on the missing fr keys.
Expected: case 1 fails, with issues at the plain-string paths. Case 2 succeeds, matching what $$AnyInstrument(['en']) already does.

Tests

packages/schemas/src/instrument/__tests__/instrument.core.test.ts:

  • it('should reject a bilingual instrument whose title is a plain string, so it cannot reach translation with nothing to translate')
  • it('should accept an instrument authored in a single-language array, as the runtime-core type allows')

E2E in testing/: with the admin token, POST /v1/instruments a bilingual bundle with a plain-string title, and assert a 422.

Suggested fix

Make the no-argument factories language-aware rather than language-agnostic. For example, have $$AnyInstrument() parse language first and then validate against $$AnyInstrument(parsedLanguage), the specialized schema that already behaves correctly. Alternatively, build it as a union of the unilingual and multilingual specializations. apps/outreach always passes a language to the factories when it generates JSON Schema, so changing the no-argument path does not affect its output.

Activity

  1. added
    BugType: existing behavior is wrong
    Difficulty: MediumStraightforward change, about a day or less
    Area: InstrumentsInstrument authoring API, bundling, runtime, rendering, measures and the built-in library
    Area: SharedCross-tier packages (schemas, react-core, etc.) not covered by another area
    on Oct 7, 2026
  2. joshunrau commented on Oct 26, 2026

    @joshunrau
    CollaboratorAuthor

    Still reproduces on main (d86740e). The cited lines have not moved, and no commit since this was filed touches packages/schemas/src/instrument.

    What the suggested fix misses

    Accepting ['en'] at upload breaks the web's instrument lists unless the info schemas change too. $InstrumentInfo and $SeriesInstrumentOverview are built from the no-argument $BaseInstrument (instrument.base.ts:166, :224-250, :304-307). The API returns details, tags and language untranslated (instruments.service.ts:570), and the web parses each response as one array (apps/web/src/hooks/useInstrumentInfoQuery.ts:28, apps/web/src/hooks/useSeriesInstrumentsOverviewQuery.ts:11). So one stored language: ['en'] instrument with a { en } title empties every useInstrumentInfoQuery list that includes it, not just its own row. If it is a series, the /admin/instruments loader awaits the overview parse (apps/web/src/routes/_app/admin/instruments.tsx:392), so the whole page fails. The same object branch makes POST /v1/instruments/series reject ['en'] in its body ($CreateSeriesInstrumentData, instrument.base.ts:276-291).

    Do not build it as a union of specializations. In the repo's zod (3.25.76, zod/v4), a union that fails every branch reports one invalid_union issue at the root with each branch's errors nested inside, so the 422 no longer points at details.title. The branch count also grows with each authoring language, which works against #1378.

    Validate language before dispatching. $$InstrumentUIOption throws a plain Error for a non-unique array (instrument.base.ts:70-72). If the dispatch reads the raw value.language, ['en', 'en'] becomes a 500 instead of a 422.

    Plan

    1. instrument.core.ts:30: define $AnyInstrument as its own constant instead of $$AnyInstrument(). Make it z.unknown().transform(...): parse { language: $InstrumentLanguage }, then $$AnyInstrument(language).safeParse(value), forwarding issues with ctx.addIssue (their paths survive) and returning z.NEVER on failure. Keep it synchronous so safeParse (instrument.core.test.ts:18), safeParseAsync (API) and parseAsync (interpreter) all keep working unchanged. Leave the factory alone: z.toJSONSchema throws on a transform, and apps/outreach converts the factories.
    2. instrument.base.ts:75-83: give the object branch optional keys, so any subset of authoring languages parses. Use z.object, not z.partialRecord: partialRecord rejects unknown keys that today's branch strips, so a stored { en, fr, es } title would fail a whole list. To derive the keys from $InstrumentAuthoringLanguage, change its annotation at packages/schemas/src/core/core.ts:96 from : z.ZodType<...> to satisfies. That also removes a hard-coded en/fr list ahead of Allow for languages in instruments not enabled in ODC (and allow for selective enablement of languages in ODC) #1378. Loosening this branch is safe because createSeries sends its generated bundle through create (instruments.service.ts:198-199), so the series route still gets the strict check.
    3. apps/web/src/routes/_app/group/manage.tsx:314 indexes a duplicate's multilingual existingTitle by the reader's language. Once ['en'] series exist, the duplicate prompt names the existing series undefined for a French reader. Fall back to a language the title has, as describeTitle does (instruments.service.ts:542).
    4. packages/schemas/AGENTS.md:68-74 calls the no-argument result "the permissive union" and says $AnyInstrument is $$AnyInstrument(), and :83-85 says what is written there must survive z.toJSONSchema. Update both in the same commit, since $AnyInstrument becomes the exception.

    No migration is needed. Stored bundles are read back without the schema (instruments.service.ts:427-440), and web validates only in dev (apps/web/src/hooks/useInstrument.ts:20). Broken ones will start failing in the playground preview (apps/playground/src/preview/PreviewApp.tsx:67). A repository re-import of one logs a 422 instead of reaching the conflict, because validation (:102) runs before the id check (:112), but sync keeps the association by merging repo.instrumentIds (apps/api/src/instrument-repos/instrument-repos.service.ts:126). Labels returned by a kind: 'dynamic' render stay unvalidated (instrument.form.ts:111-114, :160-164).

    Tests

    • instrument.core.test.ts: the two cases from the issue. Also: language: 'en' with a { en, fr } title is rejected; a rejection's issue path is ['details', 'title'], which pins dispatch over a union; and ['en', 'en'] gives an issue at ['language'] without throwing.
    • instrument.base.test.ts: $InstrumentInfo accepts language: ['en'] with { en } title, description and tags.
    • apps/web/src/routes/_app/group/__tests__/manage.test.tsx: a duplicate whose existingTitle lacks the reader's language still names the series.
    • E2E in testing/src/specs/admin-instruments.spec.ts, through the series route so no bundle is hand-written. Extend ApiClient.createSeries (testing/src/support/api-client.ts:79-92, which hard-codes language: 'en' and a plain title) to take details and language, and check that a series created with ['en'] and a { en } title is found by searching the series view. That test catches the first point above. createSeries expects a 201, so post ['en', 'fr'] with a plain title directly through apiRequestContext, as the archive test does at :63-67, and expect a 422 (it gets a 201 today).

    Land this before #1378. #1706 adds refinements in the same folder; if they go in the factories, the dispatch picks them up, so the two can land in either order. #1807 also edits api-client.ts.

    Triage note written with Claude Code. Code references point at main @ d86740e.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Area: InstrumentsInstrument authoring API, bundling, runtime, rendering, measures and the built-in libraryArea: SharedCross-tier packages (schemas, react-core, etc.) not covered by another areaBugType: existing behavior is wrongDifficulty: MediumStraightforward change, about a day or lessPriority: MediumReal value; schedule normally

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions