[MC-458-J] fix(frontend): key grouped-concept cognate/similarity on the concept uid

[TrueNorth49]

Problem

In Compare, buildSpeakerForm resolved grouped-concept cognate and similarity under the selected member's raw csv row id (e.g. "89"), via cognateKey = concept.variants[selectedIdx].conceptKey (and the analogous mergedKeys[selectedIdx]). But the MC-458-E uid migration (#640) re-keyed cognate_sets and similarity (both in ROOT_CONCEPT_KEYED_BLOCKS) by the concept identity uid ("c-89"), and the backend auto-promotes legacy keys to uid on read (promote_legacy_uid_keys).

Result: every concept with ≥2 members (e.g. a concept elicited in two surveys) showed a blank cognate letter, while:

• singletons worked — they key on concept.key (the uid);
• per-speaker flags worked on the same rows — flagKey already uses concept.key.

It also meant a manual cognate edit on a grouped concept didn't survive a reload: the write used the raw member id, then the migration moved it to the uid, and the next read (raw id) couldn't find it.

This is the unfinished frontend half of the uid migration: the cognate/similarity read+write key was never moved off the per-realization id for grouped concepts.

Fix

In both grouped branches of buildSpeakerForm (mergedKeys and source-item variants), key cognate + similarity on concept.key (the uid) — matching flagKey and the uid-keyed store. The selected variant still drives which realization is displayed; it no longer determines the cognate key. Singleton keying is unchanged. One concept identity → one cognate decision per speaker, which is the intended model.

Data/language-agnostic: the change and tests use generic conceptKey/speakerId/uid placeholders; no speaker, lexeme, or corpus is involved.

Tests

• Rewrote the two tests that locked the old per-realization keying (…from the selected variant, …from the selected member) to assert uid keying, with member-id keys present as decoys the uid must override.
• Added a regression test for the production shape: two survey rows unified under one uid, cognate stored under the uid → resolves.
• npx vitest run — 1198 passed, 9 skipped, 0 failed. tsc --noEmit clean.

Scope / follow-ups (not in this PR)

This converges the last stranded reader onto the uid. The broader hardening the design note already deferred — a single key-mint chokepoint + branded ConceptKey type, making apply_sksync emit uid directly, and a corpus-level "every decision key is a uid" guard test — remains worth doing to prevent any future surface from drifting again.

🤖 Generated with Claude Code

[TrueNorth49]

Review — PR #648 (self-review, MC-458-J)

Overview. +62/−22, 2 files. Moves grouped-concept cognate+similarity lookup in buildSpeakerForm from the selected member's raw row id to concept.key (the identity uid), matching flagKey and the uid-keyed store the MC-458-E migration produces. Fixes blank cognate letters on every multi-member concept and makes manual cognate edits survive a reload.

Correctness — verified, not assumed:

• The live backend serves uid-keyed cognate_sets and similarity (promote_legacy_uid_keys runs in load_enrichments + the compare route; both blocks are in ROOT_CONCEPT_KEYED_BLOCKS). Reading both under the uid is therefore correct. ✓
• readCompareDecisionFields uses the one cognateKey for both cognate and similarity, so the single change fixes both consistently. ✓
• Singleton path untouched (cognateKey = concept.key already); flags untouched (flagKey = concept.key). ✓
• The live identity-grouped concepts (e.g. the 16 reported) have concept.key = uid (c-89), confirmed against /api/compare/bundles; the added regression test pins exactly that shape. ✓

