Make selectedReferences undefined if referenceIdentifierField is undefined

[anna-parker]

Linked to suggestion from @theosanderson: #5799 (comment) - the selectedReferences object should be undefined in the case where the referenceIdentifierField is undefined

Screenshot

PR Checklist

• All necessary documentation has been adapted.
• The implemented feature is covered by appropriate, automated tests.
• Any manual testing that has been done is documented (i.e. what exactly was tested?)

🚀 Preview: https://updates.loculus.org

[claude]

This PR appears to address #5891 by making referenceIdentifierField required in type signatures and adding proper type guards to avoid the typecast mentioned in the issue.

[anna-parker]

now if referenceIdentifierField is undefined so is the selectedReferences object

[claude]

Claude finished @anna-parker's task in 1m 56s —— View job

---

Code Review Complete

I've reviewed the changes in this PR that make selectedReferences undefined when referenceIdentifierField is undefined. Overall, this is a solid type-safety improvement that addresses the concern raised in the linked discussion.

Summary

Strengths:

• Consistent type propagation throughout the codebase
• Proper handling of undefined values with appropriate checks
• Tests updated to reflect the new behavior
• The changes correctly enforce that when there's no referenceIdentifierField, there should be no selectedReferences object

Areas for consideration:

• Some verbose type narrowing patterns (see inline comments)
• A few opportunities for code cleanup (removing outdated TODOs)
• Runtime validation that could potentially be moved to config validation time

Type Safety

The PR successfully makes the types more explicit by:

1. Making selectedReferences and setSelectedReferences optional in component props
2. Ensuring they're only defined when referenceIdentifierField is defined
3. Using conditional logic to create these values only when appropriate (see useSearchPageState.ts:154-167)

The pattern of using ternary operators with object spreads to satisfy TypeScript's type narrowing is correct, though somewhat verbose.

Code Quality

The changes maintain good defensive programming practices:

• Triple-checks in SearchForm.tsx:185-187 ensure all related values are defined together
• Proper early returns when selectedReferences is undefined
• Consistent handling across all affected components

Testing

Test updates in search.spec.ts and sequenceTypeHelpers.spec.ts look appropriate and continue to pass with the new type signatures.

Minor Issues (see inline comments)

I've added inline comments on:

1. An outdated TODO comment that references the issue this PR addresses
2. Verbose type narrowing patterns that might benefit from clarification
3. A conditional check that might be over-defensive given TypeScript's guarantees

Recommendation

This PR is ready to merge after addressing the inline comments. The changes are type-safe, well-tested, and improve code clarity by making the relationship between referenceIdentifierField and selectedReferences explicit.

---

[claude]

Good defensive programming! This triple-check ensures all three related values are defined together before rendering the ReferenceSelector. However, TypeScript should guarantee that if referenceIdentifierField is defined, then both selectedReferences and setSelectedReferences are also defined (based on the logic in useSearchPageState.ts). Consider whether this could be simplified to just checking referenceIdentifierField !== undefined with a type assertion, or adding a comment explaining why all three checks are necessary.

[anna-parker]

yeah... I still think this is kinda ugly