Skip to content

Replace Tenor with Nostr-native GIF picker + Blossom uploads#3704

Draft
alltheseas wants to merge 9 commits intodamus-io:masterfrom
alltheseas:gif-keyboard
Draft

Replace Tenor with Nostr-native GIF picker + Blossom uploads#3704
alltheseas wants to merge 9 commits intodamus-io:masterfrom
alltheseas:gif-keyboard

Conversation

@alltheseas
Copy link
Copy Markdown
Collaborator

@alltheseas alltheseas commented Mar 19, 2026

Summary

  • Remove Tenor dependency entirely (API dies June 2026) — replaced with protocol-native GIF discovery via kind 1063 file metadata events and kind:1 note scouring
  • Add Blossom (NIP-B7) upload support as an app-wide media uploader, not just post composer — profile pictures, banners, and onboarding all route through Blossom when selected
  • Add NIP-94 FileMetadata model for parsing kind 1063 events
  • Add Web-of-Trust filter to GIF picker (reuses existing FriendFilter + TrustedNetworkButton pattern)
  • Bootstrap GIF catalog (163 curated GIFs) for instant results while relay queries run
  • NSFW filtering on both kind 1063 and kind:1 scour paths via content-warning/hashtag detection

Commits (9)

  1. Add Blossom types, auth, and uploader — BlossomTypes, BlossomAuth (kind 24242), BlossomUploader (raw binary PUT with SHA-256 validation)
  2. Add Blossom server settings and upload integration — Settings UI, routing in PostView + EditPictureControl, regression test for Blossom routing
  3. Add NIP-94 FileMetadata model (kind 1063) — Decoder + 12 tests
  4. Add GIF relay discovery via kind 1063 query — GIFSearchModel streaming from relays
  5. Add kind:1 GIF link scour for bootstrap catalog — Extracts .gif URLs from text notes
  6. Remove Tenor dependency, add NSFW filtering — Deletes TenorAPIClient/TenorModels, adds content-warning checks on both discovery paths
  7. Add Nostr-native GIF picker UI replacing Tenor — Rewritten GIFPickerView with Kingfisher grid
  8. Add bootstrap GIF catalog from relay scour (163 GIFs) — Bundled JSON for instant display
  9. Add Web-of-Trust filter to GIF picker — FriendFilter toggle, bootstrap GIFs (nil pubkey) always pass through

Test plan

  • BlossomTests: 23 tests pass (auth, upload, types, hash validation)
  • FileMetadataTests: 12 tests pass (parsing, edge cases)
  • EditPictureControlTests: 7 tests pass (includes Blossom routing regression test)
  • Manual: open GIF picker, verify GIFs load, search works, WoT toggle filters
  • Manual: set default uploader to Blossom, upload profile picture and banner
  • Manual: verify NSFW-tagged GIFs are hidden when setting is enabled

Status: UNTESTED on device — automated unit tests pass, manual verification pending.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added Blossom server support for media uploads with configurable server URLs
    • Added decentralized GIF discovery from Nostr relays with NIP-94 metadata support
    • Added Media Servers settings to configure uploading service
    • Added friend-based GIF filtering in the GIF picker
    • Added NSFW filtering for discovered GIFs
  • Refactor

    • Replaced Tenor API integration with Nostr relay-based GIF discovery
  • Tests

    • Added Blossom and NIP-94 FileMetadata test coverage

Blossom (NIP-B7) foundation for decentralized media uploads,
replacing the deprecated NIP-96 approach.

- BlossomTypes: server URL (HTTPS-only), blob descriptor, errors
- BlossomAuth: kind 24242 auth events (BUD-11)
- BlossomUploader: PUT /upload with streaming SHA256 (BUD-02)
- NostrKind: add file_metadata (1063), blossom_server_list (10063),
  blossom_auth (24242)
- Handle new kinds in HomeModel and LoadableNostrEventView switches
- 23 tests covering URL validation, SHA256, auth, blob decoding

Closes: damus-v3d

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Signed-off-by: alltheseas <[email protected]>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 19, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0b897b42-ca39-4af2-8429-3ad7d1987261

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR introduces Blossom file hosting integration with NIP-94 metadata support, replacing Tenor-based GIF discovery with relay-sourced discovery. It adds Blossom uploader, authentication, server settings UI, and integrates file uploads across profile pictures, banners, and GIF selection while removing Tenor API dependencies.

Changes

