Simple Format import: words in the speaker name override the role in parentheses (#90)

.factory/oracles/90.json

@@ -0,0 +1 @@
+{ "issue": 90, "files": ["packages/shared/__tests__/simpleFormatParser.test.js", "e2e/zoom.timer-flow.spec.js"] }

.factory/plans/90-bracket-role-wins.md

@@ -0,0 +1,88 @@
+# Simple Format import: words in the speaker name override the role in parentheses (#90)
+
+## Problem
+In Agenda → Import Text → Simple Format, a role keyword in the speaker's name beats the
+built-in role written in the brackets. `Tim Timer (Standard Speech)` imports as Short Roles,
+so the timer shows 1/1:30/2 min cards instead of 5/6/7 for that speaker.
+
+## Findings
+- The custom-role half of the issue is already fixed: since #77 (724944f, f34bdff),
+  `parseSimpleFormatText` matches the last bracket against custom roles first
+  ([simpleFormatParser.js:92](../../packages/shared/simpleFormatParser.js)). The issue's
+  `(Closing)` and `(Reflection)` lines import correctly when those custom roles exist.
+- What is still broken: when the bracket names no custom role, the parser falls back to
+  `detectRoleFromText(line, names)` on the **whole line**
+  ([simpleFormatParser.js:93](../../packages/shared/simpleFormatParser.js)). The keyword
+  ladder in `detectRoleFromText`
+  ([timingRules.js:67-174](../../packages/shared/timingRules.js)) checks `includes()` in a
+  fixed order, so a name keyword earlier in the ladder wins over the bracket's role:
+  - `Tim Timer (Standard Speech)` → Short Roles (`timer`; exact `standard speech` only
+    matches when it is the whole text).
+  - `Dominique - General Evaluation (Table Topics)` → General Evaluation (GE is checked
+    before Table Topics).
+- Both apps call the same shared parser (`apps/zoom-app/src/context/TimerContext.jsx:476`,
+  `apps/web/src/context/TimerContext.jsx:244`), so one fix covers both.
+- Coverage: unit `packages/shared/__tests__/simpleFormatParser.test.js`; Playwright
+  `e2e/zoom.timer-flow.spec.js` "Simple Format import assigns custom roles" and
+  `e2e/web.agenda-flow.spec.js`; Zoom-client spec `agenda-import` (covers
+  `simpleFormatParser.js`; its "Not covered" lists #90 as a known open bug).
+
+## Approach
+Detect the role from the bracket before the whole line:
+
+```
+role = matchCustomRole(bracket)                       // unchanged (#77)
+    ?? detectRoleFromText(bracket, names, null)       // new: bracket keywords, no default
+    ?? detectRoleFromText(line, names)                // unchanged fallback
+```
+
+`detectRoleFromText` gets an optional third parameter `fallback` (default
+`'Standard Speech'`, today's behaviour for every existing caller). Passing `null` returns
+`null` when no exact name or keyword matched, so a bracket with no role words (`(A)`,
+`(Pathways L2)`) still falls back to today's whole-line detection.
+
+Rejected:
+- Detecting from the bracket only, never the line: changes lines whose bracket holds a
+  duration or project and whose role is in the name. Wider blast radius than the issue.
+- Stripping the name before detection: the name isn't reliably separable ("Tim Timer",
+  "Dominique - General Evaluation").
+
+## Acceptance criteria
+- AC-1: Given no custom roles, when `Tim Timer (Standard Speech)` is imported, then the
+  item is `{ name: 'Tim Timer', role: 'Standard Speech' }`.
+- AC-2: Given no custom roles, when `Dominique - General Evaluation (Table Topics)` is
+  imported, then its role is `Table Topics`: the bracket's role beats an earlier keyword in
+  the name.
+- AC-3: Given a bracket with no role words, when `Person (A)` (no custom roles) and
+  `Tim Timer (Pathways L2)` are imported, then today's whole-line detection still applies:
+  `Standard Speech` and `Short Roles`.
+- AC-4: Given a custom role `Closing`, when `Tim Timer (Closing)` is imported, then its role
+  is `Closing` (#77's custom-first order is unchanged).
+- AC-5: In the real Zoom client, pasting `Tim Timer (Standard Speech)` in Simple Format shows
+  that row with Standard Speech and its 5/6/7 min thresholds.
+
+## Test list
+| AC | Unit | Playwright | Zoom-client spec |
+|---|---|---|---|
+| AC-1 | `simpleFormatParser.test.js`, new describe "built-in role in brackets (#90)" | `e2e/zoom.timer-flow.spec.js`, new test "Simple Format import: the bracket's built-in role beats the name (#90)" | `agenda-import` IM-05 (NEW) |
+| AC-2 | same describe | same test | — |
+| AC-3 | same describe | — | — |
+| AC-4 | same describe | — | — |
+| AC-5 | — | — | `agenda-import` IM-05 (NEW) |
+
+`timingRules.test.js` gets a unit test for the new `fallback` parameter during implement
+(it is an implementation detail, so it is not an oracle test).
+
+## Risk
+Low. Shared import parsing only; no `riskHigh` path. EasySpeak import and every other
+`detectRoleFromText` caller keep the default fallback, so their behaviour is unchanged.
+
+## Open questions
+- **Q1.** A bracket with no role words but a role keyword in the name
+  (`Tim Timer (Pathways L2)`): keep today's whole-line detection (Short Roles)?
+  **Default: yes**, keep it. The bracket says nothing about the role, and some agendas put
+  the role in the name part.
+- **Q2.** A bracket keyword that is less specific than the name
+  (`Ice Breaker - Ana (Speaker 1)`): the bracket wins (`Speaker` → Standard Speech), as the
+  issue's "each item gets the role in its parentheses" asks? **Default: yes**, the bracket
+  wins.

e2e/zoom-client/INDEX.md

@@ -5,7 +5,7 @@ Read this first. Update it in the same PR as any spec change.
 | Spec | Status | Verified | Covers |
 |---|---|---|---|
 | [timer-core](timer-core.md) | current | 2e965b6, 2026-11-11 | start/colours/finish, resume after close; `audience_size` on `timer_started` for hosts, omitted for non-hosts (#107) |
-| [agenda-import](agenda-import.md) | draft | never | Simple Format import assigns custom roles from the bracket text; imported role's thresholds on real video (#77) |
+| [agenda-import](agenda-import.md) | current | 0ba1f1c, 2026-11-11 | Simple Format import assigns custom roles from the bracket text; imported role's thresholds on real video (#77) |
 | [zoom-deeplink](zoom-deeplink.md) | draft | never | "Open in Zoom" deeplink resolves and launches this build's app (dev vs production client id) |
 | auth-identity | **not migrated** | — | source: `docs/ZOOM_TEST_PLAN.md` Steps 1, 1b |
 | [connection-notice](connection-notice.md) | draft | never | guest-mode banner and "Approve Toastmusters Timer in Zoom" modal at open; raised mid-session when Zoom drops the grant, modal held until the timer stops (#85) |

e2e/zoom-client/agenda-import.md

@@ -1,7 +1,7 @@
 ---
 id: agenda-import
 title: Simple Format import assigns custom roles in the Zoom client
-status: draft
+status: current
 covers:
   - packages/shared/simpleFormatParser.js
   - apps/zoom-app/src/context/TimerContext.jsx
@@ -15,8 +15,9 @@ needs:
   - camera on, in a meeting
   - club presets not live on this device (not in a club, or "Use mine" in Edit Rules)
   - no custom roles on the device before IM-01 (note and remove any in the run log)
-verified_at: never
-verified_on: never
+  - built-in roles not hidden on this device (Standard Speech and Table Topics in the role dropdown), or IM-05's Live-tab check falls back to the first listed role
+verified_at: 0ba1f1c
+verified_on: 2026-11-11 (Zoom 7.2.5, macOS 26.7.2, dev c80dca2b)
 ---
 
 # Simple Format import assigns custom roles in the Zoom client
@@ -92,6 +93,25 @@ device.
 Evidence: page script: `toastmaster_agenda` is empty or absent, and
 `toastmaster_role_order` is `[]`.
 
+### IM-05 · auto · A built-in role in the brackets beats a keyword in the name
+Do: first run IM-04 (clean device). Agenda tab → Import Text → "Simple Format". Put these two
+lines on the clipboard with `pbcopy`, click the textarea, paste with ⌘V, press Import.
+```
+Tim Timer (Standard Speech)
+Dominique - General Evaluation (Table Topics)
+```
+Expect: toast "Imported 2 speaker(s)". Row "Tim Timer" shows Standard Speech and row
+"Dominique - General Evaluation" shows Table Topics. Click "Tim Timer": the Live tab role
+dropdown shows Standard Speech and the timing rules read Green: 5 minutes, Yellow: 6 minutes,
+Red: 7 minutes (not Short Roles' 1 / 1:30 / 2).
+Evidence: page script: `toastmaster_agenda` items are `Tim Timer` / `Standard Speech` and
+`Dominique - General Evaluation` / `Table Topics`. Screenshot of the Live tab rules line.
+Then Clear All in the Agenda tab.
+Run 2026-11-11 (0ba1f1c): the import half passed on the real client. The Live-tab half could
+not be checked: that device hid the built-in roles, so the dropdown had no Standard Speech and
+`LiveTab.jsx` moved the row to the first listed role (ChairToast1, 0:30/0:45/1:00). See the
+new `needs` line.
+
 ## Not covered
 - Custom roles that come from Club Pro presets, or from older saved rules with no role
   order. The PR now covers these too (`customRoleNames` comes from the dropdown, not from
@@ -101,10 +121,12 @@ Evidence: page script: `toastmaster_agenda` is empty or absent, and
 - Tie-breaks between custom roles, whole-word-only matching, and the built-in guard (a
   bracket like `(Ice Breaker)` is never taken by a partial custom match). Unit tests in
   `simpleFormatParser.test.js` cover these, and the client adds nothing to them.
+- An agenda row whose role is a hidden built-in: selecting it times the speaker with the
+  first listed role, with no notice, while the row keeps the original role (seen 2026-11-11;
+  behaviour from e876ca8, not this spec's change). No step locks it in.
 - EasySpeak import. This change did not touch it, and it has no spec yet.
-- `(Table Topics Speech)` imports as a `Table Topics` role with no timing rules (#91). A
-  built-in keyword in the name overrides a built-in role in the bracket (#90). Both are
-  known open bugs. No step locks in either result.
+- `(Table Topics Speech)` imports as a `Table Topics` role with no timing rules (#91), a
+  known open bug. No step locks in that result. IM-05 covers #90 (fixed: the bracket's built-in role is tried before the whole line).
 - The web app (`apps/web`) runs the same shared parser. Unit tests cover it in
   `apps/web/src/context/TimerContext.test.jsx`, and `e2e/web.agenda-flow.spec.js` runs this
   import (IM-01, IM-02) in a plain browser.

e2e/zoom.timer-flow.spec.js

@@ -182,6 +182,25 @@ test.describe('Zoom App — Functional Checks', () => {
     await expect(page.getByText('Timing rules: Green: 15 seconds, Yellow: 30 seconds, Red: 45 seconds')).toBeVisible();
   });
 
+  test("Simple Format import: the bracket's built-in role beats the name (#90)", async ({ page }) => {
+    await page.goto('/zoom/');
+
+    await page.getByRole('button', { name: /AGENDA/i }).click();
+    await page.getByRole('button', { name: /Import Text/i }).click();
+    await page.getByRole('button', { name: 'Simple Format' }).click();
+    await page
+      .getByPlaceholder('John Doe (Ice Breaker)')
+      .fill('Tim Timer (Standard Speech)\nDominique - General Evaluation (Table Topics)');
+    await page.getByRole('button', { name: 'Import', exact: true }).click();
+
+    await expect(page.getByText('Imported 2 speaker(s)')).toBeVisible();
+
+    const rows = page.locator('.cursor-pointer').filter({ has: page.getByRole('button', { name: /^Edit / }) });
+    await expect(rows).toHaveCount(2);
+    await expect(rows.locator('.font-medium')).toHaveText(['Tim Timer', 'Dominique - General Evaluation']);
+    await expect(rows.locator('.text-sm')).toHaveText(['(Standard Speech)', '(Table Topics)']);
+  });
+
   test('no console errors about missing context providers', async ({ page }) => {
     await page.goto('/zoom/');
     await page.waitForTimeout(1000);

packages/shared/__tests__/simpleFormatParser.test.js

@@ -117,6 +117,32 @@ describe('parseSimpleFormatText: PRD worked examples', () => {
   });
 });
 
+describe('parseSimpleFormatText: built-in role in brackets (#90)', () => {
+  it('AC-1: the bracket role beats a keyword in the name ("Tim Timer (Standard Speech)")', () => {
+    expect(parseSimpleFormatText('Tim Timer (Standard Speech)', [])).toEqual([
+      { name: 'Tim Timer', role: 'Standard Speech' },
+    ]);
+  });
+
+  it('AC-2: the bracket role beats an earlier keyword in the name ("General Evaluation (Table Topics)")', () => {
+    const [item] = parseSimpleFormatText('Dominique - General Evaluation (Table Topics)', []);
+    expect(item.role).toBe('Table Topics');
+  });
+
+  it('AC-3: a bracket with no role words keeps whole-line detection', () => {
+    expect(parseSimpleFormatText('Person (A)', [])).toEqual([{ name: 'Person', role: 'Standard Speech' }]);
+    expect(parseSimpleFormatText('Tim Timer (Pathways L2)', [])).toEqual([
+      { name: 'Tim Timer', role: 'Short Roles' },
+    ]);
+  });
+
+  it('AC-4: a custom role in the bracket still wins ("Tim Timer (Closing)")', () => {
+    expect(parseSimpleFormatText('Tim Timer (Closing)', ['Closing'])).toEqual([
+      { name: 'Tim Timer', role: 'Closing' },
+    ]);
+  });
+});
+
 describe('matchCustomRole', () => {
   it('matches ignoring case, spacing and punctuation', () => {
     expect(matchCustomRole(' opening   REMARKS ', ['Opening Remarks'])).toBe('Opening Remarks');

packages/shared/__tests__/timingRules.test.js

@@ -121,6 +121,16 @@ describe('getDefaultGraceAfterRed', () => {
 });
 
 describe('detectRoleFromText', () => {
+  describe('fallback parameter', () => {
+    it('returns null instead of Standard Speech when fallback is null and nothing matches', () => {
+      expect(detectRoleFromText('Pathways L2', null, null)).toBeNull();
+    });
+
+    it('still returns a matched role when fallback is null', () => {
+      expect(detectRoleFromText('Table Topics', null, null)).toBe('Table Topics');
+    });
+  });
+
   describe('exact case-insensitive matches', () => {
     it('maps "standard speech" to Standard Speech', () => {
       expect(detectRoleFromText('standard speech')).toBe('Standard Speech');

packages/shared/simpleFormatParser.js

@@ -89,7 +89,11 @@ export function parseSimpleFormatText(text, customRoleNames = []) {
     const groups = [...line.matchAll(BRACKET_GROUP)];
     const bracket = groups.length > 0 ? groups[groups.length - 1][1] : '';
     const customRole = bracket ? matchCustomRole(bracket, names) : null;
-    const role = customRole ?? detectRoleFromText(line, names);
+    // Bracket first (custom, then built-in keywords) so a keyword in the name can't beat it (#90)
+    const role =
+      customRole ??
+      (bracket ? detectRoleFromText(bracket, names, null) : null) ??
+      detectRoleFromText(line, names);
     return { name, role };
   });
 }

packages/shared/timingRules.js

@@ -66,7 +66,8 @@ export function getDefaultGraceAfterRed(role) {
 
 // Helper to detect role from text
 // customRoleNames: optional array of user-added role names; exact match (case-insensitive) is tried first
-export function detectRoleFromText(text, customRoleNames = null) {
+// fallback: role returned when nothing matched; pass null to get null instead (caller has its own next step)
+export function detectRoleFromText(text, customRoleNames = null, fallback = 'Standard Speech') {
   const normalized = text.toLowerCase().trim();
 
   if (customRoleNames && customRoleNames.length > 0) {
@@ -169,6 +170,5 @@ export function detectRoleFromText(text, customRoleNames = null) {
     return 'Speech Evaluation';
   }
 
-  // Default fallback
-  return 'Standard Speech';
+  return fallback;
 }