Tests. Full npx vitest run = 1198 passed / 9 skipped / 0 failed; tsc --noEmit clean. The two tests that locked the old per-realization keying were rewritten to assert uid keying with member-id decoys the uid must override (so they'd fail if the fix regressed), plus a production-shape regression test.

Scoped limitation (documented, not a regression): the fix sets cognateKey = concept.key in both grouped branches. For identity-grouped concepts concept.key is the uid → correct (this is the path Compare uses post MC-458-D). The legacy source-item grouping path (groupConceptEntries) mints concept.key = min(member csv ids) rather than a uid; there cognate would still key on a raw id. That path is superseded by concept-identity in Compare, and the change is no worse than before for it (it already used a raw sibling id). Flagging for awareness; fully retiring that path belongs to the MC-458 convergence work.

Follow-ups (out of scope, noted in the PR body): single key-mint chokepoint + branded ConceptKey; make apply_sksync emit uid directly; corpus-level "every decision key is a uid" guard test. Also: the §6.1 note in 2026-06-02-concept-key-namespace-collision.md still describes per-realization cognate keying and should be updated to reflect the uid decision.

AGENTS.md review rules: (1) language/data-agnostic ✓ — generic placeholders, no speaker/lexeme/corpus; (2) readable ✓; (3) docs — covered by the persistence doc + PR body; the stale §6.1 note is the one doc debt, tracked above.

Risk: low; localized to grouped-concept decision keying, fully covered by tests.

Verdict: approve. Correct, tested, and the right convergence step. Leaving merge to the maintainer.

[TrueNorth49]

Mandatory PR Review Checklist — PR #648 (MC-458-J)

Per pr-review-checklist.md. Concrete evidence per section.

1. Scope Confirmation & Sufficiency

• git diff origin/main...HEAD --stat: 3 files, +63/−22 — src/lib/speakerForm.ts (+14/−6), src/lib/speakerForm.test.ts (+70/−22), docs/reports/2026-06-02-concept-key-namespace-collision.md (+1, §8 corrective).
• PR claim: move grouped-concept cognate+similarity lookup from the selected member's raw row id to the concept uid. Diff matches 1:1 — the only logic change is cognateKey = concept.key in the two grouped branches of buildSpeakerForm (mergedKeys and source-item variants); singleton branch already used concept.key.
• Sufficient? Yes for the root problem (multi-member concepts rendering blank cognate, and manual edits not surviving reload).
• Scoped limitation (stated, not a regression): the legacy groupConceptEntries source-item path mints concept.key = min(member csv id) rather than a uid; there cognate would still key on a raw id. That path is superseded by concept-identity in Compare (MC-458-D) and the change is no worse than before for it.
• Verdict: scope matched and sufficient.

2. Test Execution

• npx vitest run → Test Files 135 passed; Tests 1198 passed | 9 skipped | 0 failed. 0 new failures.
• ./node_modules/.bin/tsc --noEmit → clean (exit 0).
• npm run build → ✓ built in 3.67s. (The ">500 kB chunk" notice is pre-existing and unrelated to a 2-line logic change.)
• Backend pytest / ruff: N/A — diff touches no python/ files (git diff --name-only = the 3 files above).
• Coverage of changed lines: speakerForm.test.ts directly exercises both changed branches. The two tests that locked the old per-realization keying were rewritten to assert uid keying with member-id decoy keys the uid must override (so they fail if the fix regresses), plus a new production-shape regression test (MC-458-J). Before→after is encoded in the diff (old asserted cognateKey === '599'/'248'; now === 'uid:hair'/'uid:head' + cognate from the uid).

3. Critical Systems Impact & Regression Check

• git diff --name-only = speakerForm.ts, speakerForm.test.ts, one docs file. No src/stores/, *annotate*, src/api/contracts/, python/server_routes/, waveform, or save-logic files touched.
• Systems verdicts:
  • Annotation persistence / Save-Load / tiers / boundaries — NO REGRESSION. Not touched; buildSpeakerForm is read-only over enrichments.
  • Waveform & temporal — NO REGRESSION. Untouched.
  • Frontend↔Backend contracts — NO REGRESSION. Read side only; reads manual_overrides.cognate_sets/similarity which MC-458-E + promote_legacy_uid_keys serve uid-keyed; this aligns the reader with that contract.
  • UI state (Compare cognate display) — CORRECTLY EXTENDED. buildSpeakerForm called only at src/ParseUI.tsx:1564; only the cognateKey assignment changed (IPA/realization/selectedIdx/flag paths unchanged). ParseUI.test.tsx 137 passed; compare component tests 87 passed.
  • Inference / batch / data-model invariants — untouched.

4. Interactive Surface Verification

• form.cognateKey is consumed only at SpeakerFormsTable.tsx:1346–1347; form.flagKey at :1357.
  • Cognate cycle (:1346 onCycle → onCycleCognate(…, form.cognateKey)) — correctly modified: now writes under the uid, matching the read (and surviving the uid migration; previously the raw-id write was orphaned on reload).
  • Cognate reset (:1347) — correctly modified: same uid alignment.
  • Speaker flag toggle (:1357 → form.flagKey) — intact: unchanged (flagKey = concept.key already).
• Tests: SpeakerFormsTable.test.tsx (26) + CognateCell.test.tsx (10) passed. Button surface: cognate cycle/reset = correctly modified; flag = intact; all others unaffected.

5. Side-Effect, Dependency & Performance

• Importers of buildSpeakerForm: only src/ParseUI.tsx:1564. Consumers of form.cognateKey: only SpeakerFormsTable.tsx:1346–1347. Tightly bounded blast radius.
• No useEffect/hook dependency changes, no shared-state mutation, no boundary/off-by-one surface. Performance identical (same lookups, different key string). similarity now read under the uid — consistent with its migration into ROOT_CONCEPT_KEYED_BLOCKS.
• No side-effect or performance concern.

6. CI Gates & Merge Hygiene

• gh pr checks 648: Backend CI pass · Frontend CI pass · Schema Compatibility pass · Parity Diff Harness pass (4/4 green).
• Base = main ✓. mergeable: MERGEABLE. mergeStateStatus: BLOCKED — awaiting the required human review approval (branch protection), not a failing gate. Worktree clean; branch up to date with remote.

7. Overall Integrity Verdict

Integrity Verdict: SAFE TO MERGE (pending the required human approval — agents do not merge).

• Scope: matched & sufficient; one superseded legacy path noted.
• Tests: 1198 pass / 0 fail; tsc clean; build OK; regression encoded.
• Critical systems: no regressions (only Compare cognate read/write keying, correctly aligned to uid).
• UI surface: cognate cycle/reset correctly modified; flag intact.
• Risks: low, fully test-covered.
• Recommended before merge: none blocking. Follow-ups (post-merge): single key-mint chokepoint + branded ConceptKey; make apply_sksync emit uid directly; corpus-level "every decision key is a uid" guard test; retire the legacy source-item grouping path in Compare.

8. Communication, Language & Docs Hygiene (corrective — applied)

• 8.1 language/data-agnostic: title, body, and review comments describe the generic mechanism (uid keying for grouped concepts); no language/speaker/corpus framing. No edit needed.
• 8.2 readable: PR body leads with the problem and the fix in plain prose. OK.
• 8.3/8.4 docs (applied, not asked): the §6.1 note in docs/reports/2026-06-02-concept-key-namespace-collision.md still claimed decisions key on the "underlying realization's concept id," which this change (and MC-458-E) supersede. Corrected in-branch (commit 2b20a88):

  added: "Superseded by MC-458 (uid identity) + MC-458-E. Decisions now key on the concept identity uid, not the per-realization member id … Compare reads them under concept.key (the uid) for grouped concepts too (MC-458-J)."

  • Broader user-facing keying docs live in docs/data-persistence-model.md (PR [MC-456-B] docs: persistence model — runtime read path, block shapes, served-but-stale troubleshooting #647). Docs verdict: present & now compliant.