Cohort / File(s) Summary
Blossom Foundation
damus/Core/Blossom/BlossomTypes.swift, damus/Core/Blossom/BlossomAuth.swift, damus/Core/Blossom/BlossomUploader.swift
Added Blossom server URL validation, descriptor models, authorization header generation, and SHA-256 based upload verification with error handling.
NIP-94 File Metadata
damus/Core/NIPs/NIP94/FileMetadata.swift
Implemented NIP-94 kind 1063 file metadata struct with tag encoding/decoding and integration helper for Blossom descriptors.
Nostr Event Kind Extensions
damus/Core/Nostr/NostrKind.swift, damus/Core/Nostr/NostrFilter.swift
Added kinds for file metadata (1063), blossom auth (24242), blossom server list (10063); extended filter with MIME type support (#m tag).
GIF Discovery Pipeline
damus/Shared/Media/GIF/GIFSearchModel.swift, damus/Shared/Media/GIF/GIFPickerView.swift, damus/Shared/Media/GIF/bootstrap_gifs.json
Replaced Tenor-based GIF picker with Nostr relay discovery for kind 1063 file metadata and kind 1 URLs; added NSFW filtering, deduplication, and friend filtering.
GIF Removal
damus/Shared/Media/GIF/TenorAPIClient.swift, damus/Shared/Media/GIF/TenorModels.swift
Removed Tenor API client and data models as GIF discovery is now Nostr-based.
Media Upload Integration
damus/Shared/Media/Models/ImageUploadModel.swift, damus/Shared/Media/Models/MediaUploader.swift
Extended upload protocol with Blossom path; added blossom uploader case with server URL support and isBlossom discriminator.
Settings & Server Configuration
damus/Features/Settings/Models/UserSettingsStore.swift, damus/Features/Settings/Views/MediaServersSettingsView.swift, damus/Features/Settings/Views/ConfigView.swift
Added blossom server URL setting (default: https://blossom.nostr.build); removed tenor API key; added media servers settings UI with URL validation.
Settings UI Cleanup
damus/Features/Settings/Views/AppearanceSettingsView.swift
Removed Tenor API key input field from GIFs settings section.
Picture/Banner Upload Wiring
damus/Features/Profile/Views/EditPictureControl.swift, damus/Features/Profile/Views/ProfilePictureSelector.swift, damus/Shared/Media/Images/BannerImageView.swift
Extended picture/banner controls with blossom server URL parameter and conditional Blossom upload routing.
Post View Integration
damus/Features/Posting/Views/PostView.swift
Added Blossom upload conditional branch when uploader is Blossom.
Event Kind Handling
damus/Features/Events/Models/LoadableNostrEventView.swift, damus/Features/Timeline/Models/HomeModel.swift
Added no-op handling for file metadata, blossom auth, and blossom server list kinds.
Navigation Routing
damus/Shared/Utilities/Router.swift
Added MediaServersSettings route and view mapping.
Test Coverage
damusTests/BlossomTests.swift, damusTests/FileMetadataTests.swift, damusTests/EditPictureControlTests.swift, .beads/issues.jsonl
Added comprehensive tests for Blossom auth/URL parsing, NIP-94 metadata encoding/decoding, file metadata round-tripping, Blossom upload flow, and issue tracking entries.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant App as Damus App
    participant Relay as Nostr Relay
    participant Server as Blossom Server
    
    User->>App: Tap upload picture
    App->>App: Generate Blossom auth<br/>(keypair, action, expiration)
    App->>Server: PUT /upload<br/>(auth header, binary data)
    Server->>Server: Verify auth<br/>Compute SHA-256
    Server-->>App: BlossomBlobDescriptor<br/>(url, sha256, size)
    App->>App: Verify hash match
    App->>App: Create FileMetadata<br/>(NIP-94 kind 1063)
    App->>Relay: Publish FileMetadata event
    App->>User: Success
Loading
sequenceDiagram
    actor User
    participant App as Damus App
    participant Cache as Local Cache
    participant Relay as Nostr Relay
    
    User->>App: Open GIF picker
    App->>Cache: Load bootstrap GIFs
    App->>App: Display bootstrap GIFs
    App->>Relay: Subscribe kind 1063<br/>(MIME: image/gif)
    Relay-->>App: File metadata events
    App->>App: Deduplicate by SHA-256
    App->>App: Filter NSFW<br/>(content-warning tags)
    App->>App: Filter by friend network<br/>(optional)
    App-->>User: Display discovered GIFs
    User->>App: Search query
    App->>Relay: Subscribe kind 1063<br/>(hashtag filter)
    Relay-->>App: Filtered results
    App-->>User: Display matches
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

gif, technical, pr-active-review

Suggested reviewers

  • danieldaquino
  • jb55

Poem

🐰 A Blossom blooms in Nostr's light,
Files hosted, hashes verified right,
GIFs discovered through relay streams,
No Tenor needed—just decentralized dreams!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 37.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The PR description provides a comprehensive summary of changes, lists all 9 commits, includes a test plan with specific test counts, and notes the current testing status. However, the description does not follow the provided template structure (no explicit Summary section header, no Standard/Experimental checklist, no Test Report section with Device/iOS/Setup details, no explicit sign-off confirmation, and no changelog entry reference). Consider restructuring the description to follow the template: add explicit section headers, complete the Standard PR Checklist, provide device/iOS testing details in a Test Report section, and confirm changelog entries have been added.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title concisely summarizes the two primary changes: removing Tenor and replacing it with a Nostr-native GIF picker, plus adding Blossom upload support.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.

Add a .trivyignore file to your project to customize which findings Trivy reports.

@alltheseas
Copy link
Copy Markdown
Collaborator Author

@coderabbitai review please

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 19, 2026

@alltheseas Sure, I'll review the changes in this PR now!


🧠 Learnings used
Learnt from: CR
Repo: damus-io/damus PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:12:51.605Z
Learning: Code should be human reviewable
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
damus/Features/Posting/Views/PostView.swift (1)

465-478: ⚠️ Potential issue | 🟠 Major

Move preview generation off the main actor.

This branch still calls getImage(media:) before the first await. That helper does synchronous file reads and video thumbnail generation, so triggering uploads from the composer UI will still hitch on large assets even though the network upload is async. Please push that preprocessing to a background queue and hop back only for state updates.

As per coding guidelines, "Never block the main thread: All network requests, database queries, and expensive computations must run on background threads/queues using Task { }, DispatchQueue.global(), or Swift concurrency (async/await). UI updates must dispatch back to @MainActor".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@damus/Features/Posting/Views/PostView.swift` around lines 465 - 478, The
handle_upload(media: MediaUpload) function is calling getImage(media:) (which
does synchronous file I/O and video thumbnail work) before any await, blocking
the main actor; move the image/preview work to a background context (e.g.,
Task.detached or DispatchQueue.global()) so getImage(media:) and
calculate_blurhash(img:) run off the main actor, then hop back to `@MainActor`
only to assign UI state such as mediaUploadUnderProgress and to start any
UI-triggered uploads; specifically, run getImage(media:) and blurhash
calculation off the main actor, capture their results, then on MainActor
continue the existing uploader selection and calls to
image_upload.start/startBlossomUpload.
♻️ Duplicate comments (1)
damus.xcodeproj/project.pbxproj (1)

7419-7419: ⚠️ Potential issue | 🔴 Critical

Duplicate malformed BuildFile entry.

This is the same malformed entry as Line 1526, appearing in a different build target. Both instances need to be corrected.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@damus.xcodeproj/project.pbxproj` at line 7419, There is a duplicate malformed
BuildFile reference with ID D73E5EFE2C6A97F4007EB227 appearing twice (once at
the original location and again in another target); locate both occurrences of
the BuildFile entry with ID D73E5EFE2C6A97F4007EB227 in the project.pbxproj and
remove or replace the second/duplicate occurrence so each build target
references a valid BuildFile object (either remove the duplicate from the
target's sources/build phase or point it to the correct BuildFile entry used at
the original location). Ensure the target's PBXBuildFile list and the
corresponding PBXSourcesBuildPhase no longer contain the malformed ID to prevent
project corruption.
🧹 Nitpick comments (7)
damus/Core/Nostr/NostrFilter.swift (1)

39-54: mime_types not included in initializer or copy(from:).

The new mime_types property is not included in the init parameters (line 39) nor copied in copy(from:) (line 53). This means:

  1. Callers must set mime_types after construction rather than at init time
  2. Copying a filter with mime_types set will lose that value

This appears intentional given other properties like parameter follow the same pattern, but worth confirming this is acceptable for your GIF discovery use case.

♻️ Optional: Add mime_types to init and copy
-    init(ids: [NoteId]? = nil, kinds: [NostrKind]? = nil, referenced_ids: [NoteId]? = nil, pubkeys: [Pubkey]? = nil, since: UInt32? = nil, until: UInt32? = nil, limit: UInt32? = nil, authors: [Pubkey]? = nil, hashtag: [String]? = nil, quotes: [NoteId]? = nil) {
+    init(ids: [NoteId]? = nil, kinds: [NostrKind]? = nil, referenced_ids: [NoteId]? = nil, pubkeys: [Pubkey]? = nil, since: UInt32? = nil, until: UInt32? = nil, limit: UInt32? = nil, authors: [Pubkey]? = nil, hashtag: [String]? = nil, quotes: [NoteId]? = nil, mime_types: [String]? = nil) {
         self.ids = ids
         self.kinds = kinds
         self.referenced_ids = referenced_ids
         self.pubkeys = pubkeys
         self.since = since
         self.until = until
         self.limit = limit
         self.authors = authors
         self.hashtag = hashtag
         self.quotes = quotes
+        self.mime_types = mime_types
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@damus/Core/Nostr/NostrFilter.swift` around lines 39 - 54, The initializer
NostrFilter.init(...) and the static copy(from:) currently omit the mime_types
property, causing callers to set it post-construction and causing copy(from:) to
strip mime_types; update the init signature for NostrFilter to accept
mime_types: [String]? (or the appropriate type) and assign it to
self.mime_types, and update public static func copy(from:) to include
mime_types: from.mime_types when creating the new NostrFilter so copies preserve
that field.
damus/Shared/Media/GIF/bootstrap_gifs.json (1)

1-165: External URL availability risk.

The bootstrap catalog hardcodes 164 URLs across multiple third-party servers (blossom.primal.net, nostr.build, tenor.com, giphy.com, etc.). If any servers become unavailable or URLs become stale, users will see broken thumbnails.

Consider:

  • Periodic validation of URL availability in CI
  • Graceful handling of 404s in the GIF picker (already may exist in image loading)

This is acceptable for initial release given the bootstrap is supplemented by relay discovery.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@damus/Shared/Media/GIF/bootstrap_gifs.json` around lines 1 - 165, The
bootstrap_gifs.json hardcodes many external GIF URLs which can break if upstream
hosts are unavailable; add a CI validation step that fetches each URL in
damus/Shared/Media/GIF/bootstrap_gifs.json and fails (or reports) on non-2xx
responses, and update the GIF picker image-loading logic to gracefully handle
404/timeout by falling back to a local placeholder or trying the next URL in the
list; implement the CI check as a script (e.g., validate_bootstrap_gifs) and
wire it into the repository's CI pipeline, and ensure the GIF picker’s loader
catches fetch/load errors and uses the fallback behavior.
damusTests/EditPictureControlTests.swift (1)

273-302: Assert the forwarded Blossom server URL too.

This only proves the Blossom branch was chosen. Because the mock ignores serverURL, a regression that passes nil or the wrong setting would still go green. Capture the received URL in MockImageUploadModel and assert it matches the configured Blossom server.

🧪 Suggested test hardening
     class MockImageUploadModel: ImageUploadModelProtocol {
         static var lastStartCalled = false
         static var lastBlossomCalled = false
+        static var lastBlossomServerURL: String?
 
         static func resetFlags() {
             lastStartCalled = false
             lastBlossomCalled = false
+            lastBlossomServerURL = nil
         }
 
         required init() {}
 
         func start(media: damus.MediaUpload, uploader: any damus.MediaUploaderProtocol, mediaType: damus.ImageUploadMediaType, keypair: damus.Keypair?) async -> damus.ImageUploadResult {
             MockImageUploadModel.lastStartCalled = true
             return damus.ImageUploadResult.success(get_test_uploaded_url())
         }
 
         func startBlossomUpload(media: damus.MediaUpload, keypair: damus.Keypair?, serverURL: String?) async -> damus.ImageUploadResult {
             MockImageUploadModel.lastBlossomCalled = true
+            MockImageUploadModel.lastBlossomServerURL = serverURL
             return damus.ImageUploadResult.success(get_test_uploaded_url())
         }
     }
         XCTAssertTrue(MockImageUploadModel.lastBlossomCalled, "Blossom uploader should route to startBlossomUpload")
         XCTAssertFalse(MockImageUploadModel.lastStartCalled, "Blossom uploader should not call the NIP-96 start path")
+        XCTAssertEqual(MockImageUploadModel.lastBlossomServerURL, "https://blossom.example.com")

As per coding guidelines, "Add or update unit tests in damusTests/ alongside feature changes, especially when touching parsing, storage, or replay logic".

Also applies to: 395-413

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@damusTests/EditPictureControlTests.swift` around lines 273 - 302, Update the
test to assert the Blossom server URL is forwarded: enhance MockImageUploadModel
to record the received serverURL (e.g. add a lastServerURL or similar) and then,
in testBlossomUploaderRoutesToBlossomPath, after triggering the upload and
awaiting the expectation, add an XCTAssertEqual comparing
MockImageUploadModel.lastServerURL to the configured blossom_server_url
("https://blossom.example.com") to ensure the ViewModel passed the correct URL
to the Blossom branch (keep references to MockImageUploadModel,
MockBlossomUploader,
view_model.request_upload_authorization/.confirm_upload_authorization and
blossom_server_url).
damus/Shared/Media/Models/ImageUploadModel.swift (1)

80-85: Document the new Blossom upload contract.

This adds a protocol requirement and a second upload path with different preconditions and error cases, but neither declaration is documented.

As per coding guidelines, "Ensure docstring coverage for any code added or modified".

Also applies to: 118-146

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@damus/Shared/Media/Models/ImageUploadModel.swift` around lines 80 - 85, Add
docstrings to the new protocol and its methods: document
ImageUploadModelProtocol with a short description of its responsibility, and add
clear documentation for start(media:uploader:mediaType:keypair:) and
startBlossomUpload(media:keypair:serverURL:) that explains purpose, parameters,
return value (ImageUploadResult), and the preconditions and error cases specific
to the Blossom path (e.g., serverURL required, different auth semantics). Ensure
the docs cover both upload flows and any differences in behavior referenced in
the implementation (also update the related methods at the other modified range
around startLine 118-146) so the protocol and both methods meet the project's
docstring guidelines.
damus/Features/Settings/Views/MediaServersSettingsView.swift (1)

10-56: Consider adding a docstring for the view.

Per coding guidelines, docstring coverage is expected for added code. A brief description of the view's purpose would improve maintainability.

📝 Suggested docstring
+/// Settings view for configuring media upload services and Blossom server URLs.
 struct MediaServersSettingsView: View {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@damus/Features/Settings/Views/MediaServersSettingsView.swift` around lines 10
- 56, Add a brief docstring for the MediaServersSettingsView struct describing
its purpose and what it manages (e.g., presents media uploader selection and
Blossom server configuration), placing it immediately above the declaration for
MediaServersSettingsView; ensure it mentions any important observed state
(settings: UserSettingsStore) and behavior (dismissing on .switched_timeline) so
readers understand init, body, and the view’s responsibilities.
damus/Core/Blossom/BlossomUploader.swift (1)

51-58: Consider adding a request timeout.

The URLRequest doesn't specify a timeout. For large file uploads, this could lead to indefinite hangs if the server becomes unresponsive. Consider setting timeoutInterval for reliability.

⏱️ Proposed fix
         var request = URLRequest(url: server.uploadURL)
         request.httpMethod = "PUT"
         request.httpBody = fileData
+        request.timeoutInterval = 120 // 2 minutes for upload
         request.setValue(auth, forHTTPHeaderField: "Authorization")
         request.setValue(media.mime_type, forHTTPHeaderField: "Content-Type")
         request.setValue(String(fileData.count), forHTTPHeaderField: "Content-Length")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@damus/Core/Blossom/BlossomUploader.swift` around lines 51 - 58, The
URLRequest used for uploads in BlossomUploader (the `request` created for
`server.uploadURL` before calling `URLSession.shared.data(for:delegate:)`) lacks
a timeout; set `request.timeoutInterval` to a sensible default (or read a
configurable timeout passed to the BlossomUploader/initializer) right after
constructing the URLRequest so large uploads won't hang indefinitely, and ensure
any callers or tests that expect different timeouts can override this
configuration.
damus/Shared/Media/GIF/GIFSearchModel.swift (1)

20-36: Fill in docstrings for the remaining new methods.

The type docs are there, but both DiscoveredGIF initializers, init(damus_state:), cancel(), and the private bootstrap/stream/extraction helpers are still undocumented. Please document those too so the discovery and cancellation flow is self-describing.

As per coding guidelines, "Ensure docstring coverage for any code added or modified".

Also applies to: 51-53, 100-120, 124-232

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@damus/Shared/Media/GIF/GIFSearchModel.swift` around lines 20 - 36, Add
missing docstrings to fully document the discovery and cancellation flow:
document both DiscoveredGIF initializers (the initializer taking
eventID:url:thumbURL:dim:alt:pubkey: and the init(url:) convenience
initializer), the init(damus_state:) initializer, the cancel() method, and each
private bootstrap/stream/extraction helper function used in GIF discovery; for
each, describe purpose, parameters, return behavior, side effects (e.g.,
network/stream usage or cancellation), and any threading/ownership expectations
so callers understand how discovery starts, yields results, and is cancelled.
Ensure the docs reference the exact symbol names
(DiscoveredGIF.init(eventID:url:thumbURL:dim:alt:pubkey:),
DiscoveredGIF.init(url:), init(damus_state:), cancel(), and the private
bootstrap/stream/extraction helper functions) and follow existing docstring
style and formatting used in the file.
🤖 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 1526: The PBXBuildFile entry with UUID D73E5EFE2C6A97F4007EB227 is
malformed because it lacks the required fileRef property; remove this
PBXBuildFile object from the project.pbxproj (or add a correct fileRef pointing
to the intended PBXFileReference UUID) and also remove its occurrence from the
Sources build phase list where it is referenced so the build phase no longer
points to D73E5EFE2C6A97F4007EB227; ensure any replacement fileRef uses an
existing PBXFileReference UUID and keep the PBXBuildFile syntax consistent with
other entries (isa = PBXBuildFile; fileRef = <UUID>;) if you choose to correct
instead of delete.

In `@damus/Features/Settings/Views/MediaServersSettingsView.swift`:
- Line 13: Remove the unused `@State` property by deleting the declaration of
editingServerURL in MediaServersSettingsView (the line: "@State private var
editingServerURL: String = \"\""); ensure there are no remaining references to
editingServerURL elsewhere in the view or related methods, and run a build or
grep to confirm it wasn't intended for any UI binding before committing.

In `@damus/Shared/Media/GIF/bootstrap_gifs.json`:
- Line 35: The JSON bootstrap list contains an entry with a WebP URL
("https://files.sovbit.host/.../ccfbfc0b.webp") in the GIF catalog; either
remove or replace that URL with a proper .gif file, or update the GIF-loading
pipeline (the GIF picker/display code that consumes bootstrap_gifs.json) to
explicitly accept/handle animated WebP (add MIME/type checks, fallback to a
converted GIF or a safe placeholder, and update any validators that assume
".gif" extensions) so the consumer code no longer breaks when encountering the
.webp entry.

In `@damus/Shared/Media/GIF/GIFSearchModel.swift`:
- Around line 57-74: The load() method spawns two independent Tasks (searchTask
and scourTask) that race and cause loading to be cleared too early and allow
stale results after cancel(); replace this with a single supervising Task (or
add a request token) that launches/awaits both discovery paths
(queryFileMetadata(...) and scourKind1ForGIFs(...)) so loading is set to true at
start and only set to false when the supervised work completes; also ensure
cancel() increments/invalidates the request token and every point that mutates
seenURLs/gifs or updates UI checks the current token (or Task.isCancelled) and
drops results from superseded requests (matching search()’s approach) so queued
MainActor handlers cannot leak stale data.
- Around line 41-42: GIFSearchModel is `@MainActor` so its async methods
(queryFileMetadata, streamFilter, scourKind1ForGIFs and similar blocks at 65–73,
86–97, 124–232) run on the main thread and block UI; mark those async methods as
nonisolated so relay stream iteration and per-event parsing (for await loops,
decode_file_metadata, parse_post_blocks) run off the main actor, and wrap only
the state updates to published properties (gifs, loading) in MainActor.run or
Task { `@MainActor` in ... } to switch back to the main thread when mutating UI
state. Ensure each method signature is updated to include nonisolated and that
any accesses to actor-isolated properties are performed inside a MainActor hop.

In `@damus/Shared/Media/Models/ImageUploadModel.swift`:
- Around line 126-145: In the BlossomUploader.upload catch block inside
ImageUploadModel.start(media:...), preserve the cancellation semantics by
detecting cancellation errors (e.g., URLError(.cancelled) / NSURLErrorCancelled)
and treating them the same way the original path does: clear progress on the
main queue but do not emit the error haptic or return .failed(error); instead
return the cancellation outcome used elsewhere (or the same value returned for
cancelled in the original branch). Update the catch to branch on cancellation vs
other errors so only non-cancellation failures trigger
UINotificationFeedbackGenerator().notificationOccurred(.error) and
.failed(error).

---

Outside diff comments:
In `@damus/Features/Posting/Views/PostView.swift`:
- Around line 465-478: The handle_upload(media: MediaUpload) function is calling
getImage(media:) (which does synchronous file I/O and video thumbnail work)
before any await, blocking the main actor; move the image/preview work to a
background context (e.g., Task.detached or DispatchQueue.global()) so
getImage(media:) and calculate_blurhash(img:) run off the main actor, then hop
back to `@MainActor` only to assign UI state such as mediaUploadUnderProgress and
to start any UI-triggered uploads; specifically, run getImage(media:) and
blurhash calculation off the main actor, capture their results, then on
MainActor continue the existing uploader selection and calls to
image_upload.start/startBlossomUpload.

---

Duplicate comments:
In `@damus.xcodeproj/project.pbxproj`:
- Line 7419: There is a duplicate malformed BuildFile reference with ID
D73E5EFE2C6A97F4007EB227 appearing twice (once at the original location and
again in another target); locate both occurrences of the BuildFile entry with ID
D73E5EFE2C6A97F4007EB227 in the project.pbxproj and remove or replace the
second/duplicate occurrence so each build target references a valid BuildFile
object (either remove the duplicate from the target's sources/build phase or
point it to the correct BuildFile entry used at the original location). Ensure
the target's PBXBuildFile list and the corresponding PBXSourcesBuildPhase no
longer contain the malformed ID to prevent project corruption.

---

Nitpick comments:
In `@damus/Core/Blossom/BlossomUploader.swift`:
- Around line 51-58: The URLRequest used for uploads in BlossomUploader (the
`request` created for `server.uploadURL` before calling
`URLSession.shared.data(for:delegate:)`) lacks a timeout; set
`request.timeoutInterval` to a sensible default (or read a configurable timeout
passed to the BlossomUploader/initializer) right after constructing the
URLRequest so large uploads won't hang indefinitely, and ensure any callers or
tests that expect different timeouts can override this configuration.

In `@damus/Core/Nostr/NostrFilter.swift`:
- Around line 39-54: The initializer NostrFilter.init(...) and the static
copy(from:) currently omit the mime_types property, causing callers to set it
post-construction and causing copy(from:) to strip mime_types; update the init
signature for NostrFilter to accept mime_types: [String]? (or the appropriate
type) and assign it to self.mime_types, and update public static func
copy(from:) to include mime_types: from.mime_types when creating the new
NostrFilter so copies preserve that field.

In `@damus/Features/Settings/Views/MediaServersSettingsView.swift`:
- Around line 10-56: Add a brief docstring for the MediaServersSettingsView
struct describing its purpose and what it manages (e.g., presents media uploader
selection and Blossom server configuration), placing it immediately above the
declaration for MediaServersSettingsView; ensure it mentions any important
observed state (settings: UserSettingsStore) and behavior (dismissing on
.switched_timeline) so readers understand init, body, and the view’s
responsibilities.

In `@damus/Shared/Media/GIF/bootstrap_gifs.json`:
- Around line 1-165: The bootstrap_gifs.json hardcodes many external GIF URLs
which can break if upstream hosts are unavailable; add a CI validation step that
fetches each URL in damus/Shared/Media/GIF/bootstrap_gifs.json and fails (or
reports) on non-2xx responses, and update the GIF picker image-loading logic to
gracefully handle 404/timeout by falling back to a local placeholder or trying
the next URL in the list; implement the CI check as a script (e.g.,
validate_bootstrap_gifs) and wire it into the repository's CI pipeline, and
ensure the GIF picker’s loader catches fetch/load errors and uses the fallback
behavior.

In `@damus/Shared/Media/GIF/GIFSearchModel.swift`:
- Around line 20-36: Add missing docstrings to fully document the discovery and
cancellation flow: document both DiscoveredGIF initializers (the initializer
taking eventID:url:thumbURL:dim:alt:pubkey: and the init(url:) convenience
initializer), the init(damus_state:) initializer, the cancel() method, and each
private bootstrap/stream/extraction helper function used in GIF discovery; for
each, describe purpose, parameters, return behavior, side effects (e.g.,
network/stream usage or cancellation), and any threading/ownership expectations
so callers understand how discovery starts, yields results, and is cancelled.
Ensure the docs reference the exact symbol names
(DiscoveredGIF.init(eventID:url:thumbURL:dim:alt:pubkey:),
DiscoveredGIF.init(url:), init(damus_state:), cancel(), and the private
bootstrap/stream/extraction helper functions) and follow existing docstring
style and formatting used in the file.

In `@damus/Shared/Media/Models/ImageUploadModel.swift`:
- Around line 80-85: Add docstrings to the new protocol and its methods:
document ImageUploadModelProtocol with a short description of its
responsibility, and add clear documentation for
start(media:uploader:mediaType:keypair:) and
startBlossomUpload(media:keypair:serverURL:) that explains purpose, parameters,
return value (ImageUploadResult), and the preconditions and error cases specific
to the Blossom path (e.g., serverURL required, different auth semantics). Ensure
the docs cover both upload flows and any differences in behavior referenced in
the implementation (also update the related methods at the other modified range
around startLine 118-146) so the protocol and both methods meet the project's
docstring guidelines.

In `@damusTests/EditPictureControlTests.swift`:
- Around line 273-302: Update the test to assert the Blossom server URL is
forwarded: enhance MockImageUploadModel to record the received serverURL (e.g.
add a lastServerURL or similar) and then, in
testBlossomUploaderRoutesToBlossomPath, after triggering the upload and awaiting
the expectation, add an XCTAssertEqual comparing
MockImageUploadModel.lastServerURL to the configured blossom_server_url
("https://blossom.example.com") to ensure the ViewModel passed the correct URL
to the Blossom branch (keep references to MockImageUploadModel,
MockBlossomUploader,
view_model.request_upload_authorization/.confirm_upload_authorization and
blossom_server_url).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 25fb2712-ed02-49e0-867c-e65b0760582d

📥 Commits

Reviewing files that changed from the base of the PR and between bfad604 and 84b0b8e.

📒 Files selected for processing (29)
  • .beads/issues.jsonl
  • damus.xcodeproj/project.pbxproj
  • damus/Core/Blossom/BlossomAuth.swift
  • damus/Core/Blossom/BlossomTypes.swift
  • damus/Core/Blossom/BlossomUploader.swift
  • damus/Core/NIPs/NIP94/FileMetadata.swift
  • damus/Core/Nostr/NostrFilter.swift
  • damus/Core/Nostr/NostrKind.swift
  • damus/Features/Events/Models/LoadableNostrEventView.swift
  • damus/Features/Posting/Views/PostView.swift
  • damus/Features/Profile/Views/EditPictureControl.swift
  • damus/Features/Profile/Views/ProfilePictureSelector.swift
  • damus/Features/Settings/Models/UserSettingsStore.swift
  • damus/Features/Settings/Views/AppearanceSettingsView.swift
  • damus/Features/Settings/Views/ConfigView.swift
  • damus/Features/Settings/Views/MediaServersSettingsView.swift
  • damus/Features/Timeline/Models/HomeModel.swift
  • damus/Shared/Media/GIF/GIFPickerView.swift
  • damus/Shared/Media/GIF/GIFSearchModel.swift
  • damus/Shared/Media/GIF/TenorAPIClient.swift
  • damus/Shared/Media/GIF/TenorModels.swift
  • damus/Shared/Media/GIF/bootstrap_gifs.json
  • damus/Shared/Media/Images/BannerImageView.swift
  • damus/Shared/Media/Models/ImageUploadModel.swift
  • damus/Shared/Media/Models/MediaUploader.swift
  • damus/Shared/Utilities/Router.swift
  • damusTests/BlossomTests.swift
  • damusTests/EditPictureControlTests.swift
  • damusTests/FileMetadataTests.swift
💤 Files with no reviewable changes (3)
  • damus/Features/Settings/Views/AppearanceSettingsView.swift
  • damus/Shared/Media/GIF/TenorModels.swift
  • damus/Shared/Media/GIF/TenorAPIClient.swift

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; };
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if the project file has syntax issues by validating with plutil
plutil -lint damus.xcodeproj/project.pbxproj

# Search for other potential malformed BuildFile entries
rg -n "BuildFile in Sources.*=.*{isa = PBXBuildFile; };" damus.xcodeproj/project.pbxproj

Repository: damus-io/damus

Length of output: 259


🏁 Script executed:

# Read the file around line 1526 to see the actual entry
sed -n '1520,1530p' damus.xcodeproj/project.pbxproj

# Also find some valid BuildFile entries for comparison
grep -n "BuildFile in Sources" damus.xcodeproj/project.pbxproj | head -20

Repository: damus-io/damus

Length of output: 1862


🏁 Script executed:

# Search for all references to the UUID D73E5EFE2C6A97F4007EB227
grep -n "D73E5EFE2C6A97F4007EB227" damus.xcodeproj/project.pbxproj

# Check the context around line 7419
sed -n '7410,7430p' damus.xcodeproj/project.pbxproj

Repository: damus-io/damus

Length of output: 1656


Remove the malformed BuildFile entry that is missing the required fileRef property.

Line 1526 defines a PBXBuildFile without referencing an actual source file. All valid PBXBuildFile entries must include a fileRef property pointing to a file reference UUID. This entry will cause Xcode build failures.

Either remove this entry entirely or correct it by adding the missing fileRef property. The entry also appears referenced in the sources build phase list at line 7419 and should be removed from there as well.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@damus.xcodeproj/project.pbxproj` at line 1526, The PBXBuildFile entry with
UUID D73E5EFE2C6A97F4007EB227 is malformed because it lacks the required fileRef
property; remove this PBXBuildFile object from the project.pbxproj (or add a
correct fileRef pointing to the intended PBXFileReference UUID) and also remove
its occurrence from the Sources build phase list where it is referenced so the
build phase no longer points to D73E5EFE2C6A97F4007EB227; ensure any replacement
fileRef uses an existing PBXFileReference UUID and keep the PBXBuildFile syntax
consistent with other entries (isa = PBXBuildFile; fileRef = <UUID>;) if you
choose to correct instead of delete.

struct MediaServersSettingsView: View {
@ObservedObject var settings: UserSettingsStore
@Environment(\.dismiss) var dismiss
@State private var editingServerURL: String = ""
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Remove unused @State variable.

editingServerURL is declared but never referenced anywhere in this view. This appears to be leftover code.

🧹 Proposed fix
     `@ObservedObject` var settings: UserSettingsStore
     `@Environment`(\.dismiss) var dismiss
-    `@State` private var editingServerURL: String = ""
📝 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
@State private var editingServerURL: String = ""
`@ObservedObject` var settings: UserSettingsStore
`@Environment`(\.dismiss) var dismiss
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@damus/Features/Settings/Views/MediaServersSettingsView.swift` at line 13,
Remove the unused `@State` property by deleting the declaration of
editingServerURL in MediaServersSettingsView (the line: "@State private var
editingServerURL: String = \"\""); ensure there are no remaining references to
editingServerURL elsewhere in the view or related methods, and run a build or
grep to confirm it wasn't intended for any UI binding before committing.

"https://blossom.primal.net/fd5969bd56718a103eab4b55fd2e3c16abb883761dc39de8d13b401f4d15de77.gif",
"https://blossom.primal.net/fee21a999711bd84c0f4f4bd0b5d8e859fd1792c39485ab990566fe0f311917b.gif",
"https://blosstr.com/a4fdb405f0bf2ee5793567a9f8ee8d0a65e3061c5db8a391674d9d2e7a4f5336.gif",
"https://files.sovbit.host/media/c06a1cf64f929bffc260392df43941a4b79c52b1817b4c54d061f9b26bd5e7b9/05b3a0beb16f50f12663cd2973c8600f96cfc318f46002e15e19ad96ccfbfc0b.webp",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

WebP file in GIF bootstrap catalog.

Line 35 contains a .webp file rather than a .gif:

https://files.sovbit.host/media/.../ccfbfc0b.webp

While animated WebP is supported by modern iOS, this may cause inconsistent behavior if the GIF picker UI or downstream code assumes .gif format. Consider either:

  1. Removing this entry
  2. Verifying the GIF picker handles WebP correctly
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@damus/Shared/Media/GIF/bootstrap_gifs.json` at line 35, The JSON bootstrap
list contains an entry with a WebP URL
("https://files.sovbit.host/.../ccfbfc0b.webp") in the GIF catalog; either
remove or replace that URL with a proper .gif file, or update the GIF-loading
pipeline (the GIF picker/display code that consumes bootstrap_gifs.json) to
explicitly accept/handle animated WebP (add MIME/type checks, fallback to a
converted GIF or a safe placeholder, and update any validators that assume
".gif" extensions) so the consumer code no longer breaks when encountering the
.webp entry.

Comment on lines +41 to +42
@MainActor
class GIFSearchModel: ObservableObject {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n damus/Shared/Media/GIF/GIFSearchModel.swift | head -120

Repository: damus-io/damus

Length of output: 4189


🏁 Script executed:

cat -n damus/Shared/Media/GIF/GIFSearchModel.swift | sed -n '120,240p'

Repository: damus-io/damus

Length of output: 4635


🏁 Script executed:

# Check if Task behavior is similar elsewhere in the codebase for context
rg -t swift "Task\s*\{" damus/Shared/Media/GIF/ -A 2 | head -40

Repository: damus-io/damus

Length of output: 750


🏁 Script executed:

# Check how Swift treats methods in `@MainActor` classes
# Let's look for similar patterns and how they're handled elsewhere in the codebase
rg -t swift "@MainActor\s+class" damus/Shared/Media/GIF/ -A 10 | head -50

Repository: damus-io/damus

Length of output: 40


🏁 Script executed:

# Let me verify Swift concurrency behavior and check if there's any documentation
# about Task isolation in the codebase
rg -t swift "Task\s*\{" damus/ --max-count 5 -B 2 -A 4 | head -80

Repository: damus-io/damus

Length of output: 5553


🏁 Script executed:

# Let me check how advancedStream is implemented to understand if it runs on main or background
rg -t swift "func advancedStream" damus/ -A 15

Repository: damus-io/damus

Length of output: 2196


🏁 Script executed:

# Check the full context of streamFilter and how it handles the async stream iteration
cat -n damus/Shared/Media/GIF/GIFSearchModel.swift | sed -n '133,155p'

Repository: damus-io/damus

Length of output: 991


🏁 Script executed:

# Verify if non-isolated async methods inherit class isolation
# Check if there are any explicit isolation markers on streamFilter and queryFileMetadata
cat -n damus/Shared/Media/GIF/GIFSearchModel.swift | sed -n '124,156p'

Repository: damus-io/damus

Length of output: 1354


🏁 Script executed:

# Check if there are examples of nonisolated async functions in the codebase
rg -t swift "nonisolated\s+async" damus/ | head -20

Repository: damus-io/damus

Length of output: 40


🏁 Script executed:

# Let's look for how Task behavior is documented in similar relay streaming code
rg -t swift "AsyncStream\|for await" damus/Core/Networking/ -B 3 -A 5 | head -100

Repository: damus-io/damus

Length of output: 40


🏁 Script executed:

# Let me verify the actual isolation behavior by checking if justUseACopy has any special handling
rg -t swift "justUseACopy" damus/ -B 2 -A 8 | head -60

Repository: damus-io/damus

Length of output: 4721


🏁 Script executed:

# Let me check how ZapsModel handles similar streaming to see if there's a pattern
cat -n damus/Features/Zaps/Models/ZapsModel.swift | grep -A 20 "zapCommsListener = Task"

Repository: damus-io/damus

Length of output: 876


🏁 Script executed:

# Check if ZapsModel is `@MainActor` or has any isolation markers
cat -n damus/Features/Zaps/Models/ZapsModel.swift | head -35

Repository: damus-io/damus

Length of output: 1159


🏁 Script executed:

# Check if there's any explicit nonisolated marking on async methods in `@MainActor` classes
rg -t swift "nonisolated\s+(async|func)" damus/ -B 2 | head -40

Repository: damus-io/damus

Length of output: 775


🏁 Script executed:

# Check what decode_file_metadata and parse_post_blocks do to understand if they're expensive
rg -t swift "func decode_file_metadata|func parse_post_blocks" damus/ -A 5

Repository: damus-io/damus

Length of output: 930


🏁 Script executed:

# Check if HomeModel is also `@MainActor`
grep -n "@MainActor" damus/Features/Timeline/Models/HomeModel.swift | head -5

Repository: damus-io/damus

Length of output: 146


Move relay consumption and event parsing off @MainActor.

Because GIFSearchModel is @MainActor and the async methods queryFileMetadata, streamFilter, and scourKind1ForGIFs lack the nonisolated keyword, they inherit the actor's isolation. This causes the for await stream iteration and per-event parsing (decode_file_metadata, parse_post_blocks) to run on the main thread, blocking the UI while multi-relay results arrive.

Mark these async methods as nonisolated to offload the stream consumption to a background context, or restructure the class to keep actor isolation only on the published properties. Return to @MainActor only when updating gifs and loading.

Also applies to: 65–73, 86–97, 124–232

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@damus/Shared/Media/GIF/GIFSearchModel.swift` around lines 41 - 42,
GIFSearchModel is `@MainActor` so its async methods (queryFileMetadata,
streamFilter, scourKind1ForGIFs and similar blocks at 65–73, 86–97, 124–232) run
on the main thread and block UI; mark those async methods as nonisolated so
relay stream iteration and per-event parsing (for await loops,
decode_file_metadata, parse_post_blocks) run off the main actor, and wrap only
the state updates to published properties (gifs, loading) in MainActor.run or
Task { `@MainActor` in ... } to switch back to the main thread when mutating UI
state. Ensure each method signature is updated to include nonisolated and that
any accesses to actor-isolated properties are performed inside a MainActor hop.

Comment on lines +57 to +74
func load(limit: UInt32 = 200) {
cancel()
seenURLs.removeAll()
gifs.removeAll()

// Seed with bootstrap catalog immediately
loadBootstrapGIFs()

searchTask = Task {
self.loading = true
await self.queryFileMetadata(limit: limit)
}

scourTask = Task {
await self.scourKind1ForGIFs(limit: limit)
self.loading = false
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "GIFSearchModel.swift"

Repository: damus-io/damus

Length of output: 103


🏁 Script executed:

wc -l ./damus/Shared/Media/GIF/GIFSearchModel.swift

Repository: damus-io/damus

Length of output: 107


🏁 Script executed:

cat -n ./damus/Shared/Media/GIF/GIFSearchModel.swift

Repository: damus-io/damus

Length of output: 9809


🌐 Web query:

Swift Task cancellation AsyncStream in-flight events queued callbacks behavior

💡 Result:

When a Swift Task hosting a for await loop over an AsyncStream is cancelled via Task.cancel, the behavior for in-flight events, queued callbacks, and yields is as follows: - The iterator stops pulling new values: The for await loop terminates because the AsyncStream's iterator checks Task.isCancelled internally and returns nil, ending iteration. No new await next calls occur. - Queued/buffered values are dropped: Any values previously yielded by continuation.yield but not yet consumed by the iterator are discarded. The stream does not deliver them to the consumer after cancellation. - In-flight yields may or may not succeed: If a producer calls yield after the consumer task is cancelled (continuation terminated), the yield is ignored/dropped by the AsyncStream. - Producer does not auto-stop: The code running the producer (often a separate Task inside the AsyncStream init closure) continues unless it explicitly checks for cancellation (e.g., Task.checkCancellation or Task.isCancelled) or uses continuation.onTermination to clean up and cancel its own work. - Recommendation: Producers should check Task.isCancelled before yielding and use onTermination for cleanup, e.g.: AsyncStream { continuation in continuation.onTermination = { _ in /* cleanup */ } Task { while !Task.isCancelled { // produce and yield try? await Task.sleep(...) } } } This ensures cooperative cancellation. Apple's docs emphasize cooperative cancellation: Task.cancel sets a flag; code must check it. Examples from sources confirm: buffered bytes from URLSession.bytes drain post-cancel but new fetches stop; producer Tasks need manual handling.

Citations:


🏁 Script executed:

rg -A 5 -B 5 "request.*token|generation.*token|requestID|requestGeneration" ./damus/Shared/Media/GIF/

Repository: damus-io/damus

Length of output: 40


🏁 Script executed:

rg -n "Task \{" ./damus/Shared/Media/GIF/GIFSearchModel.swift

Repository: damus-io/damus

Length of output: 149


🏁 Script executed:

sed -n '77,98p' ./damus/Shared/Media/GIF/GIFSearchModel.swift

Repository: damus-io/damus

Length of output: 656


Coordinate loading state across independent tasks and scope GIF results to the active request.

Two independent tasks in load() cause coordination failures. scourTask unconditionally sets loading = false at line 72 while searchTask may still be iterating—if the kind:1 scour finishes first, the spinner disappears prematurely even though kind:1063 queries are active. Additionally, cancel() uses cooperative task cancellation, allowing @MainActor event handlers already queued before cancellation to execute after state is cleared, leaking stale results. The search() function avoids both issues by using a single task; apply the same pattern to load() by supervising both discovery paths from one parent task, or introduce a request generation token to drop updates from superseded work.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@damus/Shared/Media/GIF/GIFSearchModel.swift` around lines 57 - 74, The load()
method spawns two independent Tasks (searchTask and scourTask) that race and
cause loading to be cleared too early and allow stale results after cancel();
replace this with a single supervising Task (or add a request token) that
launches/awaits both discovery paths (queryFileMetadata(...) and
scourKind1ForGIFs(...)) so loading is set to true at start and only set to false
when the supervised work completes; also ensure cancel() increments/invalidates
the request token and every point that mutates seenURLs/gifs or updates UI
checks the current token (or Task.isCancelled) and drops results from superseded
requests (matching search()’s approach) so queued MainActor handlers cannot leak
stale data.

Comment on lines +126 to +145
do {
let descriptor = try await BlossomUploader.upload(
media: media,
server: server,
keypair: keypair,
delegate: self
)

DispatchQueue.main.async {
self.progress = nil
UINotificationFeedbackGenerator().notificationOccurred(.success)
}
return .success(descriptor.url)
} catch {
DispatchQueue.main.async {
self.progress = nil
UINotificationFeedbackGenerator().notificationOccurred(.error)
}
return .failed(error)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Preserve cancellation semantics on the Blossom path.

start(media:...) suppresses error feedback for NSURLErrorCancelled, but this new branch always emits .error in the catch. Canceling an upload will now look like a failure instead of a user-initiated abort.

🔧 Suggested fix
         } catch {
             DispatchQueue.main.async {
                 self.progress = nil
-                UINotificationFeedbackGenerator().notificationOccurred(.error)
+                if let nsError = error as NSError?,
+                   nsError.domain == NSURLErrorDomain,
+                   nsError.code == NSURLErrorCancelled {
+                    print("Upload forced cancelled by user after Cancelling the Post, no feedback triggered.")
+                } else {
+                    UINotificationFeedbackGenerator().notificationOccurred(.error)
+                }
             }
             return .failed(error)
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@damus/Shared/Media/Models/ImageUploadModel.swift` around lines 126 - 145, In
the BlossomUploader.upload catch block inside ImageUploadModel.start(media:...),
preserve the cancellation semantics by detecting cancellation errors (e.g.,
URLError(.cancelled) / NSURLErrorCancelled) and treating them the same way the
original path does: clear progress on the main queue but do not emit the error
haptic or return .failed(error); instead return the cancellation outcome used
elsewhere (or the same value returned for cancelled in the original branch).
Update the catch to branch on cancellation vs other errors so only
non-cancellation failures trigger
UINotificationFeedbackGenerator().notificationOccurred(.error) and
.failed(error).

alltheseas and others added 7 commits March 18, 2026 23:34
Add MediaServersSettingsView with uploader picker and Blossom server URL
configuration. Wire Blossom upload path through ImageUploadModel and
PostView so media uploads route to the configured Blossom server when
selected. Add .blossom case to MediaUploader enum.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Parse and create NIP-94 file metadata events with tags for url, m,
x, size, dim, blurhash, thumb, alt, and more. Includes helper to
build FileMetadata from Blossom upload results. 12 tests.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Add #m tag filter to NostrFilter for mime type filtering. Create
GIFSearchModel that queries relays for kind 1063 events with
m=image/gif, returning DiscoveredGIF results. Supports both
browsing (load) and hashtag-based search.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Extend GIFSearchModel to scour kind:1 text notes for .gif URLs as a
bootstrap mechanism when kind 1063 metadata events are scarce. Both
sources run concurrently and deduplicate by URL.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Delete TenorAPIClient and TenorModels (Tenor API dies June 2026).
Remove tenor_api_key setting. Add content-warning and #nsfw tag
filtering to GIF discovery when hide_nsfw_tagged_content is enabled.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Replace Tenor-dependent GIFPickerViewModel with GIFSearchModel backed
by kind 1063 relay queries and kind:1 URL scouring. GIFThumbnailView
now uses DiscoveredGIF. GIF selection still inserts URL into post
content via onGIFSelected callback.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Bundle 163 GIF URLs scraped from kind:1 and kind:1063 events across
6 relays. GIFSearchModel loads these immediately on open (shuffled)
while relay queries run in the background, ensuring the picker is
never empty.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Reuse the existing FriendFilter + TrustedNetworkButton pattern from
notifications to let users filter GIF results by their trusted network.
Bootstrap GIFs (nil pubkey) always pass through the filter.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@alltheseas
Copy link
Copy Markdown
Collaborator Author

@jb55 mentions

If we add one i would like a setting that disables downloading gifs since it would be lots of data

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant