refactor(frontend): Split friends.ts store into focused modules

[enko]

Summary

• Splits the 806-line friends.ts into three focused files:
  • friends.ts (194 lines) — core friend CRUD, derived stores, re-exports
  • friendSubresources.ts (542 lines) — all subresource CRUD operations (phones, emails, addresses, URLs, photos, dates, met info, social profiles, professional history, relationships, circles)
  • friendListFilter.ts (94 lines) — filter/search state persistence
• All existing imports from $lib/stores/friends continue to work via re-exports — zero consumer changes needed

Test plan

• Verify all imports from $lib/stores/friends resolve correctly
• Verify friend list filtering and search persist across navigation
• Verify all subresource CRUD operations work on friend detail page
• Verify relationship type loading and friend search autocomplete

Depends on #119

Closes #113

🤖 Generated with Claude Code

[github-actions]

The buildSearchParams and parseSearchParams methods take an external FriendListFilterState argument rather than reading the store's own current state. This creates an unusual API where callers must subscribe and pass state back in manually. Consider either (a) making these pure utility functions exported separately from the store, or (b) using Svelte's get(friendListFilter) internally so callers invoke them without arguments. The current design leaks internal state shape to callers unnecessarily.

[github-actions]

The FriendListFilterState interface is not exported. Consumers who want to type-annotate variables holding a snapshot of the filter state (e.g. in Svelte components using let state: FriendListFilterState) cannot do so without re-defining the type. Consider exporting it.

[github-actions]

The FriendsUpdate type is defined as (fn: (state: FriendsState) => FriendsState) => void, but storeAction (and Svelte's writable update) actually spreads a Partial<FriendsState> onto the current state, meaning the callback only returns a partial. This is safe at runtime because of the spread in storeAction, but the type could be more accurately described as (fn: (state: FriendsState) => Partial<FriendsState>) => void to match actual usage. As written, TypeScript will reject callbacks that return only a partial object — though since storeAction already types it correctly via S extends { isLoading: boolean; error: string | null }, this is an edge case that may not surface in practice.

[github-actions]

🔍 Automated Code Review

Commit	84e58a6
Reviewed	2026-03-04T20:30:00Z
Status	⚠️ Changes Partially Addressed

What Changed Since Last Review

Commit 84e58a6 — refactor(frontend): Address review suggestions for friendListFilter — addressed two of the three previously flagged issues:

Issue	Status
buildSearchParams/parseSearchParams as store methods with awkward API	✅ Fixed — now standalone exported pure functions
FriendListFilterState interface not exported	✅ Fixed — now exported for consumer type annotations
FriendsUpdate type narrower than storeAction callback signature	⚠️ Inline comment posted in prior run — low severity

Remaining Concerns

⚠️ Missing tests for new pure utility functions

buildSearchParams and parseSearchParams in friendListFilter.ts contain meaningful parsing logic — comma-splitting filter values, enum casting for relationship_category — that is not covered by any test file. The AGENTS.md specifies a >80% coverage target and all tests must pass before merging.

⚠️ Unsafe relationship_category cast in parseSearchParams

filters.relationship_category = relationshipCategory.split(',') as (
  | 'family'
  | 'professional'
  | 'social'
)[];

A malformed URL (e.g. ?relationship_category=invalid) will pass the cast silently and send an invalid enum value downstream to the API. A runtime guard (e.g., .filter(v => ['family','professional','social'].includes(v))) would be safer, or rely on API-layer validation if that is already guaranteed.

Findings

Severity	Count
🚨 Critical	0
⚠️ Important	2 (missing tests, unsafe cast)
💡 Suggestion	0

AGENTS.md Compliance

• ✅ Architecture patterns — clean module split with backward-compatible re-exports
• ✅ TypeScript strict patterns — no any types, proper type exports
• ⚠️ Testing standards — no tests added for new friendListFilter.ts utility functions (>80% target)

Summary

The refactor is well-structured and the previous review's two main design issues have been addressed. The remaining concerns are: (1) unit tests for buildSearchParams/parseSearchParams per AGENTS.md coverage requirements, and (2) the unsafe as cast for relationship_category parsing. Neither is a blocking correctness issue if API validation can be relied upon, but the test gap is a policy concern.

---

_{🤖 Automated review by Claude Code • View CI Run}

[github-actions]

This is a well-executed refactor that successfully splits an 806-line monolithic store into three focused modules with zero consumer-facing changes. The main actionable items are: (1) the buildSearchParams/parseSearchParams API design which leaks internal state shape to callers; (2) the unexported FriendListFilterState interface; (3) missing unit tests for the new modules, particularly the URL parsing logic in friendListFilter.ts; and (4) an unsafe as cast for relationship_category that could silently accept invalid enum values from URL parameters. Requesting changes primarily for the missing tests (per project's stated >80% coverage requirement) and the API design concern.

[github-actions]

⚠️ Re-review after commit 84e58a6: Two of three previously flagged issues have been fixed (FriendListFilterState now exported, buildSearchParams/parseSearchParams now standalone pure functions). Remaining items: (1) unit tests for new utility functions per AGENTS.md coverage requirements, (2) unsafe as-cast for relationship_category in parseSearchParams. No new issues found. See updated summary comment for details.