chore: remove dead petname code from Account model - #42
Merged
Conversation
Petname was deprecated in build 46 (Account.displayLabel was updated to ignore it; doc comment at the time said "petname field on Account remains for on-disk backward compat but UI ignores it"). Two builds later, petname has zero production usage: - displayLabel resolves to kind:0 displayName -> kind:0 name -> 8-char pubkey prefix (never consults petname) - All production callers of addAccount/generateAccount pass petname=nil - renamePetname has zero production callers (only tests exercised it) - The "@petname" mentions in 3 view doc comments were stale — describing aspirational behavior that never shipped past build 46 Only live use was MultiAccountDiagnosticsView (dev-menu-gated) — its "petname" row + timestamp-petname Generate button. Both removed. Removed: - petname: String? field from Account struct - petname: parameter from Account init + 9 call sites - petname: parameter from addAccount + generateAccount - renamePetname(for:to:) method (~14 LOC + doc) - sanitizePetname(_:) private helper (~7 LOC) - petname display + petname-on-generate in MultiAccountDiagnosticsView - 3 renamePetname* tests (testRenamePetname_persistsAndUpdatesCurrent, testRenamePetname_sanitizesInput_audit_A3, testRenamePetname_emptyAfterSanitization_setsNilNotEmptyString) - 2 displayLabel-ignores-petname tests ( testAccount_displayLabel_prefersDisplayNameOverPetname, testAccount_displayLabel_ignoresPetname_usesPubkeyPrefixWhenNoProfile) - Stale "@petname" doc comments in SlimIdentityBar, SigningAsHeader, ConnectAccountContextBar - Stale "renamePetname" doc references in AppState class doc comment + SharedModels.swift Account struct doc + AppStateMultiAccountTests class doc + AccountModelTests internal comments Updated 7 incidental tests to drop petname argument + use addedAt as the disambiguator in testAccount_equatable_isContentBased and testAddAccount_duplicateNsec_switchesToExisting (where petname had been used to verify "didn't replace existing"). Backward compat: existing on-disk Account JSON includes "petname":null (or rare non-null from dev menu). Swift's JSONDecoder ignores unknown keys by default, so build-69+ decoding existing rows is fine — the petname key is silently dropped on next encode. Verification: - xcodebuild test -skip-testing:ClaveUITests on iPhone 17 / iOS 26.4: - Pre-baseline (main @ d535054): 236 passed / 0 failed - Post-deletion (this branch): 231 passed / 0 failed (236 - 5) - ** TEST SUCCEEDED ** in both runs Net change: -108 LOC across 10 files (38 insertions, 146 deletions). Lands as a focused dead-code removal PR before Stage 4a (AccountManager extraction) so the structural refactor stays clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
DocNR added a commit
that referenced
this pull request
May 9, 2026
) Stage 4a — penultimate sub-extraction in the AppState god-object split. Moves all account lifecycle + bunker-URI + persistence logic out of AppState.swift into a dedicated extension file. Cleanup follow-up to the petname-removal PR (#42) that simplified the surface first. Moves out of Clave/AppState.swift (16 methods + 1 MARK header): - var bunkerURI (computed) + bunkerURI(for:) (per-account) - loadAccounts (was private, now internal — called by loadState) - recoverAccountsFromKeychainIfNeeded (Stage 2 deferred — moves now) - cleanupOrphanLegacyKeychainEntry (Stage 2 deferred — moves now) - persistAccountsList (private, stays private — only inter-extension) - persistAccounts - persistCurrentAccountPubkey (private, stays private) - switchToAccount(pubkey:) - addAccount(nsec:) - generateAccount() - deleteAccount(pubkey:) - rotateBunkerSecret() - importKey / generateKey / deleteKey (legacy wrappers preserved) - refreshBunkerSecret() (no-op kept for source-compat) - "// MARK: - Multi-account methods (Task 5)" header Into the new file: - Clave/AppState+AccountManager.swift (312 LOC) — extension AppState What stays in AppState.swift: - @observable stored properties (accounts, currentAccount, bunkerSecretsTick, etc.) — Swift forbids stored properties in extensions. Pointer comment added at the multi-account state block. - init() with NotificationCenter observers - loadState() coordinator (calls into the extension via cross-file extension dispatch — works because all are extension AppState on the same final class in the same module) One AppState.swift access change required: - bunkerSecretsTick was `private(set) var` (file-scope setter). rotateBunkerSecret in the extension needs to mutate it across files, so dropped `private(set)` to internal-set. Same constraint pattern as Stage 3b's persistAccounts widen. Cross-extension calls all resolve correctly: - addAccount calls registerWithProxy (Stage 3c) + fetchProfileIfNeeded (Stage 3b) — works - deleteAccount calls unregisterWithProxy + unpairClientWithProxy (Stage 3c) + cachedImageURL (Stage 3b) — works - switchToAccount calls loadCachedProfileImage + fetchProfileIfNeeded (Stage 3b) — works Zero behavior change: - Function bodies preserved byte-for-byte - All `private` modifiers preserved on persistAccountsList + persistCurrentAccountPubkey (only called within extension file — same-file private access) - Modifiers loosened only where strictly required by cross-file calls (loadAccounts, recoverAccountsFromKeychainIfNeeded, cleanupOrphanLegacyKeychainEntry: private -> internal because loadState in AppState.swift calls them) Test impact: zero changes. - AppStateMultiAccountTests calls appState.<method> via extension dispatch — works unchanged. External callers unchanged: - ClaveApp + 7 view files call appState.<method> — extension dispatch. No pbxproj edits — Clave/ directory is auto-synced. Verification: - xcodebuild test -skip-testing:ClaveUITests on iPhone 17 / iOS 26.4: - Pre-baseline (main @ f51bc18): 231 passed / 0 failed - Post-extraction (this branch): 231 passed / 0 failed - ** TEST SUCCEEDED ** in both runs AppState.swift: 893 -> 611 LOC (-282, -31.6%). Combined Stages 1+2+3+petname+4a: 2,338 -> 611 LOC (-73.9% from sprint start). Stage 4b (PendingApprovalCoordinator, ~290 LOC) is the final extraction. After it lands, AppState should reach the BACKLOG sketched ~300 LOC slim target. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
DocNR added a commit
that referenced
this pull request
May 9, 2026
Internal-only build bundling three merges since build 69 was tagged: - Sprint 5a (#41): bunker pair-cap bypass fix - Petname removal (#42): -108 LOC dead code - Stage 4a (#43): AccountManager extension, -282 AppState LOC Bumps CURRENT_PROJECT_VERSION 69 -> 70 across 4 targets x 2 configs. Cumulative AppState refactor reduction: 2,338 -> 611 LOC (-73.9%). Stage 4b (PendingApprovalCoordinator) is the final refactor PR. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Petname was deprecated in build 46 —
Account.displayLabelwas updated to ignore it, with a comment noting "petname field on Account remains for on-disk backward compat but UI ignores it as of build 46." Two builds later, petname has zero production usage:displayLabelresolves to kind:0displayName→ kind:0name→ 8-char pubkey prefix (never consults petname)addAccount/generateAccountpasspetname: nilrenamePetnamehas zero production callers (only tests exercised it)@petnamementions in 3 view doc comments were stale — describing aspirational behavior that never shipped past build 46The only live use was MultiAccountDiagnosticsView (dev-menu-gated) — its "petname" row + timestamp-petname Generate button. Both removed.
Numbers
10 files changed, −108 net LOC (38 insertions, 146 deletions).
What was removed
Production code:
petname: String?field fromAccountstruct + initpetname:parameter fromaddAccount+generateAccount+ 9 call sitesrenamePetname(for:to:)method + doc (~14 LOC)sanitizePetname(_:)private helper (~7 LOC)Account()constructor petname args inAppState+ProfileFetcher.swiftMultiAccountDiagnosticsView@petnamedoc comments inSlimIdentityBar,SigningAsHeader,ConnectAccountContextBarrenamePetnamedoc references in 3 source files + 2 test filesTests:
renamePetnametests deleted (the methods they tested are gone)displayLabel-ignores-petname tests deleted (the assertion is meaningless without petname to ignore)petname:argument;testAddAccount_duplicateNsec_switchesToExistingnow usesaddedAtas the disambiguator (was using petname before)testAccount_equatable_isContentBasednow usesaddedAtas the differentiatorBackward compatibility
Existing on-disk
AccountJSON has"petname":null(or rare non-null values from the dev menu). Swift'sJSONDecoderignores unknown keys by default, so build 69+ decoding existing rows is fine — thepetnamekey is silently dropped on next encode. No migration needed.Test plan
main(d535054): 236 passed / 0 failed ✅** TEST SUCCEEDED **in both runsgrep -rnE "petname|sanitizePetname|renamePetname" --include="*.swift"→ zero matchesWhy now
Lands as a focused dead-code removal PR before Stage 4a (AccountManager extraction). Keeps the structural refactor PR clean — Stage 4a can be a pure file move without bundling product decisions.
🤖 Generated with Claude Code