PT-4550: Short-name-first project names, through one shared helper

[jolierabideau]

Code Review Summary

Branch: pt-4550-shortname-first

Base: origin/pt-4549-titlebar-project-selector

Date: 2026-09-17

Review model: Claude Opus 5

Files changed: 50 (33 source, 17 committed lib/*/dist build artifacts)

Jira: PT-4550 — "Make shortname-first consistent across tab titles, dialogs and components"

Note for the reviewer: this PR targets pt-4549-titlebar-project-selector, not main. All counts and findings below are against that base. Diffing against main adds the parent branch's titlebar work (53 files / 43 commits) and is not what this PR proposes.

Overview

Project names previously rendered in two different orders — the Simple-mode toolbar showed True Meaning Arabic (arb) while ProjectSelector showed arb - True Meaning Arabic — and each surface carried its own copy of the fullName && fullName !== shortName de-dup rule. Five sites sorted project lists by three different keys. This branch introduces one shared helper set in platform-bible-utils (formatProjectName, hasDistinctFullName, compareProjectsByName, normalizeFullName, PROJECT_NAME_SEPARATOR), routes every formatting and sorting site through it, and backs the invariant with a repo-wide sweep test that fails the build when either rule is re-inlined.

The review made substantial in-flight changes, concentrated in three areas: the sweep test was rewritten because its original patterns could not catch the re-inlinings it existed to prevent; the read side of the platform.fullName setting was consolidated into one normalizer; and the project-selector trigger's accessible name was fixed so screen-reader users are told which project is selected. Full detail below.

Open Findings

Everything below that is checked ([x]) was fixed during the review and is in this branch. These are the items left open, in the order I would address them:

#	Finding	Where	Why it was left
1	One rejecting PDP blanks the entire settings-sidebar project list	settings-tab.component.tsx:47	Arguably a real bug, not a cleanup — needs an owner's call on Promise.allSettled vs a per-setting catch
2	ProjectSelectorProject.fullName required → optional is breaking for out-of-repo extensions	lib/platform-bible-react/experimental	Needs a release-note / communication decision, not a code change
3	Sibling types still require fullName: string	ProjectItem, FindProject, ProjectOption	Root cause of the remaining adapter coercions; migrating three types and their consumers is too wide for a review pass
4	Duplicate per-project fetch of names the metadata service already returns	settings-tab.component.tsx:47	Doubles per-project IPC; third near-identical getProjectNames helper
5	Consumer-level sort-order changes untested	find.component.tsx, checks-side-panel.component.tsx	User-visible reordering with no test pinning the resulting order
6	Settings search-on-full-name asserted at props level only	settings-sidebar.component.test.tsx	Closes DoD 3.3; needs one companion test with the real selector
7	Resource labels: adopt short-name-first, or not?	4 exempted DBL sites	Open question with no ticket yet — the four EXEMPT entries are the complete site list
8	manage-books.web-view.tsx:795,854 still mirror the short name	not in this PR's diff	Behavior already correct via de-dup; the type documents the fallback as intentional

Plus the open Minor items listed further down. None of these block merge on their own; items 1 and 3 are the two I would not let drift.

API Changes

• lib/platform-bible-utils: added hasDistinctFullName(names: ProjectNames): boolean
• lib/platform-bible-utils: added formatProjectName(names: ProjectNames): string
• lib/platform-bible-utils: added compareProjectsByName(a: ProjectNames, b: ProjectNames): number
• lib/platform-bible-utils: added normalizeFullName(fullName: unknown): string | undefined (added during review)
• lib/platform-bible-utils: added PROJECT_NAME_SEPARATOR: ' - ' (added during review)
• lib/platform-bible-utils: added type ProjectNames = { shortName: string; fullName?: string }
• lib/platform-bible-react/experimental: BREAKING — ProjectSelectorProject.fullName changed from string (required) to string | undefined (optional)
• lib/platform-bible-react: ProjectInfo gained optional projectFullName?: string; projectName unchanged
• lib/platform-bible-react: SettingsSidebar now passes triggerLabelFormat="shortNameAndFullName" to its internal <ProjectSelector>, so the trigger renders "{short} - {full}" instead of the short name alone
• lib/platform-bible-react: ProjectSelector's trigger aria-label now carries the current selection (changed during review — see Critical/Important #6)
• extensions/platform-scripture-editor: added getTabTitleProjectName(papi, projectId) (added during review)
• lib/papi-dts/papi.d.ts: no changes

Findings

Critical — Must address before merge

None.

Important — Should address before merge

• The adoption sweep could not catch the re-inlinings it existed to prevent. Its patterns required the literal identifiers shortName/fullName plus either a literal !==/=== or two ${} interpolations on one line. Measured against nine realistic re-inlinings, it caught 1 of 9. Three live sites were confirmed invisible to it: checklist.component.tsx:238 (fullName ?? shortName, no comparison operator) and scripture-text-grid-contents.utils.ts:232 / view-options-long-name.utils.ts:21 (which spell the short name name). (fixed during review: rewrote the sweep to anchor on the structural shape — any non-comment line naming a *[Ff]ullName field alongside a composing operator — now catching 8 of 9; the ninth, destructuring to renamed locals, is named explicitly in the docstring. Adoption sites are skipped structurally via a helper-name list, tests are scoped out, stories are kept in scope.)
• A live consumer hand-rolled the de-dup rule and behaved differently from the helper. checklist.component.tsx:238 produced an empty tooltip and a blank aria-label for fullName: '', where hasDistinctFullName treats '' as absent. (fixed during review: routed through normalizeFullName.)
• EXEMPT entries were keyed on substrings that could match more than one line. contains: 'row.rowKey' matched two lines in project-selector.component.tsx, only one of which was the intended cmdk haystack — so a future violation landing on the other would have been silently swallowed. (fixed during review: a second test asserts every exemption matches exactly one line, so both stale and over-broad keys now fail loudly. It has already caught two stale entries and five missing helper-skip entries during this review.)
• The separator was duplicated outside the helper. platform-bible-toolbar.tsx:184 passed a literal separator=" - " while formatProjectName hardcoded its own " - ", so changing one would desynchronise the visible label from its own tooltip. (fixed during review: exported PROJECT_NAME_SEPARATOR and used it at both sites, following the MARKER_STYLE_SEPARATOR precedent at paragraph-style-label.component.tsx:7; a test pins that the constant is what formatProjectName joins with.)
• Four different normalizations of the raw platform.fullName setting. The setting is typed string but a project data provider returns null for one never written, and legacy projects carry '' — so settings-tab, checklist.web-view, find.web-view and checks-side-panel.web-view had each decided separately what counts as absent. (fixed during review: added normalizeFullName(unknown) and adopted it at five read sites.)
• aria-label replaced the trigger's text, so screen-reader users were never told which project was selected. Both re-labelled surfaces passed a static group label, at every shrink step — and at the narrowest step the full name is dropped from the visible label with no title, sr-only or aria-describedby fallback. (fixed during review: the trigger composes "{group label}: {selection}", gated on a new hasSelection flag; the toolbar composes its own ariaLabel because it supplies renderTriggerLabel. Seven new tests. 41 existing assertions across 4 files were querying the old exact name and were updated to anchor on the label prefix — see Suggested Review Focus.)
• The popover row tooltip showed the long name alone. It doubles as the row's truncation disclosure (useTruncationTooltip opens it when either line is clipped), so a row whose short-name line was clipped offered a tooltip that did not contain the clipped text. (fixed during review: uses formatProjectName(row).)
• The ADR recorded tab titles as unfinished work, contradicting the issue title. PT-4550 is titled "…across tab titles, dialogs and components", and the ADR said "a follow-up is needed to route them through the helper" — reading as though a third of the ticket was missing. Investigation showed tab titles already read platform.name (the short name) and never show the full name, so the DoD item is satisfied more strongly than the helper would satisfy it. (fixed during review: rewrote the ADR consequence as a deliberate decision with its space-constraint rationale — routing tab titles through formatProjectName would append the full name into the most space-constrained surface in the app and regress them.)
• Nothing pinned which setting feeds the tab title. formatEditorTitle's tests inject the project name, so a swap of main.ts's inline callback to platform.fullName would change every tab title with no test failing. (fixed during review: extracted getTabTitleProjectName(papi, projectId) and added three tests; mutating it to platform.fullName turns 2 of 3 red.)
• The manage-books header subtitle showed the long name alone. project.fullName ?? project.shortName — a dialog project label that neither led with the short name nor routed through the helper. (fixed during review: uses formatProjectName(project), with three tests covering distinct / absent / equal full names. This is a user-visible change: the subtitle now reads 1 books in WEB - World English Bible.)
• ProjectSelectorProject.fullName required → optional is breaking for readers of the experimental surface. Every in-repo reader was updated, but an out-of-repo extension doing selected.fullName.toLowerCase() will no longer compile. The surface carries no stability guarantee (lib/platform-bible-react/src/experimental.ts header), and there is no non-breaking alternative that also fixes the mirroring problem — the change is right, it just needs announcing in the PR description / release notes rather than landing silently.
• Sibling project types still require fullName: string. The PR made ProjectSelectorProject.fullName optional but left ProjectItem (project-picker.component.tsx:20), FindProject (find.component.tsx:166) and ProjectOption (checks-side-panel.utils.ts:70) requiring it — which is why those adapters were mirroring or passing raw possibly-null values in the first place. Surfaced by four typecheck errors during review. Deliberately not resolved: migrating three types and their consumers is too wide for a review fix. The adapters now coalesce to '', which the de-dup already reads as absent and which keeps a null out of a slot the type promises is a string.
• settings-tab.component.tsx:47 fetches names it already has. getProjectNames opens a project data provider per project and makes two getSetting round trips, when projectLookupService.getMetadataForAllProjects() two functions above already returns both name and fullName (project-metadata.model.ts:25,32) — the cheap path is already used for exactly this purpose in use-project-picker-data.hook.ts:87. This doubles per-project IPC on the settings tab, and is a third near-identical getProjectNames PDP-fetch helper alongside find.web-view.tsx:124 and checklist.web-view.tsx:586.
• settings-tab.component.tsx:47 takes the whole sidebar down on one rejecting PDP. The two getSetting calls are in a Promise.all nested inside an outer Promise.all across all projects, so a single project whose PDP rejects platform.fullName blanks the entire settings-sidebar project list — where previously only platform.name had to succeed. Promise.allSettled or a per-setting catch would degrade it to one missing name.
• Consumer-level sort-order changes are untested. find.component.tsx and checks-side-panel.component.tsx both switched their sort key from fullName to compareProjectsByName — a user-visible reordering — and neither has a test asserting the resulting order. compareProjectsByName is pinned only in the utils unit test; nothing pins that a consumer's pre-sorted list agrees with ProjectSelector's own compareRows.
• The Settings adapter's search-on-full-name is asserted at the props level only. settings-sidebar.component.test.tsx mocks <ProjectSelector> and checks the projects array — justified in the file's own header comment, and the right call for distinguishing absent from mirrored. But no test shows that a distinct full name sourced from platform.fullName is actually searchable in that surface. A small companion test with the real selector would close DoD item 3.3.

[Author response: The author asked for a recommendation on each finding rather than defending or explaining the existing implementation, and approved the recommended approach in each case. The three fixes at the end of the list were deliberately left open as out of scope for a review pass.]

Minor — Consider

• The toolbar mirrored the project id into both name fields. beginOpenProject(item ?? { id: projectId, shortName: projectId, fullName: projectId }) — benign, since the de-dup collapses it, but it is the practice the branch removes everywhere else, in the branch's own headline file. (fixed during review: fullName: ''.)
• ProjectInfo uses projectName/projectFullName where the rest of the codebase uses shortName/fullName. (fixed during review, by documentation rather than rename: the type is exported from the stable barrel so renaming is breaking, and the fork turns out to be contained to exactly one adapter — the projectSelectorProjects memo. Added TSDoc naming the correspondence to ProjectNames. The related complaint that this file was invisible to the sweep has lapsed: the widened pattern matches projectFullName.)
• A stale comment described the manage-books subtitle as {full project name}. (fixed during review.)
• Whitespace-only full names are treated as distinct, so formatProjectName({ shortName: 'WEB', fullName: ' ' }) yields "WEB - ". (Not observed in practice — platform.fullName is either written or absent, and the empty-string case is handled and tested. A .trim() inside hasDistinctFullName would close it if a real project ever surfaces one.)
• use-sync-status.hook.ts:711 builds a throwaway object to compare two bare strings, and changes behavior while doing so: the old a.name.localeCompare(b.name) was case/accent-sensitive, the helper uses sensitivity: 'base', so names differing only by case now tie and fall through to the projectId tie-break. Almost certainly desirable — it matches the selector — but it is a behavior change on an existing path with no test here.
• Storybook fixtures still demonstrate the rejected mirroring pattern. project-selector.stories.tsx builds fixtures with fullName: p.shortName and the prose calls that "legacy-project fixtures", when this branch made fullName optional precisely so consumers stop mirroring. No story exercises the newly-supported absent-fullName shape. The documentation surface developers copy from now shows the shape the codebase no longer produces.
• secondaryFirst on ToolbarCompoundLabel has no production consumer now that the toolbar leads with the short name — only a unit test and a story whose 12 pt example is hypothetical rather than shipped UI. Retained deliberately (stable barrel; removing a documented prop is breaking), but it is now an untested-in-production API path documented by an invented example. Worth a deliberate keep-or-remove decision.
• use-project-picker-data.hook.ts:91 falls back to the project id for the short-name slot (m.name ?? m.id). Now that the short name leads and is the field that survives the narrowest toolbar step, a project with no platform.name but a real fullName shows an opaque id as its only visible label where the previous long-name-first label showed something readable. Consider falling back to m.fullName before m.id.
• manage-books.web-view.tsx:795,854 still mirror (p.fullName.length > 0 ? p.fullName : p.name). Deliberately not changed: the file is not in this PR's diff, behavior is already correct via the de-dup, and manage-books-dialog.types.ts:76 documents the short-name fallback as the intended contract of ManageBooksDialogProject. Flagged so the contract is a decision rather than an accident.
• home.component.tsx:301 (platform-get-resources) sorts with raw </>, case- and accent-sensitive, so the Resources table orders differently from every other project list. Pre-existing, but it makes the ADR's "five sort sites now share one comparator" incomplete — either migrate it or note in the ADR why a user-driven column sort is out of scope.
• The separator is a hyphen-minus (" - "), not an en dash. Nothing in the guidelines specifies a dash style; noted only because PROJECT_NAME_SEPARATOR is now the single place that decides it. A bidi-neutral character between two strongly-directional runs can also reorder under RTL — an en dash plus directional isolates would be the fix if an RTL locale ever shows this wrong.
• "behaviour" in settings-tab.component.tsx where the repo otherwise uses US spelling. (fixed incidentally when that comment was rewritten for the normalizeFullName adoption.)

[Author response: Not individually reviewed — the author directed the review toward the Important cluster. The remaining open minors are carried here for the review meeting.]

Template Propagation

Shared Regions Modified

None. No changed file contains a #region shared with marker.

Extension Config Changes

• [n/a] All extensions/ changes are component and web-view source. No webpack.config.ts, tsconfig.json, package.json or .eslintrc* changed, so there is nothing to propagate.

Positive Observations

• Not a single backward-facing comment across 13 commits, three of which are review-fix commits — no ticket IDs for in-PR work, no review-finding IDs, no "previously"/"used to" narration. That pattern accumulates reliably in AI-authored diffs; its absence here is notable.
• The ADR entry is correctly placed in byte-order slug position and carries all six required fields. Its Alternatives section states three rejected options with reasons, where one is the norm.
• project-util.ts's TSDoc explains why for each decision — why the !== is case-sensitive, why the separator is not localized, why the short name leads — rather than restating the code.
• use-project-picker-data.hook.test.ts:458 deliberately constructs fullName order as the exact reverse of shortName order, so a comparator reading the wrong field fails rather than coincidentally passing. A falsifiable test, not a confirming one.
• settings-sidebar.component.test.tsx mocks <ProjectSelector> and asserts on the projects prop rather than the DOM, with an explicit comment explaining why a DOM assertion cannot distinguish absent from mirrored. The right reason to reach for a mock, correctly reasoned.
• Responsive behavior matches adr-toolbar-shrink-measurement exactly: ProjectSelectorLabel reads useShrinkStepValue() from ShrinkStepContext with no container queries anywhere in the diff, and the narrowest-step floor (tw:min-w-24, sized to ESVUS16) stops shrinking rather than cutting into the short name.
• RTL holds up: the separator is its own flex item rather than a bidi-neutral character inside one text node, so the fields reorder with direction; the row and trigger use logical utilities throughout (tw:ms-auto, tw:text-start, tw:pe-4).
• ToolbarCompoundLabel already suppressed the separator when either field is missing, so the new no-full-name case cannot leave a dangling " - " at the narrowest shrink step — the edge case held without change.
• Both dist/ trees were regenerated alongside src rather than left stale, and papi.d.ts was not hand-edited.

Interview Notes

Stated purpose: review PR #2822 for adherence to code patterns and for whether it solves PT-4550.

Base branch: the author confirmed the review should run against pt-4549-titlebar-project-selector (the PR's actual base), not main.

Tab titles: asked whether the ADR's tab-title deferral was a deliberate decision driven by the PT-4216 Share Layout coupling or simply fell out of scope, the author answered: "I think it just fell out of scope." Investigation during the review then established that tab titles already satisfy the DoD item — they read platform.name and never show the full name — so the deferral was recorded as a decision rather than completed as work. The reviewer should confirm that reading, since it converts a stated gap into a stated non-gap.

Fabricated ticket reference: the review initially wrote TODO(PT-4553) into the sweep's DBL-resource exemptions as a placeholder follow-up ticket. The author caught it — "PT-4553 already exists for something else in our Jira" — and the reference was removed in favor of a prose deferral that states plainly there is no ticket yet. There is still no ticket for the resource-label question; if resource labels should adopt a short-name-first format, the four exemptions in project-name-adoption.test.ts are the complete site list.

Design authority in this review: for each of the seven findings the author asked for a recommendation ("What do you recommend?", "what do you recommend?", "go with 1 and 2", "Work through the cluster", "Fix three mor[e]") and approved the recommended approach rather than proposing an alternative or explaining the original implementation's reasoning. The author did exercise judgment on scope — approving the cluster work, and catching the invented ticket ID. But the design rationale for the in-review changes originated in the review, not with the author. The reviewer should walk the author through the sweep rewrite, the hasSelection gating, and the normalizeFullName consolidation in the meeting to confirm the author can maintain them.

Not asked: the author was not asked to explain the original (pre-review) implementation in their own words, because the interview was consumed by the seven findings and the fixes they prompted. That explanation is still owed and is the best use of the review meeting.

In-Review Quality Check

Substantial changes were made during the review — 6 new/modified source files beyond the original diff, 17 new tests, and two library rebuilds.

• npm run typecheck — clean. Caught four real type errors mid-review, all tracing to one root cause (sibling project types still requiring fullName: string), now recorded as an open finding.
• npm run lint — 0 errors, 1 pre-existing warning. Two failures were fixed during the review: a no-null/no-null violation in a new test (suppressed with a justification, since null is the actual runtime value being guarded against) and five no-template-curly-in-string violations from EXEMPT keys quoting ${…} (resolved by choosing keys that avoid the construct).
• prettier --check across all changed source files — clean.
• npm test — 8,489 tests, 0 failures across all workspaces.
• npm run build:pbu and npm run build:pbr — both run, dist/ committed. Required because platform-bible-utils gained two exports and platform-bible-react embeds its dist.

Falsifiability was verified for every fix, not assumed:

Fix	Mutation applied	Result
Sweep rewrite	9 realistic re-inlinings injected	8 caught (was 1)
Tab-title setting	platform.name → platform.fullName	2 of 3 red
Manage-books subtitle	fullName ?? shortName	1 of 3 red
Manage-books subtitle	un-de-duped `${short} - ${full}`	2 of 3 red
aria-label + row tooltip	both reverted	3 red

Suggested Review Focus

• The accessible-name change has the widest blast radius in the diff. 41 assertions across 4 files were querying the trigger's old exact accessible name and were updated to anchor on the label prefix. That is the change most likely to surprise a reviewer skimming, and the update is mechanical enough to hide a real regression — worth a direct look at project-selector.component.test.tsx, find.component.test.tsx, platform-bible-toolbar.integration.test.tsx and project-selector.stories.tsx.
• Confirm the tab-title reading. The ADR now asserts tab titles deliberately do not use formatProjectName. If the reviewer disagrees — if tab titles should carry the full name — then a DoD item is open rather than closed, and getTabTitleProjectName is the single place to change.
• The breaking experimental API change needs a release-note decision. ProjectSelectorProject.fullName required → optional will break out-of-repo extensions that read it. Decide whether the experimental surface's no-guarantee header is sufficient or whether this warrants an explicit callout.
• Walk the author through the three largest in-review changes — the sweep rewrite (including what it deliberately cannot catch), the hasSelection gating on the trigger's accessible name, and the normalizeFullName consolidation. The design rationale for these originated in the review; confirm the author can maintain them.
• Decide on the sibling-type migration. ProjectItem, FindProject and ProjectOption still require fullName: string while ProjectSelectorProject does not. Left open deliberately; it is the root cause of the remaining adapter coercions and should be a follow-up ticket rather than drift.
• Decide whether resource labels adopt short-name-first. Four DBL-resource sites are exempted from the sweep with reasons but no ticket. If the answer is yes, those four exemptions are the complete site list.
• settings-tab.component.tsx's duplicate fetch and failure mode — it refetches names the metadata service already returns, and one rejecting PDP blanks the whole sidebar list. Both are open findings on a file this PR touched.
• The author has not yet explained the original implementation in their own words. The interview was consumed by findings and fixes; this is the best use of the meeting.

---

Summary

Project names were formatted and sorted inconsistently across the app: the Simple-mode toolbar rendered True Meaning Arabic (arb) while ProjectSelector rendered arb - True Meaning Arabic, and five sites sorted project lists by three different keys. This introduces one shared helper, makes every site use it, and adds a test that fails if either rule is re-inlined.

Serves PT-4550 (WI-24), part of the current epic PT-4530 — Sprint 90, "Simple is coherent for Saroj". IAN-NN-2.4 ≡ TODD-NTH-3.1: one piece of work at two priorities, built once.

Stacked on #2801 → #2790. Review after those; base is pt-4549-titlebar-project-selector.

Changes

New in lib/platform-bible-utils/src/project-util.ts:

type ProjectNames = { shortName: string; fullName?: string };
hasDistinctFullName(n)      // !!fullName && fullName !== shortName — the de-dup rule
formatProjectName(n)        // `${shortName} - ${fullName}`, or shortName alone
compareProjectsByName(a, b) // shortName, sensitivity 'base', no tie-break

• Toolbar label inverted to SHORT - Full Name, collapsing to bare SHORT at SHRINK_STEP.MINIMUM via the existing shrink ladder. No new responsive machinery — adr-toolbar-shrink-measurement's no-container-queries constraint is untouched.
• Five sort sites now share one comparator, each keeping its own tie-break (scrollGroupId, projectId).
• Settings full name plumbed from platform.fullName, so Settings search matches either name.
• ProjectSelectorProject.fullName is now optional, so a project with no full name stops mirroring its short name into that field.
• Adoption sweep test scans the tree and fails if the de-dup rule or a joined label is written by hand outside the helper.

⚠️ User-visible changes — each reads as a one-line diff

1. Toolbar project label inverts from Full Name (SHORT) to SHORT - Full Name. This supersedes a deliberate, documented decision; rationale and rejected alternative are in adr-project-name-short-name-first.
2. Four lists reorder — Find, checks side panel, the renderer picker, and the sync-status popover (which also gains case-insensitive ordering).
3. arb (arb) no longer renders when a project's two names match. Pre-existing bug; showSecondary never consulted a de-dup rule.
4. No redundant tooltip on the project label when there is nothing extra to reveal.

Deliberately not done

• Tab titles are a partial. They already lead with the short name (platform.name), but compose it inside a localized template that the sweep cannot see. Needs a follow-up to route them through the helper.
• TODD-NTH-3.2 is toolbar-only. The PRD's exemplar is Home, which is untouched (Home already implements the responsive behaviour).
• Find and the checks side panel reorder with no test pinning it. No existing assertion pinned order and neither has a comparator-level seam without new test infrastructure. Structurally the DoD holds — both call the one comparator — but a future edit could re-diverge and nothing would fail. Flagged for a reviewer's judgement.
• manage-books-dialog.component.tsx:1544 shows the full name only in a dialog subtitle. Contradicts "short name leads in dialogs" but is a visible product change outside this ticket's scope.
• No Power-mode assertion, despite PT-4530's gate. ToolbarCompoundLabel changed only in TSDoc and the toolbar label is Simple-only, so there is no Power surface to regress.
• Z_INDEX_OVERLAY in the settings sidebar violates z-index-tiers.md rule 2. Pre-existing on this branch — not introduced here — and wants its own ticket.
• Q5(a) is still open on PT-4530. Full scope is implemented; the DoD tags keep a later split mechanical.

Supersedes #2674

#2674 (@Sebastian-ubs) fixed the Settings adapter. Its work is absorbed here with a Co-authored-by trailer, and this branch closes the gap it left: #2674 kept fullName: info.projectFullName ?? info.projectName, which still mirrors when a project has no full name. That mirror is gone, and a test asserts it stays gone.

If #2674 lands first, the merge is one source file (settings-sidebar.component.tsx, take ours) plus regenerated dist. The other three of its files auto-merge cleanly.

API surface

• ProjectSelectorProject.fullName / ProjectRow.fullName → optional. Ships from experimental.ts (no stability guarantee); a widening for producers, and every in-repo read is guarded.
• ProjectInfo.projectFullName → optional add on a stable-barrel type; non-breaking.
• ToolbarCompoundLabel.secondaryFirst → retained despite losing its only in-repo consumer. Stable barrel, documented prop; removal would be breaking.

AI Involvement

AI-assisted throughout, under human direction. The design was brainstormed interactively, adversarially reviewed against the PRD and Definition of Done, then implemented task-by-task with a fresh review after each task and a whole-branch review at the end.

The whole-branch review caught a regression 8468 passing tests missed: the label passed secondary as always-defined, which made ToolbarCompoundLabel treat it as "partial" at every width and open a tooltip repeating the visible text. Four defects in the implementation plan were also caught during execution — a constant that did not survive a re-base, two non-falsifiable tests, and a verification command that scanned gitignored generated docs. Each is recorded in the commit history.

Testing

• npm run typecheck — exit 0
• npm run lint — exit 0
• npm test — 0 failures
• prettier --check (repo's --ignore-path .prettierignorerun) — clean
• Adoption sweep verified falsifiable: materializing the pre-adoption toolbar makes it fail on platform-bible-toolbar.tsx:184
• Tooltip and de-dup tests verified falsifiable against the pre-fix implementation
• Manual verification of the four reorderings in a running app

Risk Level

Medium — the logic is small and well-tested, but four user-visible surfaces change at once, and the branch sits third in a draft stack whose dist bundles conflict by construction (PT-4530 cluster B).

---

This change is 

[katherinejensen00]

Review — PR #2822 (PT-4550, shortname-first project names)

To: @jolierabideau
Base: pt-4549-titlebar-project-selector (merge-base dbc6ea38728) — not main
Head reviewed: 7198a2caf44
Line numbers are head-file lines and each is a line the diff adds or changes, so each comment attaches cleanly.

Overall: the shared-helper design is sound and the helper module itself is well-tested and well-documented. What I would fix before merge is not the helper but the edges around it: one read path surfaces a localized *Name Missing* placeholder as a real full name, two new tests cannot fail, one new sort is dead code, and the ADR and PR description claim guarantees the sweep test does not actually provide.

---

Important

1. platform.fullName returns a localized *Name Missing* placeholder when unset, so Settings will show "WEB - *Name Missing*"

File: src/renderer/components/settings-tabs/settings-tab.component.tsx:58 · Severity: Important

normalizeFullName only treats null, undefined and '' as absent, but a project data provider never returns those for this setting — it returns the contribution default.

When the Paratext FullName setting is missing, ParatextProjectDataProvider.cs:1783-1786 falls through to ProjectSettingsService.GetDefault, and core-project-settings-info.data.ts:18 declares that default as '%project_full_name_missing%' → "*Name Missing*" (assets/localization/en.json:361). normalizeFullName returns that string unchanged, hasDistinctFullName reports true, and the Settings sidebar — which this PR switched to triggerLabelFormat="shortNameAndFullName" — renders WEB - *Name Missing*, with the same text in the row's second line and in the trigger's accessible name. Before this PR that surface showed WEB.

Your own C# comments already name this exact hazard: ProjectSummary.cs:89-91 says its string.Empty fallback "does NOT reproduce platform.fullName's default - a localized %project_full_name_missing% placeholder", and ProjectMetadata.cs:63-66 says FullName is null when the setting is absent. So the metadata-sourced surfaces (toolbar, manage-books) will show WEB while the PDP-sourced surfaces show WEB - *Name Missing* — the cross-surface inconsistency this PR exists to remove. The same read is new in find.web-view.tsx:134, checks-side-panel.web-view.tsx:52 and checklist.web-view.tsx:375.

Fix: source the full name from getMetadataForAllProjects() (which this component already calls, and which is null-when-unset by design), or have normalizeFullName reject the placeholder. See also #9.

2. The new toolbar accessible name announces the error stand-in as if it were the selected project

File: src/renderer/components/platform-bible-toolbar.tsx:345 · Severity: Important

When the current-project lookup fails, sighted users see the error message but screen-reader users are told the project is called "??? - Unable to load current project details".

use-project-picker-data.hook.ts:375-379 returns a stand-in { shortName: '???', fullName: 'Unable to load current project details' } on error, and :362 sets currentSimpleProjectError. The visible label branch checks that error at :331-332 and renders errorMessage; the new selectorAriaLabel at :345-348 does not, so it runs the stand-in through formatProjectName. The announced text is also hardcoded English. Gate the name on currentProjectError the same way renderTriggerLabel does.

Minor, same line: displayedProject ?? pendingProject has a dead right-hand side — use-pending-project.hook.ts:112 already returns displayedProject = pendingProject ?? currentProject.

3. The new "opens no tooltip on hover" test passes whether or not the de-dup exists

File: src/renderer/components/platform-bible-toolbar.test.tsx:1354 · Severity: Important

This test renders at a shrink step where the tooltip it asserts against can never open in jsdom, so it cannot fail.

It calls renderAtStep(SHRINK_STEP.WIDE). At WIDE, showSecondary is true, so isShowingPartialLabel is false (toolbar-compound-label.component.tsx:133) and the partial-label path never fires. The only other opener, useTruncationTooltip, needs scrollWidth > clientWidth, which is 0 > 0 in jsdom, and the controlled Tooltip ignores Radix's own open requests (:222-224). Changing platform-bible-toolbar.tsx:184 to an un-de-duped secondary={fullName} leaves it green. The regression it names — a tooltip repeating a fully visible label — only occurs at SHRINK_STEP.MINIMUM; render it there.

4. The new "orders all projects by short name" test passes with the sort deleted

File: src/renderer/hooks/use-project-picker-data.hook.test.ts:458 · Severity: Important

The fixture is already in the expected order, so the test only catches a comparator reading the wrong field — not a missing sort.

The metadata is [{ name: 'AAA' }, { name: 'ZZZ' }] and the assertion is ['AAA','ZZZ'] (:463-472). Removing .sort(compareProjectsByName) from use-project-picker-data.hook.ts:433 leaves the input order, which equals the expectation. Reverse the fixture order (the sibling test at :458 does exactly this trick for fullName and is the model to copy).

5. The ADR and PR say the sweep fails the build if either rule is re-inlined; it cannot see a re-inlined sort at all

File: .context/standards/Architecture-Decisions.md:2091 (Decision bullet) · Severity: Important

The sort half of the invariant has no enforcement, so the ADR records a guarantee the code does not provide — and the ADR log is add-only.

COMPOSING_OPERATOR (project-name-adoption.test.ts:46) is /(===?|!==?|\?\?|\|\||\$\{|\.join\(| \+ )/, with no localeCompare and no </>; :188 additionally requires a *fullName token on the same line. So re-adding the exact line this PR removed — .sort((a, b) => a.fullName.localeCompare(b.fullName, undefined, { sensitivity: 'base' })) — is invisible. Either narrow the ADR wording to the formatting/de-dup rule, or extend the sweep. While editing: the Decision lists three helpers but the shipped set is five (normalizeFullName, PROJECT_NAME_SEPARATOR are missing), and "the short name leads everywhere" is not true of tab titles, the four exempted DBL sites, or home.component.tsx:299-330 — worth one clause each.

6. The new pre-sorts in Find and the checks side panel are dead code, and the "four lists reorder" claim is wrong

File: extensions/src/platform-scripture/src/find/find.component.tsx:776 (also checks-side-panel.component.tsx:166) · Severity: Important

ProjectSelector re-sorts every section itself, so these two lists did not change order — before or after this PR.

sortedProjects is passed only as the projects prop (find.component.tsx:856,865; checks-side-panel.component.tsx:225). Every grouping path re-sorts with compareRows: partitionAndSort covers both the flat and grouped paths (project-selector.rows.ts:483,485-486), the language/versification/type groupings sort at :535,551,583,594,628,639, custom grouping at :756-766,785, and the dispatch's default branch routes to partitionAndSort (project-selector.component.tsx:1068-1098). The only unsorted section is lastUsed's recent list (:662), which neither consumer can reach — the checks panel is pinned to availableGroupings={['openTabs']} (checks-side-panel.component.tsx:227). And compareRows already sorted by shortName at sensitivity: 'base' before this PR (rows.ts:451).

So of the four claimed reorderings, only the renderer "More projects" list actually reorders (use-project-picker-data.hook.ts:433, previously by fullName); sync-status changes only for names differing by case (use-sync-status.hook.ts:705 already sorted by name). Please drop the two pre-sorts and their imports, correct the ADR Consequences and the PR body's "Four lists reorder", and retire the open finding asking for tests of an order nothing can observe — a reviewer following the manual test plan will hunt for changes that cannot happen.

7. Wrapping a name in normalizeFullName is a free pass around the sweep

File: src/renderer/components/projects/project-name-adoption.test.ts:28 · Severity: Important

normalizeFullName is in HELPERS, but it normalizes rather than formats, so any line mentioning it is dropped before the operator check.

:247 does if (HELPERS.test(line)) return [];. That means normalizeFullName(p.fullName) ?? p.shortName — precisely the long-name-first fallback the sweep exists to catch — passes silently. The live example is checklist.component.tsx:238. Remove normalizeFullName from HELPERS, or better, strip the helper-call span from the line and test the remainder, then exempt checklist.component.tsx:238 explicitly.

---

Minor

8. The sweep cannot see a ternary fallback or a bare && render

File: src/renderer/components/projects/project-name-adoption.test.ts:46 · Severity: Minor

Two live mirroring sites are written in a shape the operator list does not contain.

manage-books.web-view.tsx:795,854 read fullName: p.fullName.length > 0 ? p.fullName : p.name — no listed operator, so no candidate. Likewise {project.fullName && <span>{project.fullName}</span>} (renders with no de-dup) and .concat(' - ', fullName). Adding \? to COMPOSING_OPERATOR covers the common case; if you would rather not widen it, add the ternary to the docstring's "does NOT cover" list, which currently claims a single-line composition naming a *fullName field fails the test.

9. Settings re-fetches per project what the metadata call above it already returned, and one rejection empties the list

File: src/renderer/components/settings-tabs/settings-tab.component.tsx:56 · Severity: Minor

getAllProjectIdsFromMetadata throws away name/fullName, then getProjectNames opens a PDP per project to fetch them back.

ProjectMetadata already carries both (project-metadata.model.ts:25,32), and use-project-picker-data.hook.ts builds the same pair without opening a data provider. The nested Promise.all (:198-203) also means one rejecting PDP leaves allProjectOptions = [] — the whole sidebar list blank. Both are on your open list, so this is confirmation rather than a new ask; I raise it because moving to the metadata path fixes #1 and the staleness below at the same time. Staleness: usePromise has [] deps, so names are fetched once; "Project Full Name" is editable in this very tab, and now that it is displayed, an edit leaves the sidebar stale until the tab reopens.

10. The accessible name is composed by concatenation in two places with a hardcoded ": "

File: lib/platform-bible-react/src/components/advanced/project-selector/project-selector.component.tsx:1321 (also platform-bible-toolbar.tsx:348) · Severity: Minor

Both pieces are localized but the join is not, so translators cannot reorder or re-punctuate it (CJK would want ：).

Code-Style-Guide.md:228 and Localization-Guide.md:457 both ask for a {placeholder} template via formatReplacementString; the checklist in this same diff already does ariaLabelTemplate.replace('{name}', …). Worth considering as an alternative that removes the concatenation, the localization issue and most of the 41-assertion test churn: leave aria-label alone and point aria-labelledby at the group label plus the trigger text, or move the group label to aria-describedby.

Related, same component: the ariaLabel prop's TSDoc (:312-316) still says it is "announced in place of its visible label", which is no longer what happens. And manage-books-dialog.component.tsx:2267 passes ariaLabel="Select project", which now announces "Select project: WEB" — a behavior change no consumer was told about.

11. Nothing pins the new Settings trigger format

File: lib/platform-bible-react/src/components/advanced/settings-components/settings-sidebar.component.tsx:200 · Severity: Minor

Deleting triggerLabelFormat="shortNameAndFullName" fails no test, although the API-changes list calls the resulting label a user-visible change.

settings-sidebar.component.test.tsx already captures the mocked selector's props and asserts on projects; one more line — expect(props.triggerLabelFormat).toBe('shortNameAndFullName') — closes it.

12. The two new stories teach the mirroring pattern this PR removes

File: lib/platform-bible-react/src/components/advanced/project-selector/project-selector.stories.tsx:113 · Severity: Minor

ShortNameTriggerLabel and ShortNameTriggerLabelNoScrollGroups are both new here and build fixtures with fullName: p.shortName, calling them "legacy-project fixtures".

This PR made fullName optional precisely so consumers stop mirroring, and the sweep deliberately keeps stories in scope because "they are the surface developers copy from" — but it cannot flag these (a plain assignment has no composing operator). The comment at :115 also quotes the inline rule fullName && fullName !== shortName, which this PR replaced with hasDistinctFullName. Build the fixtures by omitting fullName, and point the prose at the helper.

13. ProjectSelectorProject.fullName's TSDoc invites mirroring and describes the old tooltip

File: lib/platform-bible-react/src/components/advanced/project-selector/project-selector.rows.ts:26 · Severity: Minor

"Optional — omit it (or pass the short name)" recommends the thing the PR is removing, and "as the tooltip title" is stale now that the row tooltip renders formatProjectName(row) (project-selector.component.tsx:743).

Suggested: "Full name, shown as the row's muted second line. Omit it when the project has none — don't copy the short name in; the selector already renders one line when the names match."

14. Two EXEMPT reasons do not match what the exempted code does

File: src/renderer/components/projects/project-name-adoption.test.ts:105 · Severity: Minor

An exemption is only as good as its reason, and these two will mislead whoever revisits them.

• :105 — reason "the label itself is composed downstream" is half true: the toolbar's selector de-dups, but ProjectPicker renders column 2 as {p.fullName} unconditionally (project-picker.component.tsx:147), fed by the mirror at use-project-picker-data.hook.ts:90. In the "More projects" dialog a project with no metadata FullName shows WEB | WEB — the duplicate the PR body says "no longer renders". The rendering itself is pre-existing and out of scope; the reason isn't.
• :153 — reason "DBL resource long-name slot — resource metadata, not project names" doesn't hold: scripture-text-grid-contents.utils.ts:218-233 builds type: 'project' rows for locally installed read-only projects from platform.name/fullName metadata, and resource-collection-options.component.tsx:90 joins them with an em dash rather than PROJECT_NAME_SEPARATOR.

15. The PR description contradicts the code and the ADR in three places

File: PR description (no code line) · Severity: Minor

A reviewer reading the description is told a DoD item is open when the ADR says it is closed, and that a site is unchanged when it is changed.

• "Tab titles are a partial … Needs a follow-up to route them through the helper" vs the ADR and getTabTitleProjectName's TSDoc, which call this a deliberate decision.
• "manage-books-dialog.component.tsx:1544 shows the full name only … outside this ticket's scope" vs :1547, now formatProjectName(project) with three new tests.
• "User-visible changes" omits three: the manage-books subtitle, the Settings trigger now rendering {short} - {full}, and the accessible-name change. The "Changes" code block also lists 3 of the 5 helpers.

Also: "Four lists reorder" is wrong per #6, and the IAN-NN-2.4 ≡ TODD-NTH-3.1 line will not parse for most readers.

16. The checklist column's accessible name drops the short name it is labelling

File: extensions/src/platform-scripture/src/components/checklist.component.tsx:239 · Severity: Minor

The column shows WEB but announces "Project: World English Bible", so a screen-reader user cannot match the column to the toolbar or tab titles.

:238-239 computes normalizeFullName(fullName) ?? shortName and :247-252 puts it in aria-label over visible text of shortName. formatProjectName({ shortName, fullName }) would keep the short name first and match the rest of the branch. Under finding #1 this announces "Project: *Name Missing*".

17. The new sort key is read without the null guard the old one had

File: extensions/src/platform-scripture/src/find/find.component.tsx:776 · Severity: Minor (lower confidence)

compareProjectsByName calls a.shortName.localeCompare, and shortName comes straight from getSetting('platform.name') with no normalization, where the previous sort key was coerced to ''.

find.web-view.tsx:129-134 guards fullName through normalizeFullName but passes projectShortName through raw; checklist.web-view.tsx:592-595 documents that "pdp.getSetting can return null for missing settings" and guards for it. If a PDP ever does, the sort throws a TypeError during render. I did not find a provider that actually returns null here — for the core C# provider the default path (#1) prevents it — so treat this as hardening, not a live bug.

18. The rule is enforced by a build-failing test but recorded in no standard

File: .context/standards/Architecture-Decisions.md:2104 · Severity: Minor

CLAUDE.md asks that a decision which hardens into a rule also be folded into the relevant standard, since that is what the agents read on the next feature.

Nothing in .context/standards/ or .claude/rules/ outside the ADR mentions formatProjectName/compareProjectsByName; Component-Selection-Quick-Reference.md:178 describes the selector's two name fields without the rule. A short "Project names" section naming the helpers, the de-dup rule and how to exempt a site would do it.

19. Consider inline exemption markers instead of the central substring list

File: src/renderer/components/projects/project-name-adoption.test.ts:5 · Severity: Minor

A central list keyed on literal substrings of 23 sites across three trees means an unrelated reformat in extensions/ fails a renderer test, with the reason living far from the code it excuses.

The repo's own precedent for this shape of guard — the Send/Receive write gate, which CLAUDE.md documents — puts a per-site // SR-write-gate: exempt — <reason> marker at the write and keeps only not-applicable files in a central list. That also makes the "exactly one match" test unnecessary. (A source-scan vitest test is otherwise well-precedented here — papi-dts-data-providers.test.ts, shipped-simple-layout-order.test.ts — so I would not push you toward an ESLint rule; if you considered and rejected one, the ADR's Alternatives is the place to say so.)

---

Nits — batch these or skip them

20. PROJECT_NAME_SEPARATOR's TSDoc overstates RTL safety

File: lib/platform-bible-utils/src/project-util.ts:51 — "the surrounding element's direction handles right-to-left layout" holds for the toolbar, where the separator is its own flex item, but formatProjectName returns one text node with a bidi-neutral hyphen, and that string is used in tooltips, the manage-books subtitle and aria-labels.

21. String.replace with a string replacement interprets $&, $`, $' and $$

File: extensions/src/platform-scripture/src/components/checklist.component.tsx:239 — a project named Nuevo $' Test renders as Project: Nuevo Test. Pre-existing, but it is the line you touched; a replacer function or formatReplacementString fixes it.

22. The scroll-group middle dot lands inside the accessible name

File: lib/platform-bible-react/src/components/advanced/project-selector/project-selector.component.tsx:1283 — in projectScrollGroup mode the trigger announces "Project: WEB · A"; screen readers read the dot.

23. The ADR entry is 37 lines and repeats itself

File: .context/standards/Architecture-Decisions.md:2082 — the Consequences paragraph lists the five sort sites then re-lists four of them, and spends two lines on test mechanics that the TSDoc already carries. Byte-order slug placement is correct.

24. Comment length and a few wording gaps

Files: project-name-adoption.test.ts:138 carries PR context ("The short-name-first work asks only that Share Layout be left unaffected") — the strip-the-PR-context test cuts it; :25 uses UK "behaviour"; :223's docstring runs 13 lines; project-selector.component.tsx:1312 (9 lines) and platform-bible-toolbar.tsx:340 (5 lines) overlap on the same explanation; find.web-view.tsx:131 and checks-side-panel.web-view.tsx:49 carry identical 3-line comments that would read fine as one line each.

25. compareProjectsByName forces string callers to build throwaway objects

File: src/renderer/hooks/use-sync-status.hook.ts:713 — compareProjectsByName({ shortName: a.name }, { shortName: b.name }) allocates twice per comparison. Either accept Pick<ProjectNames, 'shortName'> or export a string-level comparator that the object form delegates to.

26. A new comment's justification does not match the code it justifies

File: extensions/src/platform-scripture/src/manage-books-dialog/manage-books-dialog.component.tsx:767 — it says mirroring "would make every project look like it has a distinct full name", but hasDistinctFullName (project-util.ts:38) treats an equal full name as not distinct, and manage-books.web-view.tsx:795,854 mirrors upstream anyway. No visible bug; the reasoning will mislead.

---

What I checked and did not flag

• The adoption sweep runs green and is fast (~200 ms, 24 flagged lines, 24 exemptions each matching once); CRLF, Windows separators and SELF_REFERENTIAL_FILES all behave, and a wrong working directory fails loudly with ENOENT rather than passing silently.
• No e2e spec queries the selector's combobox by exact accessible name, so the aria-label change breaks no e2e test.
• getTabTitleProjectName is a justified test seam, and the tab-title reading in the ADR is correct: tab titles read platform.name and never showed the full name.
• use-project-picker-data.hook.test.ts:458's sibling test deliberately reverses fullName order against shortName order — a genuinely falsifiable test.
• The ADR entry's slug is in correct byte-order position.
• Toolbar test mocks do not leak between cases; the equal-names text de-dup test does fail under the regression it names (unlike its hover sibling, #3).

Process note

/code-review max did not complete — the session hit its rate limit partway through and killed nine of its ten finder agents. Findings 1–4, 16, 17, 21 and 26 come from the one angle that reported back before it died (I independently confirmed #1 against the C# source). The rest come from three full reviews — quality/soundness, architecture, comments/docs — and an adversarial pass over all of them, which downgraded several severities and refuted a handful of items that are not in this report. Worth re-running /code-review max after the limit resets for the coverage the missing nine angles would have added.

@katherinejensen00 partially reviewed 52 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on jolierabideau).

---

src/renderer/components/settings-tabs/settings-tab.component.tsx line 58 at r1 (raw file):

  const [projectName, projectFullNameRaw] = await Promise.all([
    pdp.getSetting('platform.name'),
    pdp.getSetting('platform.fullName'),

1. platform.fullName returns a localized *Name Missing* placeholder when unset, so Settings will show "WEB - *Name Missing*"

File: src/renderer/components/settings-tabs/settings-tab.component.tsx:58 · Severity: Important

normalizeFullName only treats null, undefined and '' as absent, but a project data provider never returns those for this setting — it returns the contribution default.

When the Paratext FullName setting is missing, ParatextProjectDataProvider.cs:1783-1786 falls through to ProjectSettingsService.GetDefault, and core-project-settings-info.data.ts:18 declares that default as '%project_full_name_missing%' → "*Name Missing*" (assets/localization/en.json:361). normalizeFullName returns that string unchanged, hasDistinctFullName reports true, and the Settings sidebar — which this PR switched to triggerLabelFormat="shortNameAndFullName" — renders WEB - *Name Missing*, with the same text in the row's second line and in the trigger's accessible name. Before this PR that surface showed WEB.

Your own C# comments already name this exact hazard: ProjectSummary.cs:89-91 says its string.Empty fallback "does NOT reproduce platform.fullName's default - a localized %project_full_name_missing% placeholder", and ProjectMetadata.cs:63-66 says FullName is null when the setting is absent. So the metadata-sourced surfaces (toolbar, manage-books) will show WEB while the PDP-sourced surfaces show WEB - *Name Missing* — the cross-surface inconsistency this PR exists to remove. The same read is new in find.web-view.tsx:134, checks-side-panel.web-view.tsx:52 and checklist.web-view.tsx:375.

Fix: source the full name from getMetadataForAllProjects() (which this component already calls, and which is null-when-unset by design), or have normalizeFullName reject the placeholder. See also #9.