[Sanilas-ai]

Fixes #1812.

InstrumentOverview and the FormContent info dialog joined instructions with ', ', so an instrument with several instructions (often full sentences) showed them as one comma-separated paragraph.

Changes

• New internal component packages/react-core/src/components/InstrumentInstructions/ (InstrumentInstructions.tsx + index.ts, not exported from src/index.ts). It takes instructions: string[] and renders one <p className="... whitespace-pre-line"> per entry. Plain paragraphs, not a list, as suggested in the issue; whitespace-pre-line keeps line breaks inside an entry. The key is the index because entries may repeat.
• InstrumentOverview.tsx and FormContent.tsx both render the instructions through it, so the markup lives in one place.
• FormContent.tsx: the info button that opens the instructions dialog now has a translated aria-label (Instructions / Instrucciones / Instructions). FormContent.test.tsx finds it with getByRole('button', { name: 'Instructions' }).
• Unit tests: InstrumentInstructions/__tests__/InstrumentInstructions.test.tsx (one <p> per entry, repeated entries, line break kept with whitespace-pre-line, empty list). The InstrumentOverview and FormContent tests assert each entry is its own <p> and the joined string is gone.
• E2E: testing/src/specs/playground.spec.ts renders a form whose clientDetails.instructions is ['Read each question.', 'Answer\nhonestly.'] through generatePlaygroundURL. It checks the overview and, after Begin, the instructions dialog: each entry is found with getByText(..., { exact: true }), and the second entry's inner text is Answer\nhonestly.. That check uses toHaveText(/^Answer\nhonestly\.$/, { useInnerText: true }), because a string toHaveText normalizes whitespace and would pass without the line break.

Verification

• packages/react-core: npx vitest run src/components: 29 files, 178 tests passed. pnpm lint (tsc + eslint) is clean.
• testing: pnpm lint (gen:routes, tsc, eslint) is clean.
• E2E on chromium: the new test passes. I checked that it fails for the right reasons:
  • with main's InstrumentOverview/FormContent (joined paragraph), it fails at the exact lookup of Read each question.;
  • with whitespace-pre-line removed from the component, it fails at the inner-text check (received "Answer honestly.").
• Local caveat: my machine's CPU has no AVX, so MongoDB 5+ (the API's in-memory replica set) can't start, and the setup project can't run. I ran this spec with a local-only Playwright config that starts just the playground dev server and drops the setup dependency (the playground tests don't touch the API). The full pnpm test:e2e still needs to run on a machine with AVX or in CI.

[joshunrau]

Thanks for picking this up. The fix itself looks right: one paragraph per entry, whitespace-pre-line, and index keys are what the triage on #1812 asked for. Two things are needed before it can merge:

1. Add an end-to-end test. Every change here needs one in testing/, and this PR has none. It matters for this fix in particular: happy-dom has no layout engine, so the unit tests can't check that whitespace-pre-line keeps line breaks. Only a real browser can. testing/src/specs/playground.spec.ts already renders instruments through generatePlaygroundURL. Add a form there whose clientDetails.instructions has two entries, for example 'Read each question.' and 'Answer' + newline + 'honestly.'. That spec builds the instrument source in a template literal, so write the newline as \\n: a bare \n puts a raw line break inside a string literal and the instrument won't compile. Then assert that:

   • the overview shows each entry as its own element, using getByText(..., { exact: true }). Without exact, Playwright matches substrings, so the old joined paragraph would pass too.
   • the second entry's innerText() is 'Answer\nhonestly.'.
   • after Begin, the instructions dialog shows the same two entries.

   The info button that opens the dialog (FormContent.tsx:55-57) has no accessible name, so the spec has nothing reliable to select it by. Give it a translated aria-label; the dialog title's t({ en: 'Instructions', es: 'Instrucciones', fr: 'Instructions' }) works. Then switch getInfoButton in FormContent.test.tsx to getByRole('button', { name: 'Instructions' }).

2. Render the instructions in one place. FormContent.tsx:70-75 and InstrumentOverview.tsx:51-56 now have the same markup: the same classes, key and comment. This bug shows why that's a problem: both components had the same join(', '), and both needed the same fix. Please move it into one small internal component, e.g. packages/react-core/src/components/InstrumentInstructions/ (InstrumentInstructions.tsx + index.ts). It takes instructions: string[] and renders one <p className="whitespace-pre-line"> per entry. Use it in both places, give it a unit test in its own __tests__/ folder, and don't export it from src/index.ts.

Reviewed at commit 221afa0.

[Sanilas-ai]

Thanks for the detailed review! Both points are done in 6689ddb, rebased on main:

1. E2E test in testing/src/specs/playground.spec.ts, as you described: two instructions, the second 'Answer\\nhonestly.' (escaped inside the template literal), exact matches in the overview and in the dialog after Begin, plus an inner-text check for the line break. The info button now has a translated aria-label, and FormContent.test.tsx uses getByRole('button', { name: 'Instructions' }). One small deviation: ESLint's playwright/prefer-web-first-assertions rewrites expect(await x.innerText()).toBe(...), and a string toHaveText normalizes whitespace. So the check is toHaveText(/^Answer\nhonestly\.$/, { useInnerText: true }). I confirmed it fails with whitespace-pre-line removed.
2. One place for the markup: packages/react-core/src/components/InstrumentInstructions/ with its own __tests__/. It's used by both components and not exported from src/index.ts.

Caveat: my CPU has no AVX, so the API's MongoDB can't start here. I ran the playground spec with only the playground server (details in the PR description). The full e2e run still needs CI or another machine.

[joshunrau]

Thanks for the quick turnaround. Both points from the last review are done. The e2e test fails for the right reasons, using a useInnerText regex to get around Playwright's whitespace normalization was a good call, and the markup now lives in InstrumentInstructions. CI is green. Two small things are left, both from the second commit:

1. Update packages/react-core/AGENTS.md. Lines 20-22 say exactly four components are deliberately left out of src/index.ts: FormContent, InteractiveContent, FileInstrumentContent and InstrumentOverview. InstrumentInstructions is now a fifth, so that statement is no longer true. The repo's rules say a change and its package's AGENTS.md must never disagree. Please add InstrumentInstructions to that list, with a few words saying it is an internal part of FormContent and InstrumentOverview.

2. Share the "Instructions" label in FormContent.tsx. The new aria-label (lines 58-62) and the dialog title (lines 73-77) both write { en: 'Instructions', es: 'Instrucciones', fr: 'Instructions' } inline. The repo prefers an inline t() only for a string that is used once, and the triage on #1812 asked for one constant here. My earlier note pointed you at the dialog title's t() call; I should have said to share it. Please move the object into one module-level constant typed LocalizedText (the type is already imported in that file) and pass that constant to t() in both places.

Reviewed at commit 6689ddb.