[WIP] feat: kind:777 spell support (NIP-A7) — custom feeds#3711
Draft
alltheseas wants to merge 8 commits intodamus-io:masterfrom
Draft
[WIP] feat: kind:777 spell support (NIP-A7) — custom feeds#3711alltheseas wants to merge 8 commits intodamus-io:masterfrom
alltheseas wants to merge 8 commits intodamus-io:masterfrom
Conversation
Contributor
📝 WalkthroughWalkthroughThis pull request adds comprehensive NIP-A7 "Spells" support to the Damus Nostr client. Spells are Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Spell UI<br/>(Views)
participant Model as Feed & Resolver<br/>(Models)
participant Net as Network<br/>(Relays)
participant DB as Event Cache
User->>UI: Select/Create Spell Feed
UI->>Model: Activate spell or build filter
Model->>Model: Parse SpellEvent (kind:777)
Model->>Model: Resolve variables ($me, $contacts)
Model->>Model: Resolve timestamps (relative→absolute)
Model->>Model: Build NostrFilter from tags
Model->>Net: Subscribe with resolved filter
Net-->>Model: Stream events
Model->>DB: Store/cache events
Model-->>UI: Update published state (.loaded)
UI-->>User: Render timeline of results
sequenceDiagram
participant User
participant FilterBuilder as FilterBuilder<br/>(UI/Model)
participant Store as FeedTabStore
participant Discovery as Discovery View
User->>FilterBuilder: Fill form (name, kinds, scope)
FilterBuilder->>FilterBuilder: Validate: name + kinds present
User->>FilterBuilder: Tap Save
FilterBuilder->>FilterBuilder: buildSavedFeed()
FilterBuilder->>FilterBuilder: Generate keypair & build kind:777 NdbNote
FilterBuilder->>FilterBuilder: Serialize to JSON
FilterBuilder-->>Store: Return SavedSpellFeed
Store->>Store: Add to savedFeeds, persist to UserDefaults
Store-->>Discovery: Update tabs list
Discovery-->>User: Show new feed in tab bar
sequenceDiagram
participant Timeline as PostingTimelineView
participant TabStore as FeedTabStore
participant SpellModel as SpellFeedModel
participant Resolver as SpellResolver
participant Net as Relays
Timeline->>TabStore: Load (on appear)
Timeline->>TabStore: seedStarterFeedsIfNeeded()
Timeline->>TabStore: selectTab(:)
TabStore-->>Timeline: selectedTab changes
Timeline->>SpellModel: activateSpellFeed()
SpellModel->>SpellModel: Parse spell event
SpellModel->>Resolver: resolve(spell, context)
Resolver-->>SpellModel: ResolvedSpell with filters
SpellModel->>Net: subscribe(filters)
Net-->>SpellModel: Stream events
SpellModel-->>Timeline: state = .loaded, events published
Timeline-->>User: Render SpellFeedView with results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
alltheseas
changed the title
alltheseas
force-pushed
the
spells/kind-777-nip-a7
branch
from
f17f889 to
0b0a8eb
Compare
Collaborator
Author
|
@coderabbitai review please |
Contributor
|
🧠 Learnings used✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (21)
damus/Features/Spells/Views/FeedManagementView.swift (2)
130-141: Add docstrings to helper functions.Per coding guidelines, ensure docstring coverage for added code.
📝 Suggested docstrings
+ /// Commits a pending feed rename, trimming whitespace. + /// - Parameter feed: The feed being renamed private func commitRename(_ feed: SavedSpellFeed) { let trimmed = editingName.trimmingCharacters(in: .whitespaces) if !trimmed.isEmpty { store.renameFeed(id: feed.id, newName: trimmed) } editingFeedId = nil } + /// Generates a brief summary of a spell's filter criteria. + /// - Parameter spell: The parsed spell event + /// - Returns: A comma-separated string of up to 3 kind filters private func spellSummary(_ spell: SpellEvent) -> String {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/Spells/Views/FeedManagementView.swift` around lines 130 - 141, Add docstrings for the two helper functions commitRename(_:) and spellSummary(_:) describing their purpose, parameters, return value (if any), and any important behavior (e.g., commitRename trims whitespace and clears editingFeedId; spellSummary returns a comma-separated summary of up to three kinds). Place concise Swift-style /// comments immediately above each function declaration so they follow the project's docstring convention.
85-92: Consider caching parsed spell to avoid repeated JSON parsing.
feed.parseSpell()is called during each render of the row. While List cells are lazy, scrolling could trigger repeated parsing. Consider memoizing the result if performance becomes an issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/Spells/Views/FeedManagementView.swift` around lines 85 - 92, The row repeatedly calls feed.parseSpell() during rendering which can cause repeated JSON parsing; cache the parsed result and use that cached value in the view. Add a cachedParsedSpell property on the Feed model or a memoizing wrapper (e.g., a lazy stored property or computed property that stores its result) and update FeedManagementView to call that cached property instead of feed.parseSpell(); ensure the cache is updated when feed.json or the underlying data changes so spellSummary(_:) receives a stable, pre-parsed Spell instance.damus/Features/Spells/Models/StarterSpells.swift (2)
10-12: Add docstrings forStarterSpellsstruct and its members.Per coding guidelines, ensure docstring coverage for added code.
📝 Suggested docstrings
+/// Provides hardcoded starter spell feeds that are seeded into `FeedTabStore` +/// on first launch to give users an initial set of custom feeds. struct StarterSpells { - /// Starter feeds seeded into FeedTabStore on first launch. + /// Starter feeds seeded into `FeedTabStore` on first launch. + /// Contains predefined feeds like "Global" and "Images". static let feeds: [SavedSpellFeed] = buildStarterFeeds() + /// Builds the initial set of starter feeds using a generated keypair. + /// + /// Each feed is created as a kind:777 spell event with appropriate filter tags. + /// - Returns: Array of `SavedSpellFeed` ready for persistence. private static func buildStarterFeeds() -> [SavedSpellFeed] {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/Spells/Models/StarterSpells.swift` around lines 10 - 12, Add documentation comments for the StarterSpells struct and its static member feeds: describe the purpose of StarterSpells (holds starter spell feeds seeded into FeedTabStore on first launch), document the feeds property type ([SavedSpellFeed]) and that it is populated by buildStarterFeeds(), and mention any invariants (e.g., immutable, seeded only on first launch) and usage context (used by FeedTabStore). Place docstrings above the struct declaration and above the feeds property, referencing StarterSpells, feeds, and buildStarterFeeds() so readers know where the data comes from.
32-44: The "Images" feed heuristic usingsearch: "http"is weak.Searching for "http" will match any URL, not just images. Consider a more specific filter or renaming the feed to something like "Links" to match the actual behavior.
💡 Possible alternatives
Option 1: Rename to reflect actual behavior:
- name: "Images", + name: "Links",Option 2: Use a more targeted search pattern if the relay supports it:
- tags: [["cmd", "REQ"], ["k", "1"], ["since", "-24h"], ["search", "http"]] + tags: [["cmd", "REQ"], ["k", "1"], ["since", "-24h"], ["search", ".jpg OR .png OR .gif OR .webp"]]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/Spells/Models/StarterSpells.swift` around lines 32 - 44, The "Images" starter feed currently creates an NdbNote with tags containing ["search", "http"] which will match any URL, not just image assets; update the SavedSpellFeed with id "starter_images" (and the displayed name "Images") to either: 1) rename the feed to "Links" (change the name field) if you intend to capture all HTTP URLs, or 2) tighten the search tag to a more specific image-oriented pattern (e.g., look for common image extensions or MIME hints in the search tag) if the relay supports more precise queries—modify the NdbNote tags array used when creating the event (the tags value passed into NdbNote) accordingly so the filter matches actual images rather than all http links.damus/Features/Spells/Views/SpellOnboardingView.swift (1)
75-88: Add docstring tofeatureRowhelper function.Per coding guidelines, ensure docstring coverage for added code.
📝 Suggested docstring
+ /// Creates a feature explanation row with an icon and descriptive text. + /// - Parameters: + /// - icon: SF Symbol name for the row icon + /// - text: Localized description text + /// - Returns: A horizontal stack view with the icon and text private func featureRow(icon: String, text: String) -> some View {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/Spells/Views/SpellOnboardingView.swift` around lines 75 - 88, Add a concise Swift docstring above the featureRow(icon:text:) helper describing its purpose (renders an HStack row with an SF Symbol and text), document the parameters `icon` (SF Symbol name) and `text` (display string), and note the return type is a SwiftUI View (some View); ensure it follows project docstyle (brief summary line, param tags for `icon` and `text`, and a returns/returns-description if your style requires it) and place it immediately above the `private func featureRow(icon: String, text: String) -> some View {` declaration.damus/Core/Nostr/NostrKind.swift (1)
37-37: Consider placingspell = 777in numeric order for consistency.The enum cases are roughly ordered by raw value.
spell = 777would fit better afterzap_request = 9734(or near other sub-1000 kinds) rather than betweencontact_card = 30_382andfollow_list = 39089.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Core/Nostr/NostrKind.swift` at line 37, Move the enum case "spell = 777" out of the high-value block and place it with the sub-1000 kinds to keep rawValue ordering consistent; specifically relocate the "case spell = 777" declaration so it sits near other low-number kinds (e.g., alongside "zap_request = 9734" / other sub-1000 entries) instead of between "contact_card" and "follow_list".damus/Features/Spells/Views/SpellDiscoveryView.swift (2)
10-23: Add docstrings for public types.The
SpellDiscoveryViewstruct lacks a documentation comment describing its purpose and usage.As per coding guidelines: "Ensure docstring coverage for any code added or modified"
📝 Suggested docstring
+/// A sheet view for browsing and adding discovered spell feeds from the network. +/// +/// Presents a searchable list of `kind:777` spell events discovered from relays, +/// allowing users to add them as feed tabs. struct SpellDiscoveryView: View {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/Spells/Views/SpellDiscoveryView.swift` around lines 10 - 23, Add a concise documentation comment for the public struct SpellDiscoveryView explaining its purpose (a SwiftUI view for discovering spells), expected inputs (DamusState and FeedTabStore), and any important behavior (uses SpellDiscoveryModel via StateObject, manages search/filter/management state and dismiss via Environment). Place the docstring above the struct declaration (SpellDiscoveryView) and mention the initializer and key state properties (model, searchText, showFilterBuilder, showManagement) so consumers know how to instantiate and what state is managed.
143-195: Add docstring forSpellDiscoveryRow.📝 Suggested docstring
+/// A row displaying a discovered spell feed with an add button. struct SpellDiscoveryRow: View {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/Spells/Views/SpellDiscoveryView.swift` around lines 143 - 195, Add a Swift docstring comment to the SpellDiscoveryRow View struct that briefly describes its purpose (renders a discoverable spell row with name, description, summary and add button), lists and explains the parameters/properties (damus_state: DamusState, discovered: DiscoveredSpell, isAdded: Bool, onAdd: () -> Void), and notes any important behavior (button disabled when isAdded, uses private var spellSummary to build the kinds summary). Place the doc comment immediately above the SpellDiscoveryRow declaration so IDEs and generated docs surface its intent and parameters.damus/Features/Spells/Views/SpellFeedView.swift (1)
10-43: Add docstrings for view types.The
SpellFeedView,SpellFeedLoadingView,SpellFeedEmptyView, andSpellFeedErrorViewstructs lack documentation comments.As per coding guidelines: "Ensure docstring coverage for any code added or modified"
📝 Suggested docstrings
+/// Displays a spell feed with loading, empty, and error states. +/// +/// Observes a `SpellFeedModel` and renders the appropriate UI based on the +/// current subscription state. struct SpellFeedView: View {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/Spells/Views/SpellFeedView.swift` around lines 10 - 43, Add documentation comments (docstrings) for the view structs introduced/modified: SpellFeedView, SpellFeedLoadingView, SpellFeedEmptyView, and SpellFeedErrorView. For each struct add a brief Swift doc comment describing its responsibility, key parameters/properties (e.g., damus_state, model, spellName, error), and any important behavior (e.g., triggers model.subscribe() on appear, displays empty/error/loading states), so the intent and public API of these views are clear.damus/Features/Spells/Models/FeedTabStore.swift (1)
100-111: Consider logging persistence failures.Both
loadSavedFeeds()andpersistSavedFeeds()silently swallow decode/encode errors. While this is acceptable for graceful degradation, logging these failures would help diagnose issues in production.📝 Optional: Add error logging
private func loadSavedFeeds() { guard let data = userDefaults.data(forKey: Self.savedFeedsKey) else { return } - if let decoded = try? JSONDecoder().decode([SavedSpellFeed].self, from: data) { + do { + let decoded = try JSONDecoder().decode([SavedSpellFeed].self, from: data) savedFeeds = decoded + } catch { + Log.error("Failed to decode saved feeds: %@", error.localizedDescription) } } private func persistSavedFeeds() { - if let data = try? JSONEncoder().encode(savedFeeds) { + do { + let data = try JSONEncoder().encode(savedFeeds) userDefaults.set(data, forKey: Self.savedFeedsKey) + } catch { + Log.error("Failed to encode saved feeds: %@", error.localizedDescription) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/Spells/Models/FeedTabStore.swift` around lines 100 - 111, Replace the silent try? usage in loadSavedFeeds() and persistSavedFeeds() with explicit do-catch blocks that log errors; in loadSavedFeeds() wrap JSONDecoder().decode([SavedSpellFeed].self, from: data) in do-catch and log a descriptive message including Self.savedFeedsKey and the decoding error if it fails, and in persistSavedFeeds() wrap JSONEncoder().encode(savedFeeds) in do-catch and log a descriptive message including Self.savedFeedsKey and the encoding error if it fails; use the app's logging facility (e.g., os_log or the project's Logger) so failures around userDefaults, savedFeeds, JSONDecoder/JSONEncoder are recorded for diagnosis.damus/Features/Spells/Views/SpellTimelineView.swift (1)
25-57: Consider movingEmptyTimelineViewoutside theLazyVStack.When
eventsis empty, placingEmptyTimelineView()inside theLazyVStackworks but is semantically odd—the empty state isn't a lazy-loaded list item. Moving the conditional outside would be cleaner:♻️ Suggested refactor
var body: some View { - LazyVStack(spacing: 0) { - if events.isEmpty { - EmptyTimelineView() - } else { + if events.isEmpty { + EmptyTimelineView() + } else { + LazyVStack(spacing: 0) { let indexed = Array(zip(events, 0...)) ForEach(indexed, id: \.0.id) { tup in // ... } } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/Spells/Views/SpellTimelineView.swift` around lines 25 - 57, The EmptyTimelineView should be returned before creating the LazyVStack when events is empty: in the body property check events.isEmpty and immediately return EmptyTimelineView(), otherwise build the LazyVStack with ForEach over indexed events; update the code around body / LazyVStack to remove the if-else inside the LazyVStack and ensure the ForEach uses indexed as before (refer to symbols: body, LazyVStack, EmptyTimelineView, events, indexed, ForEach, SpellResultView).damus/Features/Spells/Models/SpellDiscoveryModel.swift (1)
24-56: Cannot reload discoveries after initial load completes.The
guard fetchTask == nilcheck preventsload()from being called again, even after the stream ends. If the user wants to refresh discoveries, they must create a new model instance.Consider adding a
reload()method or clearingfetchTaskwhen the stream completes:♻️ Suggested enhancement
for await item in stream { guard !Task.isCancelled else { break } switch item { case .event(let lender): await lender.justUseACopy { [weak self] event in guard let self else { return } self.handleEvent(event) } case .ndbEose, .eose: self.isLoading = false case .networkEose: break } } +// Stream ended - allow reload +self.fetchTask = nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/Spells/Models/SpellDiscoveryModel.swift` around lines 24 - 56, The load() guard fetchTask == nil prevents subsequent loads; update the task lifecycle so fetchTask is cleared when the stream finishes or is cancelled (or add a reload() that cancels and resets fetchTask then calls load()). Specifically, inside the Task created in load() ensure you set fetchTask = nil when the for-await loop exits (including after handling .ndbEose/.eose and when Task.isCancelled causes a break), and/or implement reload() that calls fetchTask?.cancel(), sets fetchTask = nil and then invokes load() to restart the stream.damus/Features/Timeline/Views/PostingTimelineView.swift (2)
227-241: No user feedback when spell parsing fails.If
saved.parseSpell()returnsnil, the function silently setsactiveSpellModel = nil, leaving the user on a blank tab with no indication of what went wrong. Consider logging or showing an error state.🛡️ Suggested improvement
private func activateSpellFeed(for tabId: String) { activeSpellModel?.unsubscribe() guard case .spell(let saved) = feedTabStore.selectedTab, let spell = saved.parseSpell() else { + Log.error("Failed to parse spell for tab: %s", for: .spells, tabId) activeSpellModel = nil activeSpellFeedId = nil return } let model = SpellFeedModel(damus_state: damus_state, spell: spell) activeSpellModel = model activeSpellFeedId = tabId model.subscribe() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/Timeline/Views/PostingTimelineView.swift` around lines 227 - 241, In activateSpellFeed, when guard fails because saved.parseSpell() returns nil, currently the code silently clears activeSpellModel/activeSpellFeedId; instead add user feedback and logging: log the parse failure (include the saved value) and set an error state or notify the UI (e.g., set an optional activeSpellError or call a showError/Alert method) so the tab displays an error message rather than a blank view; update any consumers of activeSpellModel/activeSpellFeedId to read the new error state and ensure SpellFeedModel creation (SpellFeedModel(damus_state:spell) and model.subscribe()) still only runs when a valid spell exists.
219-225: Extract magic string to a constant.The UserDefaults key
"spell_onboarding_shown"is a magic string that could drift if used elsewhere.♻️ Suggested refactor
+private let spellOnboardingShownKey = "spell_onboarding_shown" + private func showOnboardingIfNeeded() { - let key = "spell_onboarding_shown" - guard !UserDefaults.standard.bool(forKey: key) else { return } + guard !UserDefaults.standard.bool(forKey: spellOnboardingShownKey) else { return } guard feedTabStore.tabs.count > 1 else { return } - UserDefaults.standard.set(true, forKey: key) + UserDefaults.standard.set(true, forKey: spellOnboardingShownKey) showSpellOnboarding = true }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/Timeline/Views/PostingTimelineView.swift` around lines 219 - 225, Extract the magic string "spell_onboarding_shown" into a single constant and use that constant in showOnboardingIfNeeded to avoid drift; e.g., add a private static let spellOnboardingShownKey = "spell_onboarding_shown" (or a similarly named private constant) on the PostingTimelineView type and replace both UserDefaults.standard.bool(forKey: key) and UserDefaults.standard.set(true, forKey: key) to reference that constant so all uses are consistent.damus/Features/Spells/Views/SpellCardView.swift (1)
111-118: Silent failure when spell parsing fails leaves an empty card.If
SpellEvent.parse(from: event)returnsnil, theEventShellrenders with an empty content closure, producing a card with header/footer but no body. Consider adding a fallback or error indicator.🛡️ Suggested fix
var body: some View { EventShell(state: state, event: event, options: options) { if let spell = SpellEvent.parse(from: event) { SpellCardBody(spell: spell, state: state) .padding(.horizontal, 16) + } else { + Text(NSLocalizedString("Unable to display spell", comment: "Fallback text when spell event cannot be parsed")) + .foregroundColor(.secondary) + .padding() } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/Spells/Views/SpellCardView.swift` around lines 111 - 118, When SpellEvent.parse(from: event) returns nil the EventShell content is empty; update the closure in SpellCardView's body so it handles the nil case by providing a fallback UI (e.g., a small placeholder or error indicator) instead of rendering nothing. Specifically, keep the existing if let spell = SpellEvent.parse(from: event) { SpellCardBody(spell: spell, state: state).padding(.horizontal, 16) } and add an else branch that returns a visible fallback view (for example a compact Text or a new SpellCardPlaceholder view that uses state/event/options for context) so the EventShell always has body content when parsing fails.damus/Features/Spells/Models/SpellEvent.swift (1)
127-131: Consider renaminglettertotagNamefor clarity.The property is called
letterbut can represent any tag name string (e.g.,"t","p","e"). While these happen to be single letters in practice,tagNamewould be more semantically accurate.♻️ Optional rename for clarity
/// A tag filter condition from a spell: `["tag", <letter>, <values...>]` struct SpellTagFilter: Equatable { - let letter: String + let tagName: String let values: [SpellValue] }Update usages in SpellResolver.swift accordingly:
- switch tagFilter.letter { + switch tagFilter.tagName {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/Spells/Models/SpellEvent.swift` around lines 127 - 131, Rename the SpellTagFilter property `letter` to `tagName` to make its intent clearer: update the struct declaration for SpellTagFilter (replace `let letter: String` with `let tagName: String`) and then update all references/usages in code that access SpellTagFilter. Specifically search and update places like SpellResolver.swift where `letter` is read or assigned so they use `tagName` instead, preserving behavior and types.damus/Features/Spells/Models/FilterBuilderModel.swift (2)
74-75: Consider usingUInt32forlimitto matchNostrFilter.limit.
NostrFilter.limitisUInt32?, but this property isInt. While this works, usingUInt32would be more type-consistent and prevent negative values at compile time.♻️ Optional type alignment
/// Result limit. - `@Published` var limit: Int = 100 + `@Published` var limit: UInt32 = 100Then update line 122:
- tags.append(["limit", "\(limit)"]) + tags.append(["limit", String(limit)])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/Spells/Models/FilterBuilderModel.swift` around lines 74 - 75, The published property limit on FilterBuilderModel is currently Int but should match NostrFilter.limit (UInt32?) to enforce non-negative values and type consistency; change the declaration of limit in FilterBuilderModel to use UInt32 (e.g., `@Published` var limit: UInt32 = 100) and update any places that read or assign it (such as the filter-building code that assigns to NostrFilter.limit—refer to the builder function that constructs/returns a NostrFilter and the usage at the former line 122) to avoid implicit Int/UInt conversions (perform explicit casts where interop is required).
82-130: Add docstring forbuildTags()method.Per coding guidelines, docstring coverage is required for added code. The method lacks documentation explaining the tag structure it builds.
📝 Add docstring
- /// Build the NIP-A7 tags from form state. + /// Build the NIP-A7 tags array from form state. + /// + /// Constructs tags for: `cmd` (always REQ), `k` (kinds), `authors` (scope), + /// `since` (relative time), `search`, `tag` (hashtag), `limit`, and `name`. + /// - Returns: A 2D array of tag strings ready for event creation. func buildTags() -> [[String]] {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/Spells/Models/FilterBuilderModel.swift` around lines 82 - 130, Add a docstring to the buildTags() method describing its purpose and the exact NIP-A7 tag structure it constructs: explain that it returns [[String]] representing tags, always includes ["cmd","REQ"], includes one or more ["k","<kind>"] entries (including parsed customKind), authors tag values ["authors","$me"] or ["authors","$contacts"] or omitted for anyone, optional ["since","-<sincePreset>"], optional ["search","<text>"], hashtag as ["tag","t","<hashtag>"] (stripped of leading #), ["limit","<limit>"], and optional ["name","<feedName>"]; mention trimming/validation behavior (whitespace trimming, customKind parsed as UInt32 > 0, deduplication with selectedKinds) and the stability of sorted selectedKinds.damus/Features/Spells/Models/SpellResolver.swift (2)
96-103: Potential integer truncation for timestamps far in the future.
SpellTimestamp.resolve(now:)returnsUInt64, butNostrFilter.since/untilareUInt32. For timestamps beyond year 2106 (when Unix time exceedsUInt32.max), this truncates silently. Current timestamps are safe, but consider defensive handling.🛡️ Optional: clamp to UInt32.max
// Since if let since = spell.since { - filter.since = UInt32(since.resolve(now: context.now)) + let resolved = since.resolve(now: context.now) + filter.since = resolved > UInt64(UInt32.max) ? UInt32.max : UInt32(resolved) } // Until if let until = spell.until { - filter.until = UInt32(until.resolve(now: context.now)) + let resolved = until.resolve(now: context.now) + filter.until = resolved > UInt64(UInt32.max) ? UInt32.max : UInt32(resolved) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/Spells/Models/SpellResolver.swift` around lines 96 - 103, The code casts SpellTimestamp.resolve(now: context.now) (which returns UInt64) directly to UInt32 for spell.since and spell.until, risking silent truncation for timestamps > UInt32.max; update the SpellResolver logic (the blocks handling spell.since and spell.until) to check the UInt64 result from SpellTimestamp.resolve(now:) and either clamp values above UInt32.max to UInt32.max or handle the overflow case explicitly (e.g., fail validation or log/throw), then assign the safe UInt32 to filter.since and filter.until accordingly so no silent truncation occurs.
105-115: Remove redundant empty-filter guard.Line 110 always assigns
[filter]when chunking is not needed, and line 108 returns at least one filter when chunking is required (sinceauthors.count > 500guarantees at least one chunk). The guard on line 113 can never be triggered.♻️ Simplify by removing the guard
// Chunk large author lists - let filters: [NostrFilter] - if let authors = filter.authors, authors.count > SPELL_MAX_AUTHORS_PER_FILTER { - filters = filter.chunked(on: .authors, into: SPELL_MAX_AUTHORS_PER_FILTER) - } else { - filters = [filter] - } - - guard !filters.isEmpty else { - return .failure(.emptyFilter) - } + let filters: [NostrFilter] = if let authors = filter.authors, + authors.count > SPELL_MAX_AUTHORS_PER_FILTER { + filter.chunked(on: .authors, into: SPELL_MAX_AUTHORS_PER_FILTER) + } else { + [filter] + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/Spells/Models/SpellResolver.swift` around lines 105 - 115, The guard that checks for an empty `filters` array is redundant and should be removed: delete the `guard !filters.isEmpty else { return .failure(.emptyFilter) }` block in the `SpellResolver` logic where `filters` (a `[NostrFilter]`) is created via `filter.chunked(on: .authors, into: SPELL_MAX_AUTHORS_PER_FILTER)` or `[filter]`; leave the preceding chunking assignment intact and ensure subsequent code uses `filters` directly since `filters` can never be empty given the chunking logic and the fallback to `[filter]`.damusTests/SpellEventTests.swift (1)
45-48: Consider adding edge case tests for boundary timestamp values.The current tests don't cover:
- Zero amount (
"0d") — should this parse or be rejected?- Very large amounts that might overflow
These edge cases could reveal parsing issues.
🧪 Optional additional test cases
func test_parse_zero_amount_timestamp() { let ts = SpellTimestamp.parse("0d") // Document expected behavior: nil or .relative with amount 0? XCTAssertNil(ts, "Zero amount should be rejected") } func test_parse_large_amount_timestamp() { let ts = SpellTimestamp.parse("999999999999d") // Verify overflow handling XCTAssertNil(ts) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damusTests/SpellEventTests.swift` around lines 45 - 48, Add two edge-case unit tests for SpellTimestamp.parse: create test_parse_zero_amount_timestamp that calls SpellTimestamp.parse("0d") and asserts the result is nil (or document and assert the chosen expected behavior), and create test_parse_large_amount_timestamp that calls SpellTimestamp.parse("999999999999d") and asserts nil to ensure overflow is handled; include brief comments in each test describing the expected behavior. Ensure the new tests reference the SpellTimestamp.parse symbol and follow the existing test naming/style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@damus.xcodeproj/project.pbxproj`:
- Line 1560: The PBXBuildFile entry with id D73E5EFE2C6A97F4007EB227 is orphaned
(no fileRef) and must be removed; delete the entire PBXBuildFile object for
D73E5EFE2C6A97F4007EB227 from the project.pbxproj and also remove that same id
from the PBXSourcesBuildPhase "files" array where it is referenced so the build
phase no longer points to a non-existent file; after removal scan the file for
any other references to D73E5EFE2C6A97F4007EB227 and eliminate them to keep the
project consistent.
In `@damus/Features/Events/Models/LoadableNostrEventView.swift`:
- Around line 81-82: The enum branch in LoadableNostrEventView incorrectly
treats the .spell case as unsupported (it is listed alongside .contacts,
.metadata, etc.), causing kind:777 note-id references to return
.unknown_or_unsupported_kind; to fix, remove .spell from this unsupported-case
list or add an explicit branch handling .spell that returns the correct
renderable state (instead of .unknown_or_unsupported_kind) so spells are
processed/returned like other supported kinds.
In `@damus/Features/Spells/Models/SpellFeedModel.swift`:
- Around line 79-84: The reset() method currently only sets events = [] and
state = .idle but does not clear the EventHolder's deduplication set, so
recreate or clear the eventHolder to reset its has_event state: inside reset()
call the existing unsubscribe(), then either set eventHolder = EventHolder() or
call a clear method on eventHolder (e.g., eventHolder.clear() or
eventHolder.has_event.removeAll()) so the deduplication Set is emptied before
future subscriptions; keep the rest of reset() behavior (events = [], state =
.idle) unchanged.
- Around line 121-125: The switch's `case .eose` uses `break`, which only exits
the switch not the surrounding `for await` loop, so when `resolved.closeOnEose`
is true the subscription still continues; fix by adding a label to the async
loop (e.g., `outerLoop:`) that encloses the `for await` and replace the `break`
in `case .eose` with a labeled break (e.g., `break outerLoop`) so that when
`resolved.closeOnEose` is true you exit the entire loop after calling
`self.flushAndMarkLoaded()`; locate the `for await` that contains `case .eose`,
the `self.flushAndMarkLoaded()` call, and the `resolved.closeOnEose` check to
implement this change.
In `@damus/Features/Spells/Views/SpellCardView.swift`:
- Around line 46-48: The "limit" badge in SpellCardView.swift is not localized;
update the SpellBadge call that uses spell.limit so the label text is produced
via NSLocalizedString (e.g. a localized format like "limit %d" or "limit %@"
with an appropriate comment) and then formatted with the numeric value before
passing to SpellBadge, keeping the SpellBadge(text: ...) usage and referencing
spell.limit and SpellBadge to locate the change.
In `@damus/Features/Spells/Views/SpellDiscoveryView.swift`:
- Around line 33-35: The current isAlreadyAdded(_ spell: DiscoveredSpell) only
compares feedTabStore.savedFeeds by id (using spell.noteId.hex()), which misses
duplicates when a locally created spell has a different id than a discovered
relay event; update isAlreadyAdded to also detect duplicates by comparing the
spell content (e.g., DiscoveredSpell.spellEventJSON or the canonical spell
payload) against savedFeeds’ stored spell content/JSON so a matching
spellEventJSON (or other canonical content fingerprint) is treated as already
added; keep the existing id check (spell.noteId.hex()) but add a secondary
comparison against savedFeeds’ content field or a content-hash to reliably catch
duplicates created with different IDs.
In `@damus/Features/Spells/Views/SpellResultView.swift`:
- Around line 26-51: The reaction branch in SpellResultView.extract incorrectly
uses event.referenced_ids.last (in the .like case) which is inconsistent with
the rest of the codebase; change it to use event.referenced_ids.first so the
.like case returns .reaction(referencedNoteId: refId, reactorPubkey:
event.pubkey, emoji: emoji) the same way .zap uses .first; update the guard to
let refId = event.referenced_ids.first and keep the rest of the logic (emoji via
to_reaction_emoji, returning .reaction) unchanged.
In `@damus/Features/Timeline/Models/HomeModel.swift`:
- Around line 291-292: The .spell branch is currently ignored but
SpellFeedModel.handleEvent only adds events to its local eventHolder, causing
missing entries in the global cache; update SpellFeedModel.handleEvent (the code
that processes .spell feed events) to also insert the returned events into
damus_state.events in addition to eventHolder so
SpellTimelineView.resolvedTapTarget can find original posts; ensure you add the
same Event objects/IDs used in eventHolder into the shared damus_state.events
cache (preserving keys/IDs) and keep existing behavior for display filtering.
---
Nitpick comments:
In `@damus/Core/Nostr/NostrKind.swift`:
- Line 37: Move the enum case "spell = 777" out of the high-value block and
place it with the sub-1000 kinds to keep rawValue ordering consistent;
specifically relocate the "case spell = 777" declaration so it sits near other
low-number kinds (e.g., alongside "zap_request = 9734" / other sub-1000 entries)
instead of between "contact_card" and "follow_list".
In `@damus/Features/Spells/Models/FeedTabStore.swift`:
- Around line 100-111: Replace the silent try? usage in loadSavedFeeds() and
persistSavedFeeds() with explicit do-catch blocks that log errors; in
loadSavedFeeds() wrap JSONDecoder().decode([SavedSpellFeed].self, from: data) in
do-catch and log a descriptive message including Self.savedFeedsKey and the
decoding error if it fails, and in persistSavedFeeds() wrap
JSONEncoder().encode(savedFeeds) in do-catch and log a descriptive message
including Self.savedFeedsKey and the encoding error if it fails; use the app's
logging facility (e.g., os_log or the project's Logger) so failures around
userDefaults, savedFeeds, JSONDecoder/JSONEncoder are recorded for diagnosis.
In `@damus/Features/Spells/Models/FilterBuilderModel.swift`:
- Around line 74-75: The published property limit on FilterBuilderModel is
currently Int but should match NostrFilter.limit (UInt32?) to enforce
non-negative values and type consistency; change the declaration of limit in
FilterBuilderModel to use UInt32 (e.g., `@Published` var limit: UInt32 = 100) and
update any places that read or assign it (such as the filter-building code that
assigns to NostrFilter.limit—refer to the builder function that
constructs/returns a NostrFilter and the usage at the former line 122) to avoid
implicit Int/UInt conversions (perform explicit casts where interop is
required).
- Around line 82-130: Add a docstring to the buildTags() method describing its
purpose and the exact NIP-A7 tag structure it constructs: explain that it
returns [[String]] representing tags, always includes ["cmd","REQ"], includes
one or more ["k","<kind>"] entries (including parsed customKind), authors tag
values ["authors","$me"] or ["authors","$contacts"] or omitted for anyone,
optional ["since","-<sincePreset>"], optional ["search","<text>"], hashtag as
["tag","t","<hashtag>"] (stripped of leading #), ["limit","<limit>"], and
optional ["name","<feedName>"]; mention trimming/validation behavior (whitespace
trimming, customKind parsed as UInt32 > 0, deduplication with selectedKinds) and
the stability of sorted selectedKinds.
In `@damus/Features/Spells/Models/SpellDiscoveryModel.swift`:
- Around line 24-56: The load() guard fetchTask == nil prevents subsequent
loads; update the task lifecycle so fetchTask is cleared when the stream
finishes or is cancelled (or add a reload() that cancels and resets fetchTask
then calls load()). Specifically, inside the Task created in load() ensure you
set fetchTask = nil when the for-await loop exits (including after handling
.ndbEose/.eose and when Task.isCancelled causes a break), and/or implement
reload() that calls fetchTask?.cancel(), sets fetchTask = nil and then invokes
load() to restart the stream.
In `@damus/Features/Spells/Models/SpellEvent.swift`:
- Around line 127-131: Rename the SpellTagFilter property `letter` to `tagName`
to make its intent clearer: update the struct declaration for SpellTagFilter
(replace `let letter: String` with `let tagName: String`) and then update all
references/usages in code that access SpellTagFilter. Specifically search and
update places like SpellResolver.swift where `letter` is read or assigned so
they use `tagName` instead, preserving behavior and types.
In `@damus/Features/Spells/Models/SpellResolver.swift`:
- Around line 96-103: The code casts SpellTimestamp.resolve(now: context.now)
(which returns UInt64) directly to UInt32 for spell.since and spell.until,
risking silent truncation for timestamps > UInt32.max; update the SpellResolver
logic (the blocks handling spell.since and spell.until) to check the UInt64
result from SpellTimestamp.resolve(now:) and either clamp values above
UInt32.max to UInt32.max or handle the overflow case explicitly (e.g., fail
validation or log/throw), then assign the safe UInt32 to filter.since and
filter.until accordingly so no silent truncation occurs.
- Around line 105-115: The guard that checks for an empty `filters` array is
redundant and should be removed: delete the `guard !filters.isEmpty else {
return .failure(.emptyFilter) }` block in the `SpellResolver` logic where
`filters` (a `[NostrFilter]`) is created via `filter.chunked(on: .authors, into:
SPELL_MAX_AUTHORS_PER_FILTER)` or `[filter]`; leave the preceding chunking
assignment intact and ensure subsequent code uses `filters` directly since
`filters` can never be empty given the chunking logic and the fallback to
`[filter]`.
In `@damus/Features/Spells/Models/StarterSpells.swift`:
- Around line 10-12: Add documentation comments for the StarterSpells struct and
its static member feeds: describe the purpose of StarterSpells (holds starter
spell feeds seeded into FeedTabStore on first launch), document the feeds
property type ([SavedSpellFeed]) and that it is populated by
buildStarterFeeds(), and mention any invariants (e.g., immutable, seeded only on
first launch) and usage context (used by FeedTabStore). Place docstrings above
the struct declaration and above the feeds property, referencing StarterSpells,
feeds, and buildStarterFeeds() so readers know where the data comes from.
- Around line 32-44: The "Images" starter feed currently creates an NdbNote with
tags containing ["search", "http"] which will match any URL, not just image
assets; update the SavedSpellFeed with id "starter_images" (and the displayed
name "Images") to either: 1) rename the feed to "Links" (change the name field)
if you intend to capture all HTTP URLs, or 2) tighten the search tag to a more
specific image-oriented pattern (e.g., look for common image extensions or MIME
hints in the search tag) if the relay supports more precise queries—modify the
NdbNote tags array used when creating the event (the tags value passed into
NdbNote) accordingly so the filter matches actual images rather than all http
links.
In `@damus/Features/Spells/Views/FeedManagementView.swift`:
- Around line 130-141: Add docstrings for the two helper functions
commitRename(_:) and spellSummary(_:) describing their purpose, parameters,
return value (if any), and any important behavior (e.g., commitRename trims
whitespace and clears editingFeedId; spellSummary returns a comma-separated
summary of up to three kinds). Place concise Swift-style /// comments
immediately above each function declaration so they follow the project's
docstring convention.
- Around line 85-92: The row repeatedly calls feed.parseSpell() during rendering
which can cause repeated JSON parsing; cache the parsed result and use that
cached value in the view. Add a cachedParsedSpell property on the Feed model or
a memoizing wrapper (e.g., a lazy stored property or computed property that
stores its result) and update FeedManagementView to call that cached property
instead of feed.parseSpell(); ensure the cache is updated when feed.json or the
underlying data changes so spellSummary(_:) receives a stable, pre-parsed Spell
instance.
In `@damus/Features/Spells/Views/SpellCardView.swift`:
- Around line 111-118: When SpellEvent.parse(from: event) returns nil the
EventShell content is empty; update the closure in SpellCardView's body so it
handles the nil case by providing a fallback UI (e.g., a small placeholder or
error indicator) instead of rendering nothing. Specifically, keep the existing
if let spell = SpellEvent.parse(from: event) { SpellCardBody(spell: spell,
state: state).padding(.horizontal, 16) } and add an else branch that returns a
visible fallback view (for example a compact Text or a new SpellCardPlaceholder
view that uses state/event/options for context) so the EventShell always has
body content when parsing fails.
In `@damus/Features/Spells/Views/SpellDiscoveryView.swift`:
- Around line 10-23: Add a concise documentation comment for the public struct
SpellDiscoveryView explaining its purpose (a SwiftUI view for discovering
spells), expected inputs (DamusState and FeedTabStore), and any important
behavior (uses SpellDiscoveryModel via StateObject, manages
search/filter/management state and dismiss via Environment). Place the docstring
above the struct declaration (SpellDiscoveryView) and mention the initializer
and key state properties (model, searchText, showFilterBuilder, showManagement)
so consumers know how to instantiate and what state is managed.
- Around line 143-195: Add a Swift docstring comment to the SpellDiscoveryRow
View struct that briefly describes its purpose (renders a discoverable spell row
with name, description, summary and add button), lists and explains the
parameters/properties (damus_state: DamusState, discovered: DiscoveredSpell,
isAdded: Bool, onAdd: () -> Void), and notes any important behavior (button
disabled when isAdded, uses private var spellSummary to build the kinds
summary). Place the doc comment immediately above the SpellDiscoveryRow
declaration so IDEs and generated docs surface its intent and parameters.
In `@damus/Features/Spells/Views/SpellFeedView.swift`:
- Around line 10-43: Add documentation comments (docstrings) for the view
structs introduced/modified: SpellFeedView, SpellFeedLoadingView,
SpellFeedEmptyView, and SpellFeedErrorView. For each struct add a brief Swift
doc comment describing its responsibility, key parameters/properties (e.g.,
damus_state, model, spellName, error), and any important behavior (e.g.,
triggers model.subscribe() on appear, displays empty/error/loading states), so
the intent and public API of these views are clear.
In `@damus/Features/Spells/Views/SpellOnboardingView.swift`:
- Around line 75-88: Add a concise Swift docstring above the
featureRow(icon:text:) helper describing its purpose (renders an HStack row with
an SF Symbol and text), document the parameters `icon` (SF Symbol name) and
`text` (display string), and note the return type is a SwiftUI View (some View);
ensure it follows project docstyle (brief summary line, param tags for `icon`
and `text`, and a returns/returns-description if your style requires it) and
place it immediately above the `private func featureRow(icon: String, text:
String) -> some View {` declaration.
In `@damus/Features/Spells/Views/SpellTimelineView.swift`:
- Around line 25-57: The EmptyTimelineView should be returned before creating
the LazyVStack when events is empty: in the body property check events.isEmpty
and immediately return EmptyTimelineView(), otherwise build the LazyVStack with
ForEach over indexed events; update the code around body / LazyVStack to remove
the if-else inside the LazyVStack and ensure the ForEach uses indexed as before
(refer to symbols: body, LazyVStack, EmptyTimelineView, events, indexed,
ForEach, SpellResultView).
In `@damus/Features/Timeline/Views/PostingTimelineView.swift`:
- Around line 227-241: In activateSpellFeed, when guard fails because
saved.parseSpell() returns nil, currently the code silently clears
activeSpellModel/activeSpellFeedId; instead add user feedback and logging: log
the parse failure (include the saved value) and set an error state or notify the
UI (e.g., set an optional activeSpellError or call a showError/Alert method) so
the tab displays an error message rather than a blank view; update any consumers
of activeSpellModel/activeSpellFeedId to read the new error state and ensure
SpellFeedModel creation (SpellFeedModel(damus_state:spell) and
model.subscribe()) still only runs when a valid spell exists.
- Around line 219-225: Extract the magic string "spell_onboarding_shown" into a
single constant and use that constant in showOnboardingIfNeeded to avoid drift;
e.g., add a private static let spellOnboardingShownKey =
"spell_onboarding_shown" (or a similarly named private constant) on the
PostingTimelineView type and replace both UserDefaults.standard.bool(forKey:
key) and UserDefaults.standard.set(true, forKey: key) to reference that constant
so all uses are consistent.
In `@damusTests/SpellEventTests.swift`:
- Around line 45-48: Add two edge-case unit tests for SpellTimestamp.parse:
create test_parse_zero_amount_timestamp that calls SpellTimestamp.parse("0d")
and asserts the result is nil (or document and assert the chosen expected
behavior), and create test_parse_large_amount_timestamp that calls
SpellTimestamp.parse("999999999999d") and asserts nil to ensure overflow is
handled; include brief comments in each test describing the expected behavior.
Ensure the new tests reference the SpellTimestamp.parse symbol and follow the
existing test naming/style.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5087635e-d934-46ff-a838-2df840a6e725
📒 Files selected for processing (32)
.beads/issues.jsonldamus.xcodeproj/project.pbxprojdamus/Core/Nostr/NostrKind.swiftdamus/Features/Events/EventView.swiftdamus/Features/Events/Models/LoadableNostrEventView.swiftdamus/Features/Spells/Models/FeedTab.swiftdamus/Features/Spells/Models/FeedTabStore.swiftdamus/Features/Spells/Models/FilterBuilderModel.swiftdamus/Features/Spells/Models/SpellDiscoveryModel.swiftdamus/Features/Spells/Models/SpellEvent.swiftdamus/Features/Spells/Models/SpellFeedModel.swiftdamus/Features/Spells/Models/SpellResolver.swiftdamus/Features/Spells/Models/StarterSpells.swiftdamus/Features/Spells/Views/FeedManagementView.swiftdamus/Features/Spells/Views/FeedTabBarView.swiftdamus/Features/Spells/Views/FilterBuilderView.swiftdamus/Features/Spells/Views/SpellCardView.swiftdamus/Features/Spells/Views/SpellDiscoveryView.swiftdamus/Features/Spells/Views/SpellFeedView.swiftdamus/Features/Spells/Views/SpellOnboardingView.swiftdamus/Features/Spells/Views/SpellResultView.swiftdamus/Features/Spells/Views/SpellTimelineView.swiftdamus/Features/Timeline/Models/HomeModel.swiftdamus/Features/Timeline/Views/PostingTimelineView.swiftdamusTests/FeedTabStoreTests.swiftdamusTests/FilterBuilderTests.swiftdamusTests/Fixtures/kind777_spells.jsondamusTests/SpellEventTests.swiftdamusTests/SpellResolverTests.swiftdamusTests/SpellResultReferenceTests.swiftdamusTests/StarterSpellsTests.swiftdocs/NIP-A7-spells.md
| D73E5EFC2C6A97F4007EB227 /* DamusAppNotificationView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D78CD5972B8990300014D539 /* DamusAppNotificationView.swift */; }; | ||
| D73E5EFD2C6A97F4007EB227 /* InnerTimelineView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4CE0E2B529A3ED5500DB4CA2 /* InnerTimelineView.swift */; }; | ||
| D73E5EFE2C6A97F4007EB227 /* (null) in Sources */ = {isa = PBXBuildFile; }; | ||
| D73E5EFE2C6A97F4007EB227 /* BuildFile in Sources */ = {isa = PBXBuildFile; }; |
Contributor
There was a problem hiding this comment.
Invalid PBXBuildFile entry with no fileRef attribute.
This PBXBuildFile entry lacks a fileRef, making it effectively a no-op placeholder. It's referenced in the Sources build phase (line 7550) but doesn't point to any actual source file. This should either be removed entirely or corrected to reference a valid file.
🔧 Suggested fix: Remove the orphan entry
Remove line 1560:
- D73E5EFE2C6A97F4007EB227 /* BuildFile in Sources */ = {isa = PBXBuildFile; };And remove the reference in the build phase (line 7550):
- D73E5EFE2C6A97F4007EB227 /* BuildFile in Sources */,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| D73E5EFE2C6A97F4007EB227 /* BuildFile in Sources */ = {isa = PBXBuildFile; }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus.xcodeproj/project.pbxproj` at line 1560, The PBXBuildFile entry with id
D73E5EFE2C6A97F4007EB227 is orphaned (no fileRef) and must be removed; delete
the entire PBXBuildFile object for D73E5EFE2C6A97F4007EB227 from the
project.pbxproj and also remove that same id from the PBXSourcesBuildPhase
"files" array where it is referenced so the build phase no longer points to a
non-existent file; after removal scan the file for any other references to
D73E5EFE2C6A97F4007EB227 and eliminate them to keep the project consistent.
Comment on lines
81
to
82
| case .contacts, .metadata, .delete, .boost, .chat, .mute_list, .list_deprecated, .draft, .nwc_request, .nwc_response, .http_auth, .status, .relay_list, .follow_list, .interest_list, .contact_card, .live, .live_chat, .spell: | ||
| return .unknown_or_unsupported_kind |
Contributor
There was a problem hiding this comment.
kind:777 note-id references are being rejected as unsupported
Line 81 places .spell in the unsupported branch, so opening a spell via this load path returns the “Can’t display note” state instead of rendering it. This appears to conflict with the spell-support objective and breaks direct note-reference handling for kind:777.
Suggested fix
- case .text, .highlight, .longform:
+ case .text, .highlight, .longform, .spell:
return .loaded(route: Route.Thread(thread: ThreadModel(event: ev, damus_state: damus_state)))
@@
- case .contacts, .metadata, .delete, .boost, .chat, .mute_list, .list_deprecated, .draft, .nwc_request, .nwc_response, .http_auth, .status, .relay_list, .follow_list, .interest_list, .contact_card, .live, .live_chat, .spell:
+ case .contacts, .metadata, .delete, .boost, .chat, .mute_list, .list_deprecated, .draft, .nwc_request, .nwc_response, .http_auth, .status, .relay_list, .follow_list, .interest_list, .contact_card, .live, .live_chat:
return .unknown_or_unsupported_kind🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus/Features/Events/Models/LoadableNostrEventView.swift` around lines 81 -
82, The enum branch in LoadableNostrEventView incorrectly treats the .spell case
as unsupported (it is listed alongside .contacts, .metadata, etc.), causing
kind:777 note-id references to return .unknown_or_unsupported_kind; to fix,
remove .spell from this unsupported-case list or add an explicit branch handling
.spell that returns the correct renderable state (instead of
.unknown_or_unsupported_kind) so spells are processed/returned like other
supported kinds.
Comment on lines
+79
to
+84
| /// Clear cached events and reset state. | ||
| func reset() { | ||
| unsubscribe() | ||
| events = [] | ||
| state = .idle | ||
| } |
Contributor
There was a problem hiding this comment.
reset() does not clear the EventHolder's deduplication state.
The EventHolder.has_event Set persists across reset() calls because only events = [] is assigned, not the eventHolder itself. If the same spell is re-subscribed after reset, previously seen events will be silently rejected as duplicates.
🐛 Suggested fix
func reset() {
unsubscribe()
+ eventHolder = EventHolder(on_queue: { [weak self] ev in
+ guard let self else { return }
+ preload_events(state: self.damus_state, events: [ev])
+ })
events = []
state = .idle
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus/Features/Spells/Models/SpellFeedModel.swift` around lines 79 - 84, The
reset() method currently only sets events = [] and state = .idle but does not
clear the EventHolder's deduplication set, so recreate or clear the eventHolder
to reset its has_event state: inside reset() call the existing unsubscribe(),
then either set eventHolder = EventHolder() or call a clear method on
eventHolder (e.g., eventHolder.clear() or eventHolder.has_event.removeAll()) so
the deduplication Set is emptied before future subscriptions; keep the rest of
reset() behavior (events = [], state = .idle) unchanged.
Comment on lines
+121
to
+125
| case .eose: | ||
| self.flushAndMarkLoaded() | ||
| if resolved.closeOnEose { | ||
| break | ||
| } |
Contributor
There was a problem hiding this comment.
break only exits the switch, not the for await loop.
When resolved.closeOnEose is true, the break statement exits only the switch block, not the async for loop. The subscription continues iterating.
🐛 Suggested fix using labeled break
- for await item in stream {
+ streamLoop: for await item in stream {
guard !Task.isCancelled else { break }
switch item {
// ...
case .eose:
self.flushAndMarkLoaded()
if resolved.closeOnEose {
- break
+ break streamLoop
}
// ...
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus/Features/Spells/Models/SpellFeedModel.swift` around lines 121 - 125,
The switch's `case .eose` uses `break`, which only exits the switch not the
surrounding `for await` loop, so when `resolved.closeOnEose` is true the
subscription still continues; fix by adding a label to the async loop (e.g.,
`outerLoop:`) that encloses the `for await` and replace the `break` in `case
.eose` with a labeled break (e.g., `break outerLoop`) so that when
`resolved.closeOnEose` is true you exit the entire loop after calling
`self.flushAndMarkLoaded()`; locate the `for await` that contains `case .eose`,
the `self.flushAndMarkLoaded()` call, and the `resolved.closeOnEose` check to
implement this change.
Comment on lines
+46
to
+48
| if let limit = spell.limit { | ||
| SpellBadge(text: "limit \(limit)") | ||
| } |
Contributor
There was a problem hiding this comment.
Localize the "limit" badge text.
The limit badge string is not wrapped in NSLocalizedString, unlike the other badges.
🌐 Suggested fix
if let limit = spell.limit {
- SpellBadge(text: "limit \(limit)")
+ SpellBadge(text: String(format: NSLocalizedString("limit %d", comment: "Badge showing spell result limit"), limit))
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus/Features/Spells/Views/SpellCardView.swift` around lines 46 - 48, The
"limit" badge in SpellCardView.swift is not localized; update the SpellBadge
call that uses spell.limit so the label text is produced via NSLocalizedString
(e.g. a localized format like "limit %d" or "limit %@" with an appropriate
comment) and then formatted with the numeric value before passing to SpellBadge,
keeping the SpellBadge(text: ...) usage and referencing spell.limit and
SpellBadge to locate the change.
Comment on lines
+33
to
+35
| private func isAlreadyAdded(_ spell: DiscoveredSpell) -> Bool { | ||
| feedTabStore.savedFeeds.contains { $0.id == spell.noteId.hex() } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how FilterBuilderModel generates the SavedSpellFeed id
ast-grep --pattern 'func buildSavedFeed() -> $_ {
$$$
}'
# Also check if there's any content-based deduplication
rg -n 'spellEventJSON' --type swift -C3Repository: damus-io/damus
Length of output: 8977
🏁 Script executed:
#!/bin/bash
# Check the SpellDiscoveryView.swift context around lines 33-35
sed -n '20,50p' damus/Features/Spells/Views/SpellDiscoveryView.swift
# Check the full SpellDiscoveryView to understand how discovered spells are added
rg -n 'addFeed\|isAlreadyAdded' damus/Features/Spells/Views/SpellDiscoveryView.swift -B2 -A2
# Check FeedTabStore.addFeed to see if there's any deduplication there
rg -n 'func addFeed' damus/Features/Spells/Models/FeedTabStore.swift -A10Repository: damus-io/damus
Length of output: 1598
🏁 Script executed:
#!/bin/bash
# Check DiscoveredSpell definition and how noteId is set
rg -n 'struct DiscoveredSpell\|class DiscoveredSpell' damus/Features/Spells/ -A15
# Check SpellDiscoveryModel to see how spells are discovered and noteId assigned
rg -n 'func discover\|DiscoveredSpell(' damus/Features/Spells/Models/SpellDiscoveryModel.swift -B2 -A5
# Check if spellEventJSON content is compared anywhere
rg -n 'spellEventJSON.*==' damus/Features/Spells/ --type swiftRepository: damus-io/damus
Length of output: 170
🏁 Script executed:
#!/bin/bash
# Check DiscoveredSpell definition and how noteId is set
rg -n 'struct DiscoveredSpell' damus/Features/Spells/ -A15
# Check SpellDiscoveryModel to see how spells are discovered and noteId assigned
rg -n 'DiscoveredSpell\(' damus/Features/Spells/Models/SpellDiscoveryModel.swift -B2 -A5
# Check if spellEventJSON content is compared anywhere
rg -n 'spellEventJSON' damus/Features/Spells/ --type swift | grep '=='
# Look for the discover function and how it processes events
rg -n 'func discover' damus/Features/Spells/Models/SpellDiscoveryModel.swift -A20Repository: damus-io/damus
Length of output: 1638
🏁 Script executed:
#!/bin/bash
# Look for how discovered spells flow into the UI and when toSavedFeed is called
rg -n 'toSavedFeed\|DiscoveredSpell' damus/Features/Spells/Views/SpellDiscoveryView.swift -B3 -A3
# Check if there's any content comparison or hash-based comparison anywhere
rg -n 'hash\|content.*compare' damus/Features/Spells/ --type swift
# Look at where addFeed is called from SpellDiscoveryView
rg -n 'addFeed' damus/Features/Spells/Views/SpellDiscoveryView.swift -B5 -A5
# Check what happens when a user clicks "add" on a discovered spell
cat -n damus/Features/Spells/Views/SpellDiscoveryView.swift | grep -A30 'func.*add\|button.*add'Repository: damus-io/damus
Length of output: 832
🏁 Script executed:
#!/bin/bash
# Let me trace the full flow - what happens if you create a spell locally, then discover it?
# Check if there's any content-based deduplication that I might have missed
# Look for any hashing or content comparison logic
rg -n 'hash\|eventJSON.*noteId\|noteId.*eventJSON' damus/Features/Spells/ --type swift
# Check the SavedSpellFeed definition to understand the full structure
cat -n damus/Features/Spells/Models/FeedTab.swift | head -60
# See if there's any deduplication based on content elsewhere
rg -n 'contains.*spellEventJSON\|contains.*eventJSON' damus/Features/Spells/ --type swiftRepository: damus-io/damus
Length of output: 2002
🏁 Script executed:
#!/bin/bash
# Check if locally created spells are published to relays
rg -n 'publish\|relay' damus/Features/Spells/Views/FilterBuilderView.swift --type swift
# Check how SpellDiscoveryModel discovers spells - does it query relays?
rg -n 'class SpellDiscoveryModel\|subscribe\|query' damus/Features/Spells/Models/SpellDiscoveryModel.swift -A5
# Look for any scenario where locally created spells become relay spells
rg -n 'publish.*spell\|spell.*publish' damus/Features/Spells/ --type swiftRepository: damus-io/damus
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Look at the full SpellDiscoveryModel to understand how spells are discovered
cat -n damus/Features/Spells/Models/SpellDiscoveryModel.swift | head -100
# Check what events SpellDiscoveryModel receives
rg -n 'class SpellDiscoveryModel\|init\|didReceive' damus/Features/Spells/Models/SpellDiscoveryModel.swift -A10Repository: damus-io/damus
Length of output: 3340
ID-based duplicate detection allows same spell to be added with different IDs.
Locally created spells use UUID-based IDs while discovered relay spells use noteId.hex(). If a user creates a spell via FilterBuilderView and that spell is later discovered from a relay, isAlreadyAdded() won't prevent duplicate addition because it only checks event ID, not content.
Consider comparing spellEventJSON content in duplicate detection, or clarify whether this scenario is intentionally unsupported.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus/Features/Spells/Views/SpellDiscoveryView.swift` around lines 33 - 35,
The current isAlreadyAdded(_ spell: DiscoveredSpell) only compares
feedTabStore.savedFeeds by id (using spell.noteId.hex()), which misses
duplicates when a locally created spell has a different id than a discovered
relay event; update isAlreadyAdded to also detect duplicates by comparing the
spell content (e.g., DiscoveredSpell.spellEventJSON or the canonical spell
payload) against savedFeeds’ stored spell content/JSON so a matching
spellEventJSON (or other canonical content fingerprint) is treated as already
added; keep the existing id check (spell.noteId.hex()) but add a secondary
comparison against savedFeeds’ content field or a content-hash to reliably catch
duplicates created with different IDs.
Comment on lines
+26
to
+51
| static func extract(from event: NostrEvent) -> SpellResultReference { | ||
| switch event.known_kind { | ||
| case .like: | ||
| guard let refId = event.referenced_ids.last else { | ||
| return .directEvent | ||
| } | ||
| let emoji = to_reaction_emoji(ev: event) ?? "❤️" | ||
| return .reaction(referencedNoteId: refId, reactorPubkey: event.pubkey, emoji: emoji) | ||
|
|
||
| case .zap: | ||
| guard let refId = event.referenced_ids.first else { | ||
| return .directEvent | ||
| } | ||
| let senderPubkey: Pubkey | ||
| if let zapRequest = get_zap_request(event) { | ||
| senderPubkey = zapRequest.pubkey | ||
| } else { | ||
| senderPubkey = event.pubkey | ||
| } | ||
| let amountMsats = extractZapAmount(from: event) | ||
| return .zap(referencedNoteId: refId, senderPubkey: senderPubkey, amountMsats: amountMsats) | ||
|
|
||
| default: | ||
| return .directEvent | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for how other parts of the codebase handle reaction and zap referenced IDs
rg -n -C3 'referenced_ids\.(first|last)' --type=swift | head -80Repository: damus-io/damus
Length of output: 5436
Change reactions to use .first instead of .last for referenced_ids.
The reaction case uses referenced_ids.last (line 29) while zaps use referenced_ids.first (line 36). However, the rest of the codebase consistently uses .first: SpellTimelineView.swift handles both reactions and zaps with .first (line 73), Zap.swift uses .first throughout for zap references, and NostrEvent.swift uses .first for reply gathering. The inconsistent use of .last for reactions only appears to be unintentional and should be aligned with .first like zaps.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus/Features/Spells/Views/SpellResultView.swift` around lines 26 - 51, The
reaction branch in SpellResultView.extract incorrectly uses
event.referenced_ids.last (in the .like case) which is inconsistent with the
rest of the codebase; change it to use event.referenced_ids.first so the .like
case returns .reaction(referencedNoteId: refId, reactorPubkey: event.pubkey,
emoji: emoji) the same way .zap uses .first; update the guard to let refId =
event.referenced_ids.first and keep the rest of the logic (emoji via
to_reaction_emoji, returning .reaction) unchanged.
Comment on lines
+291
to
+292
| case .spell: | ||
| break |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if SpellFeedModel or preload_events inserts events into damus.events cache
echo "=== Checking SpellFeedModel event handling ==="
rg -n -A5 'func handleEvent' --glob '**/SpellFeedModel.swift'
echo ""
echo "=== Checking preload_events implementation ==="
ast-grep --pattern 'func preload_events($$$) {
$$$
}'
echo ""
echo "=== Checking if preload_events calls events.insert ==="
rg -n 'events\.insert' --glob '**/*.swift' | head -20Repository: damus-io/damus
Length of output: 3925
🏁 Script executed:
#!/bin/bash
# Get full preload_event implementation
echo "=== preload_event implementation ==="
rg -n -A20 'func preload_event' damus/Shared/Utilities/EventCache.swift | head -50
echo ""
echo "=== Check if EventCache has insert method ==="
rg -n 'func insert' damus/Shared/Utilities/EventCache.swift | head -10
echo ""
echo "=== Check SpellFeedModel for damus_state.events usage ==="
rg -n 'damus_state\.events' damus/Features/Spells/Models/SpellFeedModel.swift
echo ""
echo "=== Check EventCache.swift for all insert calls ==="
rg -n '\.insert\(' damus/Shared/Utilities/EventCache.swiftRepository: damus-io/damus
Length of output: 1878
🏁 Script executed:
#!/bin/bash
# Get full EventCache.insert implementation
echo "=== EventCache.insert method ==="
rg -n -A10 'func insert\(_ ev: NostrEvent\)' damus/Shared/Utilities/EventCache.swift
echo ""
echo "=== SpellTimelineView.resolvedTapTarget ==="
rg -n -B2 -A15 'resolvedTapTarget' damus/Features/Spells/Views/SpellTimelineView.swiftRepository: damus-io/damus
Length of output: 2108
🏁 Script executed:
#!/bin/bash
# Check if there are any callbacks from SpellFeedModel that insert into damus_state.events
echo "=== Search for any insert paths from spell subscriptions ==="
rg -n 'damus.*\.events\.insert|state\.events\.insert' damus/Features/Spells/ --glob '**/*.swift'
echo ""
echo "=== Check what handleEvent does in detail ==="
cat -n damus/Features/Spells/Models/SpellFeedModel.swift | sed -n '130,145p'
echo ""
echo "=== Check if SpellFeedModel initializes any callbacks that link to main cache ==="
rg -n -B5 -A5 'eventHolder.insert' damus/Features/Spells/Models/SpellFeedModel.swift | head -30Repository: damus-io/damus
Length of output: 1076
Spell events correctly skipped, but spell feed events should be cached in damus_state.events.
The break for .spell is appropriate since spell events are feed definitions, not displayable content. However, SpellFeedModel.handleEvent only inserts events into its local eventHolder, not into damus_state.events. This causes a problem: when users tap reactions or zaps from spell feeds, SpellTimelineView.resolvedTapTarget cannot find the referenced original events in the damus.events cache and falls back to navigating to the reaction itself rather than the original post.
To fix this, ensure events returned by spell feed subscriptions are also inserted into damus_state.events so that tap navigation resolves correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus/Features/Timeline/Models/HomeModel.swift` around lines 291 - 292, The
.spell branch is currently ignored but SpellFeedModel.handleEvent only adds
events to its local eventHolder, causing missing entries in the global cache;
update SpellFeedModel.handleEvent (the code that processes .spell feed events)
to also insert the returned events into damus_state.events in addition to
eventHolder so SpellTimelineView.resolvedTapTarget can find original posts;
ensure you add the same Event objects/IDs used in eventHolder into the shared
damus_state.events cache (preserving keys/IDs) and keep existing behavior for
display filtering.
Collaborator
Author
|
@jb55 mentions that nfilter would be a great way to handle things on damus side, maintaing spells as an interoperability layer between damus and other nostr apps |
alltheseas
force-pushed
the
spells/kind-777-nip-a7
branch
from
0b0a8eb to
a3a7caa
Compare
alltheseas
changed the title
Collaborator
Author
Register kind:777 as NostrKind.spell for NIP-A7 spell events and kind:20 as NostrKind.picture for NIP-68 picture events. Handle both in the exhaustive switches in HomeModel and LoadableNostrEventView. Route spell and picture note references through the thread view alongside text, highlight, and longform events. Changelog-Added: Add kind:777 spell and kind:20 picture event support Signed-off-by: alltheseas <[email protected]>
SpellEvent parses kind:777 Nostr events into structured filter definitions. Supports kinds, authors, ids, tag filters, timestamps (absolute/relative), search, relay hints, and metadata tags. Includes SpellVariable ($me, $contacts) and SpellTimestamp types for runtime resolution of user-relative values. 25 unit tests covering minimal/full parsing, edge cases, and real-world fixtures from 6 kind:777 events. Changelog-Added: Add NIP-A7 spell event parser for kind:777 Signed-off-by: alltheseas <[email protected]>
SpellResolver converts a parsed SpellEvent into concrete filters by resolving runtime variables ($me -> user pubkey, $contacts -> friend list) and relative timestamps (-24h, -7d) to absolute Unix times. Eagerly converts NostrFilter to NdbFilter during resolve(), surfacing conversion errors early. ResolvedSpell carries ndbFilters as the primary query type alongside NostrFilter for relay JSON serialization. 12 unit tests covering variable resolution, timestamp types, contacts expansion, NdbFilter production, and combined filter generation. Changelog-Added: Add spell resolver with NdbFilter query foundation Signed-off-by: alltheseas <[email protected]>
SpellFeedModel manages relay subscriptions for spell feeds using advancedStream, collecting events into a published array. Uses a labeled loop for correct closeOnEose break semantics, and resets EventHolder deduplication state on reset(). Queries local nostrdb directly with NdbFilter for instant results before starting relay streaming, following the same pattern as HomeModel. Display views: - SpellFeedView: loading/empty/error states with pull-to-refresh - SpellTimelineView: scrollable list of spell results - SpellCardView: kind:777 event card rendered in EventView - SpellResultView: individual result event rendering Integrates SpellCardView into EventView for rendering spell events in timelines. 11 tests for referenced event resolution. Changelog-Added: Add spell feed subscription and display views Signed-off-by: alltheseas <[email protected]>
FeedTab enum models .following and .spell(SavedSpellFeed) tabs. FeedTabStore persists saved feeds and selection to UserDefaults with add, remove, move (reorder), rename, and starter seed support. FeedTabBarView provides a horizontal scrollable tab bar with animated underline indicator, spring animations, and a [+] button for adding feeds. Follows 44pt minimum touch targets. 19 unit tests covering persistence round-trip, reorder, rename, starter seeding, and FeedTab model. Changelog-Added: Add multi-feed tab bar with UserDefaults persistence Signed-off-by: alltheseas <[email protected]>
StarterSpells provides hardcoded starter feeds seeded into FeedTabStore on first launch via signed kind:777 events: - Global notes: kind:1 from the last 24 hours - Images: kind:20 (NIP-68 picture events) from the last 24 hours SpellDiscoveryModel fetches kind:777 events from connected relays using advancedStream for browsing community-published feeds. SpellDiscoveryView presents a searchable sheet for discovering and adding feeds, with loading/empty states and add/checkmark buttons. 10 unit tests covering starter feed parsing, seeding integration, and DiscoveredSpell model. Changelog-Added: Add starter spell feeds and relay-based feed discovery Signed-off-by: alltheseas <[email protected]>
FilterBuilderModel collects user selections (kinds, author scope, time range, search, hashtags) and synthesizes NIP-A7 tags for a kind:777 spell event. Builds and signs a SavedSpellFeed on save. FilterBuilderView provides a Form-based UI with kind toggle presets, author scope picker, time range picker, search/hashtag fields, and a live tag preview section. Accessible via + button in discovery. 27 unit tests covering validation, tag building, custom kinds, author scopes, round-trip parsing, and preset data. Changelog-Added: Add visual filter builder for creating custom feeds Signed-off-by: alltheseas <[email protected]>
Wire all spell components into PostingTimelineView: - FeedTabBarView in header (conditional on tabs > 1) - Tab switching between Following and spell feeds - SpellFeedView for active spell feed display - Discovery sheet from [+] button - Onboarding sheet on first encounter FeedManagementView adds drag-to-reorder, inline rename, and swipe-to-delete for saved feeds. SpellOnboardingView provides a brief education sheet shown once on first launch. Starter feeds are seeded on appear via StarterSpells. Changelog-Added: Add feed management, onboarding, and timeline integration Signed-off-by: alltheseas <[email protected]>
alltheseas
force-pushed
the
spells/kind-777-nip-a7
branch
from
9800d3a to
44d623b
Compare
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 free
to 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
Adds full kind:777 "spell" support per NIP-A7, enabling portable custom feeds defined as Nostr events. Each spell encodes a REQ filter (kinds, authors, time range, search, hashtags) resolved at runtime with variables (
$me,$contacts) and relative timestamps (-24h,-7d). Integrates a multi-feed tab bar into the home timeline with starter feeds seeded on first launch.Uses NdbFilter as the primary query foundation — SpellResolver eagerly converts to NdbFilter at resolution time, and SpellFeedModel queries the local nostrdb directly for instant results before supplementing with relay streaming. Images feed uses kind:20 (NIP-68 picture events).
Commits (review commit-by-commit)
ae590f24— core: Add NostrKind.spell (kind:777) and NostrKind.picture (kind:20) (6 impl, 0 test)ac87b05e— feat: Add NIP-A7 spell event parser (453 impl, 449 test)17abe2af— feat: Add spell resolver with NdbFilter as primary query type (176 impl, 270 test)bd161ad6— feat: Add spell feed display views and subscription model (646 impl, 214 test)36e7552b— feat: Add feed tab system with persistence (271 impl, 226 test)e457afbc— feat: Add starter spells and feed discovery (344 impl, 134 test)a987fc45— feat: Add visual filter builder for custom feeds (363 impl, 246 test)44d623bd— feat: Integrate spell feeds into home timeline (312 impl, 0 test)Total: 2571 impl, 1539 test
Standard PR Checklist
Closes:orFixes:tags in the commit messages wherever applicable, or made sure those are not needed. See Submitting patchesTest report
Device: iPhone 17 Pro Simulator
iOS: 26.2
Damus:
44d623bd(spells/kind-777-nip-a7)Setup: 104 unit tests across 6 test suites
Steps:
xcodebuild test -only-testing:damusTests/{SpellEventTests,SpellResolverTests,SpellResultReferenceTests,FeedTabStoreTests,StarterSpellsTests,FilterBuilderTests}Results:
Other notes
docs/NIP-A7-spells.mdincluded as spec reference🤖 Generated with Claude Code