grouped UI feeds, nip-05, favorites#3392
Open
alltheseas wants to merge 11 commits intodamus-io:masterfrom
Open
grouped UI feeds, nip-05, favorites#3392alltheseas wants to merge 11 commits intodamus-io:masterfrom
alltheseas wants to merge 11 commits intodamus-io:masterfrom
Conversation
11 tasks
Collaborator
Author
|
Inspired by @dtonon |
danieldaquino
added
the
pr-in-queue
Collaborator
Author
|
Closing in favor of consolidated Linux-style topic branches. This work will be audited against AGENTS.md requirements and cherry-picked into ordered topic branches. See #3624 for tracking. |
Collaborator
Author
|
@coderabbitai review please |
Contributor
|
🧠 Learnings used✅ Actions performedReview triggered.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces a grouped timeline feature that organizes Nostr events by author. It adds filtering, pagination, and backfilling capabilities to timeline views, with domain-based author discovery for NIP-05 integration, comprehensive filtering options, and new modular UI components for grouped and filtered event display. Changes
Sequence DiagramsequenceDiagram
participant User
participant TimelineView as TimelineView<br/>(Grouped Mode)
participant Model as NIP05DomainEventsModel
participant Helpers as NIP05DomainHelpers
participant DB as Nostrdb
participant Grouper as GroupedTimelineGrouper
participant Display as GroupedListView
User->>TimelineView: Enable grouped mode
TimelineView->>Model: subscribe(domain, friend_filter)
Model->>Helpers: authors_for_domain(domain, friend_filter)
alt Friend-filtered mode
Helpers->>DB: scan_all_cached_profiles(domain)
DB-->>Helpers: matching pubkeys
else All authors mode
Helpers->>Helpers: validated_authors_for_domain(domain)
end
Helpers-->>Model: [Pubkey, ...]
Model->>Model: streamItems() with filter.authors
loop Receive events
Model->>Model: add_event(ev)
Model->>Display: insert via EventHolder
end
Display->>Grouper: group(events, filter, settings)
Grouper-->>Display: [AuthorGroup, ...]
Display->>User: Render grouped list by author
User->>TimelineView: Load more / scroll
TimelineView->>Model: load_more()
Model->>Model: fetch_older(until)
Model-->>Display: Insert older events
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
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: 11
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In `@damus/Features/NIP05/Models/NIP05.swift`:
- Around line 121-193: scan_all_cached_profiles is marked `@MainActor` but does
expensive ndb.query work — move it off the main actor by making
scan_all_cached_profiles async (remove `@MainActor`) and perform the DB work on a
background executor (e.g., Task.detached or
DispatchQueue.global()/withCheckedContinuation) so ndb.query and per-note
processing do not block the UI; change its signature to async -> Set<Pubkey>,
ensure any UI-bound profile access is hopped back to `@MainActor` only when
needed, and update authors_for_domain to be async and await
scan_all_cached_profiles (and propagate async/await to callers), keeping names
scan_all_cached_profiles and authors_for_domain to locate the changes.
In `@damus/Features/NIP05/Models/NIP05DomainEventsModel.swift`:
- Around line 166-171: fetch_older currently leaves moreFilter.authors nil when
friend_filter == .all, causing uncontrolled queries; update fetch_older to
always populate moreFilter.authors using the same domain helper used by
streamItems (the domain helpers/author list generation logic referenced near
streamItems()) instead of setting it to nil — remove the conditional that
assigns nil and call the helper to set moreFilter.authors (preserving the
existing friend_filter == .friends_of_friends behavior when that case needs a
different list) so pagination queries always constrain by the domain author
list.
- Around line 76-106: The method resolve_domain_match(pubkey: Pubkey) is unused
and contains a flawed fire-and-forget cleanup (the defer spawns a Task to remove
from requesting_profiles) and lacks the required docstring; remove the entire
nonisolated private func resolve_domain_match implementation (including its
defer and use of requesting_profiles and metaFilter/timedStream logic) from
NIP05DomainEventsModel.swift to eliminate dead code and the misleading async
cleanup; if you intend to keep similar behavior later, replace with a
documented, properly awaited MainActor removal of requesting_profiles and add a
docstring then.
- Around line 143-147: The DispatchQueue.main.async blocks inside the `@MainActor`
class NIP05DomainEventsModel are redundant and should be removed; directly set
the properties (self.loading = false, self.used_initial_page = true,
self.last_loaded_count = self.events.all_events.count) on the main actor without
re-dispatching, and apply the same simplification for the other occurrences that
update properties (e.g., the blocks around where you modify self.loading,
self.used_initial_page, and any assignments to self.events or
self.last_loaded_count at the other two locations referenced).
- Around line 29-37: The constructors and public/internal methods in
NIP05DomainEventsModel lack docstrings; add concise Swift documentation comments
(///) above the init(state:domain:friend_filter:), subscribe(...),
set_friend_filter(...), load_more(), add_event(...), and streamItems(...)
declarations explaining purpose, parameters, return values, and
thread/side-effect notes as appropriate to meet coding guidelines; ensure the
docstrings describe the role of DamusState, domain, FriendFilter, EventHolder
behavior and any async/queue semantics so reviewers can quickly understand
usage.
In `@damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift`:
- Around line 102-127: notesFromAuthorsString and friendsOfFriendsString
duplicate the same logic; consolidate into one reusable function (e.g.,
notesFromAuthorsString or a new helper like authorsSummaryString(authors:
[Pubkey], ndb: Ndb, emptyMessageKey: String, locale: Locale = .current)) that:
looks up up to three display names using ndb.lookup_profile and
Profile.displayName, truncates names, and builds the localized string via
bundleForLocale, NSLocalizedString/localizedStringFormat exactly as the current
switch does; add a parameter for the empty-state key/message so callers
(formerly friendsOfFriendsString) can pass their own NSLocalizedString key or
message, and replace both original functions with thin wrappers that call the
new helper (preserving ndb, Profile.displayName usage and
localizedStringFormat).
- Around line 58-65: The computed property allAuthors uses NSMutableOrderedSet
which can fail to bridge non-ObjC Pubkey values; replace the bridging with a
pure-Swift dedupe that preserves order: combine model.events.all_events.map {
$0.pubkey } and model.filter.authors ?? [], then iterate adding each pubkey to a
result array only if a seen Set (e.g., Set<Pubkey> using Pubkey’s Hashable
conformance) doesn't already contain it. Update the allAuthors getter to build
and return this ordered, deduplicated [Pubkey] (referencing allAuthors, Pubkey,
model.events.all_events, and model.filter.authors).
In `@damus/Features/NIP05/Views/NIP05DomainTimelineView.swift`:
- Line 50: Add a concise docstring above the `@Environment`(\.presentationMode)
var presentationMode declaration in NIP05DomainTimelineView describing what
presentationMode represents (the view's presentation/dismissal binding), how
it's used (to dismiss the view or check presentation state), and any important
behavior or threading assumptions; keep it one or two sentences following
project docstring style and place it directly above the presentationMode
property declaration.
- Around line 19-25: Add a docstring for the computed property `localizedTitle`
on the enum (the property returning NSLocalizedString for `.grouped` and
`.timeline`) describing its purpose (provides the localized tab label for the
domain timeline view), including what values it returns and that it is used for
UI tab labels; place the doc comment immediately above `var localizedTitle:
String` and keep it brief and user-facing, mentioning the `.grouped` and
`.timeline` cases and that strings are localized via NSLocalizedString.
In `@damus/Features/NIP05/Views/NIP05FilterBar.swift`:
- Around line 43-45: The struct NIP05FilterButton is missing docstrings for its
properties; add concise documentation comments above the `@ObservedObject` var
settings: NIP05FilterSettings and the `@Binding` var showFilterSheet: Bool
describing their purpose and ownership (e.g., that settings holds the filter
configuration and is observed for UI updates, and showFilterSheet is a bound
flag controlling presentation of the filter sheet). Use standard Swift doc
comment style (///) and keep descriptions short and focused so the new/modified
properties meet docstring coverage requirements.
- Around line 75-79: The filterSummary property uses hardcoded English strings
("Grouped"/"Timeline"); change it to use the view mode's localized title
instead: derive the mode label from the view-mode enum or a localized property
(e.g., use settings.viewMode.localizedTitle or a localized string lookup keyed
by settings.enableGroupedMode) and keep time as
settings.timeRange.localizedTitle so the returned string is fully localized;
update references in filterSummary to use that localized value rather than the
hardcoded "Grouped"/"Timeline".
In `@damus/Features/NIP05/Views/NIP05FilterSettingsView.swift`:
- Around line 111-118: The Picker is using a hardcoded array [3, 5, 10, 20]
instead of the declared maxNotesOptions; update the ForEach to iterate over
maxNotesOptions (e.g., ForEach(maxNotesOptions, id: \.self)) and keep the
selection binding settings.maxNotesPerUser and tag(count as Int?) so the types
remain Int? and the options stay in sync with the static array defined on the
type.
- Line 5: Remove or replace the atypical AI tool attribution in the file header
of NIP05FilterSettingsView.swift (the "Created by Claude" comment) so it matches
project conventions; update the header comment to the real author name or
project-standard metadata (or remove the line entirely) in the top-of-file
comment block so the file attribution is consistent with other source files.
In `@damus/Features/NIP05/Views/NIP05GroupedAuthorRow.swift`:
- Around line 52-67: Add a brief docstring above the computed property
postCountBadge describing its purpose and behavior: mention that it renders the
author's post count as a stylized Capsule badge, that it adapts font size and
horizontal padding when postCount is >= 100, and that it uses DamusColors.purple
for foreground and background styling; place the comment immediately above the
postCountBadge property so future readers and docs understand what the view
represents and how the layout changes with large counts.
In `@damus/Features/NIP05/Views/NIP05GroupedListView.swift`:
- Around line 115-140: The computed property authorGroups performs an expensive
grouping over events.all_events on each render; move this work off the main
thread by changing authorGroups from a computed property into an
asynchronously-updated published/state-backed property (e.g., `@State` or
`@Published` in the model) and compute its value in a background Task or
DispatchQueue.global() worker that filters via shouldIncludeEvent, applies
settings.maxNotesPerUser, and builds the [AuthorGroup] array (using
AuthorGroup(pubkey:..., latestEvent:..., postCount:...)); when the background
work completes assign the result back to the UI-bound property on the MainActor
so the view reads the precomputed array instead of recomputing on render.
In `@damus/Features/Profile/Views/CondensedProfilePicturesView.swift`:
- Around line 19-22: Add a docstring to the initializer init(state: DamusState,
pubkeys: [Pubkey], maxPictures: Int, size: CGFloat = 32.0) on
CondensedProfilePicturesView describing the purpose of the initializer and
documenting each parameter (state, pubkeys, maxPictures, size) and any defaults
or behavior (e.g., how maxPictures and size affect rendering); place the comment
immediately above the init declaration using the project's Swift doc comment
style (///) so it's picked up by documentation tools and satisfies the
coding-guideline requirement.
🧹 Nitpick comments (5)
🤖 Fix all nitpicks with AI agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Models/NIP05DomainEventsModel.swift`: - Around line 143-147: The DispatchQueue.main.async blocks inside the `@MainActor` class NIP05DomainEventsModel are redundant and should be removed; directly set the properties (self.loading = false, self.used_initial_page = true, self.last_loaded_count = self.events.all_events.count) on the main actor without re-dispatching, and apply the same simplification for the other occurrences that update properties (e.g., the blocks around where you modify self.loading, self.used_initial_page, and any assignments to self.events or self.last_loaded_count at the other two locations referenced). - Around line 29-37: The constructors and public/internal methods in NIP05DomainEventsModel lack docstrings; add concise Swift documentation comments (///) above the init(state:domain:friend_filter:), subscribe(...), set_friend_filter(...), load_more(), add_event(...), and streamItems(...) declarations explaining purpose, parameters, return values, and thread/side-effect notes as appropriate to meet coding guidelines; ensure the docstrings describe the role of DamusState, domain, FriendFilter, EventHolder behavior and any async/queue semantics so reviewers can quickly understand usage. In `@damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift`: - Around line 102-127: notesFromAuthorsString and friendsOfFriendsString duplicate the same logic; consolidate into one reusable function (e.g., notesFromAuthorsString or a new helper like authorsSummaryString(authors: [Pubkey], ndb: Ndb, emptyMessageKey: String, locale: Locale = .current)) that: looks up up to three display names using ndb.lookup_profile and Profile.displayName, truncates names, and builds the localized string via bundleForLocale, NSLocalizedString/localizedStringFormat exactly as the current switch does; add a parameter for the empty-state key/message so callers (formerly friendsOfFriendsString) can pass their own NSLocalizedString key or message, and replace both original functions with thin wrappers that call the new helper (preserving ndb, Profile.displayName usage and localizedStringFormat). - Around line 58-65: The computed property allAuthors uses NSMutableOrderedSet which can fail to bridge non-ObjC Pubkey values; replace the bridging with a pure-Swift dedupe that preserves order: combine model.events.all_events.map { $0.pubkey } and model.filter.authors ?? [], then iterate adding each pubkey to a result array only if a seen Set (e.g., Set<Pubkey> using Pubkey’s Hashable conformance) doesn't already contain it. Update the allAuthors getter to build and return this ordered, deduplicated [Pubkey] (referencing allAuthors, Pubkey, model.events.all_events, and model.filter.authors). In `@damus/Features/NIP05/Views/NIP05FilterSettingsView.swift`: - Line 5: Remove or replace the atypical AI tool attribution in the file header of NIP05FilterSettingsView.swift (the "Created by Claude" comment) so it matches project conventions; update the header comment to the real author name or project-standard metadata (or remove the line entirely) in the top-of-file comment block so the file attribution is consistent with other source files.damus/Features/NIP05/Models/NIP05DomainEventsModel.swift (2)
143-147: RedundantDispatchQueue.main.asyncinside@MainActorclass.Since
NIP05DomainEventsModelis@MainActor, all instance methods already run on the main actor. UsingDispatchQueue.main.asyncre-dispatches to the next run-loop iteration, which introduces unnecessary delay and can cause subtle ordering issues with SwiftUI observation. The same pattern repeats at lines 180 and 195.♻️ Proposed fix (line 143-147)
case .eose: - DispatchQueue.main.async { - self.loading = false - self.used_initial_page = true - self.last_loaded_count = self.events.all_events.count - } + self.loading = false + self.used_initial_page = true + self.last_loaded_count = self.events.all_events.count continueApply the same simplification at lines 180-188 and 195-197.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Models/NIP05DomainEventsModel.swift` around lines 143 - 147, The DispatchQueue.main.async blocks inside the `@MainActor` class NIP05DomainEventsModel are redundant and should be removed; directly set the properties (self.loading = false, self.used_initial_page = true, self.last_loaded_count = self.events.all_events.count) on the main actor without re-dispatching, and apply the same simplification for the other occurrences that update properties (e.g., the blocks around where you modify self.loading, self.used_initial_page, and any assignments to self.events or self.last_loaded_count at the other two locations referenced).
29-37: Missing docstrings on public/internal methods.Per coding guidelines, docstring coverage is required for added/modified code. Key methods like
init,subscribe,set_friend_filter,load_more,add_event, andstreamItemslack documentation. As per coding guidelines: "Ensure docstring coverage for any code added or modified."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Models/NIP05DomainEventsModel.swift` around lines 29 - 37, The constructors and public/internal methods in NIP05DomainEventsModel lack docstrings; add concise Swift documentation comments (///) above the init(state:domain:friend_filter:), subscribe(...), set_friend_filter(...), load_more(), add_event(...), and streamItems(...) declarations explaining purpose, parameters, return values, and thread/side-effect notes as appropriate to meet coding guidelines; ensure the docstrings describe the role of DamusState, domain, FriendFilter, EventHolder behavior and any async/queue semantics so reviewers can quickly understand usage.damus/Features/NIP05/Views/NIP05FilterSettingsView.swift (1)
5-5: AI tool attribution in file header.The "Created by Claude" attribution is atypical for a project's source files. Consider replacing with the actual author's name or handle for consistency with other files in the codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Views/NIP05FilterSettingsView.swift` at line 5, Remove or replace the atypical AI tool attribution in the file header of NIP05FilterSettingsView.swift (the "Created by Claude" comment) so it matches project conventions; update the header comment to the real author name or project-standard metadata (or remove the line entirely) in the top-of-file comment block so the file attribution is consistent with other source files.damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift (2)
102-127:notesFromAuthorsStringandfriendsOfFriendsStringare near-duplicates.These two functions share identical logic (name lookup, switch on count, same format strings) — the only differences are the
case 0fallback and localization comments. Consider consolidating into a single function with a parameter for the empty-state message.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift` around lines 102 - 127, notesFromAuthorsString and friendsOfFriendsString duplicate the same logic; consolidate into one reusable function (e.g., notesFromAuthorsString or a new helper like authorsSummaryString(authors: [Pubkey], ndb: Ndb, emptyMessageKey: String, locale: Locale = .current)) that: looks up up to three display names using ndb.lookup_profile and Profile.displayName, truncates names, and builds the localized string via bundleForLocale, NSLocalizedString/localizedStringFormat exactly as the current switch does; add a parameter for the empty-state key/message so callers (formerly friendsOfFriendsString) can pass their own NSLocalizedString key or message, and replace both original functions with thin wrappers that call the new helper (preserving ndb, Profile.displayName usage and localizedStringFormat).
58-65:NSMutableOrderedSetbridge may silently fail for non-ObjC types.If
Pubkeydoesn't bridge toNSObject,NSMutableOrderedSet(array:)will box the values as opaque objects and theas? [Pubkey]downcast may fail, silently returning[]. A pure-Swift approach avoids this risk entirely.♻️ Proposed fix
private var allAuthors: [Pubkey] { let eventAuthors = model.events.all_events.map { $0.pubkey } let filterAuthors = model.filter.authors ?? [] let combined = eventAuthors + filterAuthors - - // NSMutableOrderedSet removes duplicates while preserving insertion order - return NSMutableOrderedSet(array: combined).array as? [Pubkey] ?? [] + + var seen = Set<Pubkey>() + return combined.filter { seen.insert($0).inserted } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift` around lines 58 - 65, The computed property allAuthors uses NSMutableOrderedSet which can fail to bridge non-ObjC Pubkey values; replace the bridging with a pure-Swift dedupe that preserves order: combine model.events.all_events.map { $0.pubkey } and model.filter.authors ?? [], then iterate adding each pubkey to a result array only if a seen Set (e.g., Set<Pubkey> using Pubkey’s Hashable conformance) doesn't already contain it. Update the allAuthors getter to build and return this ordered, deduplicated [Pubkey] (referencing allAuthors, Pubkey, model.events.all_events, and model.filter.authors).
Comment on lines
+19
to
+25
| var localizedTitle: String { | ||
| switch self { | ||
| case .grouped: | ||
| return NSLocalizedString("Grouped", comment: "Tab label for grouped view of posts by author") | ||
| case .timeline: | ||
| return NSLocalizedString("Timeline", comment: "Tab label for chronological timeline view") | ||
| } |
Contributor
There was a problem hiding this comment.
Add a docstring for localizedTitle.
This is a new computed property on a user-facing enum and should be documented.
📝 Suggested docstring
- var localizedTitle: String {
+ /// Localized label used in mode-selection UI.
+ var localizedTitle: String {As per coding guidelines, Ensure docstring coverage for any code added or modified.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus/Features/NIP05/Views/NIP05DomainTimelineView.swift` around lines 19 -
25, Add a docstring for the computed property `localizedTitle` on the enum (the
property returning NSLocalizedString for `.grouped` and `.timeline`) describing
its purpose (provides the localized tab label for the domain timeline view),
including what values it returns and that it is used for UI tab labels; place
the doc comment immediately above `var localizedTitle: String` and keep it brief
and user-facing, mentioning the `.grouped` and `.timeline` cases and that
strings are localized via NSLocalizedString.
| /// Controls visibility of the filter settings sheet. | ||
| @State private var showFilterSheet: Bool = false | ||
|
|
||
| @Environment(\.presentationMode) var presentationMode |
Contributor
There was a problem hiding this comment.
Add a docstring for presentationMode.
This is a newly added environment property and should be documented.
📝 Suggested docstring
- `@Environment`(\.presentationMode) var presentationMode
+ /// Presentation mode for dismissing the timeline view.
+ `@Environment`(\.presentationMode) var presentationModeAs per coding guidelines, Ensure docstring coverage for any code added or modified.
📝 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
| @Environment(\.presentationMode) var presentationMode | |
| /// Presentation mode for dismissing the timeline view. | |
| `@Environment`(\.presentationMode) var presentationMode |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus/Features/NIP05/Views/NIP05DomainTimelineView.swift` at line 50, Add a
concise docstring above the `@Environment`(\.presentationMode) var
presentationMode declaration in NIP05DomainTimelineView describing what
presentationMode represents (the view's presentation/dismissal binding), how
it's used (to dismiss the view or check presentation state), and any important
behavior or threading assumptions; keep it one or two sentences following
project docstring style and place it directly above the presentationMode
property declaration.
Comment on lines
+52
to
+67
| var postCountBadge: some View { | ||
| let fontSize: Font = postCount >= 100 ? .caption2.weight(.medium) : .caption.weight(.medium) | ||
| return Text("\(postCount)") | ||
| .font(fontSize) | ||
| .foregroundColor(DamusColors.purple) | ||
| .frame(minWidth: 28, minHeight: 28) | ||
| .padding(.horizontal, postCount >= 100 ? 4 : 0) | ||
| .background( | ||
| Capsule() | ||
| .fill(DamusColors.purple.opacity(0.1)) | ||
| .overlay( | ||
| Capsule() | ||
| .stroke(DamusColors.purple.opacity(0.2), lineWidth: 1) | ||
| ) | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
Add a docstring for postCountBadge.
This is a new computed property and should be documented.
📝 Suggested docstring
- var postCountBadge: some View {
+ /// Badge displaying the author’s post count in the grouped list.
+ var postCountBadge: some View {As per coding guidelines, Ensure docstring coverage for any code added or modified.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus/Features/NIP05/Views/NIP05GroupedAuthorRow.swift` around lines 52 - 67,
Add a brief docstring above the computed property postCountBadge describing its
purpose and behavior: mention that it renders the author's post count as a
stylized Capsule badge, that it adapts font size and horizontal padding when
postCount is >= 100, and that it uses DamusColors.purple for foreground and
background styling; place the comment immediately above the postCountBadge
property so future readers and docs understand what the view represents and how
the layout changes with large counts.
Comment on lines
+115
to
+140
| /// Groups all events by author, applying filters and tracking the latest event per author. | ||
| var authorGroups: [AuthorGroup] { | ||
| var groupsByAuthor: [Pubkey: (latest: NostrEvent, count: Int)] = [:] | ||
|
|
||
| for event in events.all_events { | ||
| guard shouldIncludeEvent(event) else { continue } | ||
|
|
||
| if let existing = groupsByAuthor[event.pubkey] { | ||
| // Update count; keep the more recent event as "latest" | ||
| let newLatest = event.created_at > existing.latest.created_at ? event : existing.latest | ||
| groupsByAuthor[event.pubkey] = (latest: newLatest, count: existing.count + 1) | ||
| } else { | ||
| groupsByAuthor[event.pubkey] = (latest: event, count: 1) | ||
| } | ||
| } | ||
|
|
||
| // Apply max notes per user filter (exclude prolific posters) | ||
| if let maxNotes = settings.maxNotesPerUser { | ||
| groupsByAuthor = groupsByAuthor.filter { $0.value.count <= maxNotes } | ||
| } | ||
|
|
||
| // Convert to array sorted by most recent activity | ||
| return groupsByAuthor | ||
| .map { AuthorGroup(pubkey: $0.key, latestEvent: $0.value.latest, postCount: $0.value.count) } | ||
| .sorted { $0.latestEvent.created_at > $1.latestEvent.created_at } | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid computing authorGroups on the main thread.
This grouping walks the entire event list on every render and can block UI for large feeds. Move the computation to a background task and publish the result back to the view (e.g., via @State or the model).
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/NIP05/Views/NIP05GroupedListView.swift` around lines 115 -
140, The computed property authorGroups performs an expensive grouping over
events.all_events on each render; move this work off the main thread by changing
authorGroups from a computed property into an asynchronously-updated
published/state-backed property (e.g., `@State` or `@Published` in the model) and
compute its value in a background Task or DispatchQueue.global() worker that
filters via shouldIncludeEvent, applies settings.maxNotesPerUser, and builds the
[AuthorGroup] array (using AuthorGroup(pubkey:..., latestEvent:...,
postCount:...)); when the background work completes assign the result back to
the UI-bound property on the MainActor so the view reads the precomputed array
instead of recomputing on render.
Comment on lines
+19
to
+22
| init(state: DamusState, pubkeys: [Pubkey], maxPictures: Int, size: CGFloat = 32.0) { | ||
| self.model = CondensedProfilePicturesViewModel(state: state, pubkeys: pubkeys, maxPictures: maxPictures) | ||
| self.size = size | ||
| } |
Contributor
There was a problem hiding this comment.
Add a docstring for the initializer.
This is a modified API surface and should be documented.
📝 Suggested docstring
- init(state: DamusState, pubkeys: [Pubkey], maxPictures: Int, size: CGFloat = 32.0) {
+ /// Creates a condensed, overlapping stack of profile pictures.
+ init(state: DamusState, pubkeys: [Pubkey], maxPictures: Int, size: CGFloat = 32.0) {As per coding guidelines, Ensure docstring coverage for any code added or modified.
📝 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
| init(state: DamusState, pubkeys: [Pubkey], maxPictures: Int, size: CGFloat = 32.0) { | |
| self.model = CondensedProfilePicturesViewModel(state: state, pubkeys: pubkeys, maxPictures: maxPictures) | |
| self.size = size | |
| } | |
| /// Creates a condensed, overlapping stack of profile pictures. | |
| init(state: DamusState, pubkeys: [Pubkey], maxPictures: Int, size: CGFloat = 32.0) { | |
| self.model = CondensedProfilePicturesViewModel(state: state, pubkeys: pubkeys, maxPictures: maxPictures) | |
| self.size = size | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus/Features/Profile/Views/CondensedProfilePicturesView.swift` around lines
19 - 22, Add a docstring to the initializer init(state: DamusState, pubkeys:
[Pubkey], maxPictures: Int, size: CGFloat = 32.0) on
CondensedProfilePicturesView describing the purpose of the initializer and
documenting each parameter (state, pubkeys, maxPictures, size) and any defaults
or behavior (e.g., how maxPictures and size affect rendering); place the comment
immediately above the init declaration using the project's Swift doc comment
style (///) so it's picked up by documentation tools and satisfies the
coding-guideline requirement.
alltheseas
force-pushed
the
nip05-grouped-ui
branch
2 times, most recently
from
e581d31 to
651a654
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (14)
damus/Features/Timeline/Models/HomeModel.swift (4)
766-772: Add docstrings forbackfillTask,ndbRefreshTask, andbackfillPageLimit.These new properties lack documentation. As per coding guidelines, ensure docstring coverage for any code added or modified.
🤖 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 766 - 772, Add docstrings above the three properties describing their purpose and usage: for backfillTask explain it is a Task<Void, Never>? that performs relay backfills to fill older events for grouped mode and mention its lifecycle/cancellation behavior; for ndbRefreshTask explain it is a Task<Void, Never>? responsible for refreshing the ndb (or network DB) cache/state and when it runs; for backfillPageLimit (UInt32) document that it is the maximum number of events fetched per backfill page, include the unit and default value (5000) and any constraints. Place the comments directly above the declarations for backfillTask, ndbRefreshTask, and backfillPageLimit and keep them concise.
854-866: The double-break EOSE pattern is hard to follow.The
breakon line 861 exits theswitch, then line 865 re-checksitemto break thefor awaitloop. This works but is fragile. A labeled loop is the idiomatic Swift approach:♻️ Labeled loop
- for await item in damus_state.nostrNetwork.reader.advancedStream(filters: [chunkFilter], streamMode: .ndbAndNetworkParallel(networkOptimization: nil)) { + streamLoop: for await item in damus_state.nostrNetwork.reader.advancedStream(filters: [chunkFilter], streamMode: .ndbAndNetworkParallel(networkOptimization: nil)) { switch item { case .event(let lender): ... case .eose: - break + break streamLoop case .ndbEose, .networkEose: continue } - if case .eose = item { break } }🤖 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 854 - 866, The current double-break pattern (a `break` inside the `switch` for `.eose` and then `if case .eose = item { break }` after the switch) is fragile; convert the surrounding `for await` loop into a labeled loop (e.g., timelineLoop:) and replace both the post-switch `if case .eose` check and the inner switch `case .eose: break` behavior with a single `break timelineLoop` from inside the `.eose` switch arm so that `lender.justUseACopy`, `should_show_event(state: self.damus_state, ev:)`, and appending to `allEvents` remain unchanged but loop exit is explicit and idiomatic.
869-878: Bulk insert loop and redundantDispatchQueue.main.async.
HomeModelis@MainActorand theTaskinherits actor context, soself.backfilling = falsecan be set directly withoutDispatchQueue.main.async. The extra hop is unnecessary.Also, the bulk insert iterates all events sequentially on the main actor. For large result sets (up to 5000 per chunk × N chunks), this could cause a UI hitch. Consider batching or yielding periodically.
♻️ Remove redundant dispatch
- DispatchQueue.main.async { self.backfilling = false } + self.backfilling = false🤖 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 869 - 878, The loop is doing sequential main-actor work and then uses a redundant DispatchQueue.main.async to set self.backfilling; since HomeModel is `@MainActor` the DispatchQueue hop should be removed and set backfilling directly (reference HomeModel and backfilling). To avoid UI hitches when inserting large chunks, change the insertion loop around damus_state.events / favoriteEvents.insert(_:)/insert_home_event(_:) to perform batched inserts or periodically yield to the executor (e.g., process N events then await Task.yield() or insert in a batch) so the main actor can process UI work between batches.
885-914:refreshSingleAuthorEventsspawns a fire-and-forgetTaskwith no cancellation handle.Unlike
backfillTaskandndbRefreshTask, this task is not stored. Rapid calls (e.g., navigating between author profiles) will accumulate concurrent streams. Consider storing and cancelling the previous task, or document that concurrent calls are safe due to idempotent inserts.♻️ Store the task for cancellation
+ private var singleAuthorRefreshTask: Task<Void, Never>? + func refreshSingleAuthorEvents(pubkey: Pubkey, since: UInt32, source: TimelineSource) { + singleAuthorRefreshTask?.cancel() ... - Task { + singleAuthorRefreshTask = Task {🤖 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 885 - 914, The refreshSingleAuthorEvents function spawns a fire-and-forget Task (via damus_state.nostrNetwork.reader.advancedStream) without keeping a handle, causing concurrent streams to accumulate; change it to store the Task in a cancellable property (e.g., singleAuthorRefreshTask) on the HomeModel, cancel any existing task before creating a new one, and assign the new Task to that property; ensure you cancel/cleanup this stored task where other stored tasks are handled (similar to backfillTask and ndbRefreshTask) and on deinit or when the stream should stop.damus/Shared/Components/GroupedTimeline/GroupedTimelineGrouper.swift (1)
91-95: Add docstrings to internal filtering methods.
parseFilteredWords,shouldIncludeEvent,containsFilteredWords, andisTooShortlack docstrings. As per coding guidelines, ensure docstring coverage for any code added or modified.Also applies to: 97-110, 112-116
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Shared/Components/GroupedTimeline/GroupedTimelineGrouper.swift` around lines 91 - 95, Add missing docstrings for the internal filtering helpers parseFilteredWords, shouldIncludeEvent, containsFilteredWords, and isTooShort: for each function add a brief Swift doc comment that states the purpose, describes parameters and types, explains return value, and any important behavior or side‑effects (e.g., trimming, lowercasing, minimum length). Place the comments immediately above the function declarations so they follow project docstring style and cover the functions at/around parseFilteredWords, shouldIncludeEvent, containsFilteredWords, and isTooShort.damus/Features/NIP05/Models/NIP05DomainEventsModel.swift (1)
82-132: Several methods lack docstrings.
streamItems,load_more,fetch_older,add_event,set_friend_filter, andmatches_domainare all missing docstrings. As per coding guidelines, ensure docstring coverage for any code added or modified.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Models/NIP05DomainEventsModel.swift` around lines 82 - 132, Add missing Swift docstrings for the listed methods: streamItems(), load_more(), fetch_older(), add_event(_:), set_friend_filter(_:), and matches_domain(_:). For each method add a brief triple-slash (///) comment immediately above the function that states its purpose, describes parameters and return value (or that it returns Void), notes async behavior or side effects (e.g., updates state.loading, mutates events, triggers network streams, or validates NIP-05), and any important invariants (e.g., when since is set use different limits, or that matches_domain checks NIP-05 domain matching). Follow the repository’s existing docstring style (brief summary line, parameter tags if used) and keep comments concise.damus/Features/NIP05/Models/NIP05.swift (2)
220-244:ordered_domain_authorsusesNSMutableOrderedSet— consider a Swift-native approach.
NSMutableOrderedSetworks but loses type safety (requires theas? [Pubkey]cast on line 231 with a fallback to[]). A simpleseenset withfilterpreserves order and avoids the Obj-C bridge:♻️ Swift-native dedup
- let pubkeys = eventPubkeys + (filterAuthors ?? []) - let orderedUnique = NSMutableOrderedSet(array: pubkeys).array as? [Pubkey] ?? [] + let pubkeys = eventPubkeys + (filterAuthors ?? []) + var seen = Set<Pubkey>() + let orderedUnique = pubkeys.filter { seen.insert($0).inserted }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Models/NIP05.swift` around lines 220 - 244, ordered_domain_authors currently uses NSMutableOrderedSet which loses Swift type-safety; replace the Obj-C collection with a Swift-native dedup by iterating the concatenated array (eventPubkeys + (filterAuthors ?? [])), maintaining a Set<Pubkey> seen to skip duplicates while building a new [Pubkey] orderedUnique, then proceed to filter with matches_domain(domain:profiles:) and the friend_filter branch (contacts.is_in_friendosphere) unchanged; ensure Pubkey remains Hashable or use an appropriate unique key when populating seen so the behavior and order match the original.
184-201: Friends-of-friends loop on@MainActormay stall the UI for large social graphs.The
.friends_of_friendsbranch iteratescontacts.get_friend_of_friends_list()— which can be thousands of pubkeys — callingmatches_domain(profile lookups) for each, all on the main actor. Consider offloading this to a background task similar to how the.allcase usesTask.detachedfor the scan.♻️ Sketch
case .friends_of_friends: - // WOT mode: Only show users in your trusted network - for pubkey in contacts.get_friend_of_friends_list() where matches_domain(pubkey, domain: domain, profiles: profiles) { - authors.insert(pubkey) - } + // WOT mode: capture the set, then filter on background + let fofList = contacts.get_friend_of_friends_list() + // matches_domain needs `@MainActor` for profile lookups, + // so batch the check to minimize main-thread time + for pubkey in fofList { + if matches_domain(pubkey, domain: domain, profiles: profiles) { + authors.insert(pubkey) + } + }A more thorough fix would make
matches_domainnot require@MainActorby snapshotting the needed profile data first.As per coding guidelines, Never block the main thread: All network requests, database queries, and expensive computations must run on background threads/queues.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Models/NIP05.swift` around lines 184 - 201, The friends-of-friends branch runs a potentially large loop on the main actor (iterating contacts.get_friend_of_friends_list() and calling matches_domain), which can stall the UI; move that work to a background task similar to the .all case by performing the iteration inside Task.detached and returning the authors set, or snapshot the minimal profile data off-main-thread first and make matches_domain operate on that snapshot; update the .friends_of_friends case to await the detached task and fall back to validated_authors_for_domain if the result is empty.damus/Shared/Components/GroupedTimeline/GroupedListView.swift (1)
29-35:group()recomputed synchronously inbodyon every render.This is the call site for the O(n) grouping pass discussed in
PostingTimelineView. Sinceeventsis@ObservedObject, any mutation (new event insertion, flush) triggers a body re-evaluation and a full regroup. Consider lifting this computation out ofbody— e.g., into a@State/view-model property that updates via.onChange(of:)or Combine, so re-renders that don't change inputs skip the grouping work.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Shared/Components/GroupedTimeline/GroupedListView.swift` around lines 29 - 35, GroupedTimelineGrouper.group(...) is being called synchronously inside GroupedListView.body (using events.all_events, filter, settings.filterValues, Date()), causing an O(n) regroup on every SwiftUI body recompute; move this heavy computation out of body into a stored state or view-model property (e.g., `@State` or `@StateObject/`@ObservedObject) such as a `grouped` property on GroupedListView or its VM, and update that property only when inputs change by using .onChange(of:) for events.all_events, filter, and settings.filterValues (or subscribe to events' publisher via Combine) so renders that don't change those inputs skip the grouping work.damus/Features/Timeline/Views/PostingTimelineView.swift (1)
256-272: Add docstrings to new helper methods.Per coding guidelines, all added or modified code should have docstring coverage. These private helpers (
flushActiveSource,backfillForGroupedMode,refreshFromNdb) lack documentation.📝 Suggested docstrings
+ /// Flushes queued events from the active timeline source (follows or favorites) + /// into the main events list and disables further queueing. `@MainActor` private func flushActiveSource() { ... } + /// Triggers a relay backfill for the grouped mode time window. + /// - Parameter showLoading: Whether to display a loading/shimmer state during the fetch. private func backfillForGroupedMode(showLoading: Bool = true) { ... } + /// Refreshes grouped mode data from the local nostrdb cache for the current time window. private func refreshFromNdb() { ... }As per coding guidelines: "Ensure docstring coverage for any code added or modified."
🤖 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 256 - 272, Add docstrings for the three private helper methods to satisfy docstring coverage: document flushActiveSource() (annotated `@MainActor`) with a one-line summary of what it flushes and any threading/actor expectations, document backfillForGroupedMode(showLoading:) describing the purpose, the meaning and default of the showLoading parameter, how the since timestamp is computed and that it calls home.backfillForGroupedMode, and document refreshFromNdb() summarizing the since computation and that it refreshes grouped data from NDB; keep each docstring concise and follow existing project style (one-line summary + brief param/side-effect notes).damus.xcodeproj/project.pbxproj (1)
1976-1981: Synthetic UUIDs are unusual but functional — consider regenerating via Xcode for consistencyThe new
GroupedTimelineentries use hand-crafted sequential UUIDs (e.g.,A1B2C3D4E5F6A7B8C9D0E1F2,B1B2B3B4B5B6B7B8B9B0C1C2) rather than Xcode-generated random ones, which is unconventional. However, these UUIDs are:
- Unique within the project (no collisions detected)
- Properly referenced and functional
- All associated source and test files exist on disk
While Xcode-generated UUIDs are the standard practice and preferable for long-term maintainability, the current entries pose no immediate functional risk. If consistency with the rest of the project is desired, re-add these files via Xcode's File → Add Files to "damus"… dialog to generate proper UUIDs; otherwise, these entries are stable as-is.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus.xcodeproj/project.pbxproj` around lines 1976 - 1981, The PBXBuildFile entries for GroupedTimelineGrouper.swift and GroupedModeQueueManager.swift use hand-crafted sequential UUIDs (e.g., A1B2C3D4E5F6A7B8C9D0E1F2 and B1B2B3B4B5B6B7B8B9B0C1C2) which is unconventional; to fix, remove these manual entries and re-add the files using Xcode's File → Add Files to "damus"… (or close Xcode, run Xcode to re-generate UUIDs) so Xcode will create standard random UUIDs for GroupedTimelineGrouper.swift and GroupedModeQueueManager.swift and update the PBXBuildFile references consistently with the rest of the project.damus/Features/NIP05/Views/NIP05DomainTimelineView.swift (1)
56-58:contentFiltersis re-created on every view body evaluation.This computed property allocates a new
ContentFiltersand its closure array each time the view re-renders. Consider caching it or moving it to a stored property/model if profiling shows this view re-renders frequently (e.g., during streaming events).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Views/NIP05DomainTimelineView.swift` around lines 56 - 58, The computed property contentFilters currently creates a new ContentFilters (and its closures) every time the view re-evaluates; change it to a stored/cached value to avoid repeated allocation — for example, initialize a stored property once (e.g., a private let or lazy var contentFilters) or move it into the view's model/@StateObject so ContentFilters.defaults(damus_state:) is called only once; update references to use that stored contentFilters and ensure it is recalculated only when damus_state actually changes.damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift (2)
56-64: Profile DB lookups and full-event iteration run synchronously on the main thread.
allAuthorsiterates every event inmodel.events.all_eventson each body evaluation, andnotesFromAuthorsStringperforms up to 3ndb.lookup_profile_and_copycalls. Both execute synchronously within the view body (main actor). While LMDB reads are fast, the coding guidelines state: "All … database queries … must run on background threads." For small event counts this is fine in practice, but as the event set grows,allAuthors's O(n) pass on every render could become noticeable.Consider pre-computing
allAuthorsin the model or caching it via@State/.task {}, and moving the profile lookups into the same async path.As per coding guidelines, "Never block the main thread: All network requests, database queries, and expensive computations must run on background threads/queues."
Also applies to: 74-74
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift` around lines 56 - 64, The computed property allAuthors and the synchronous profile lookups (ndb.lookup_profile_and_copy invoked by notesFromAuthorsString) run on the main actor and iterate model.events.all_events on every view evaluation; move these expensive DB reads and the O(n) event scan off the main thread by computing and caching the authors list in the model (e.g., add a precomputed property or method on the view model that updates on events change) or in the view using `@State` plus a .task to populate it, and perform profile lookups from notesFromAuthorsString in the same background task/queue (or expose an async lookup method on the model) so the view body only reads cheap, precomputed data (refer to allAuthors, notesFromAuthorsString, model.events.all_events, and ndb.lookup_profile_and_copy).
101-126: Near-identical logic betweennotesFromAuthorsStringandfriendsOfFriendsString.These two functions share the same structure — profile lookup, name truncation, and switch over count with the same format strings. The only real differences are the
case 0fallback text and the profile-lookup call (thoughfriendsOfFriendsStringmanually inlines whatlookup_profile_and_copydoes). Consider extracting a shared helper parameterized by the zero-case string.♻️ Sketch of a shared helper
+/// Shared implementation for building "Notes from X, Y & Z others" strings. +private func notesFromString(_ pubkeys: [Pubkey], ndb: Ndb, locale: Locale, emptyMessage: String) -> String { + let bundle = bundleForLocale(locale: locale) + let names: [String] = pubkeys.prefix(3).map { pk in + let profile = try? ndb.lookup_profile_and_copy(pk) + return Profile.displayName(profile: profile, pubkey: pk).username.truncate(maxLength: 20) + } + switch pubkeys.count { + case 0: return emptyMessage + case 1: return String(format: NSLocalizedString("Notes from %@", bundle: bundle, comment: ""), locale: locale, names[0]) + case 2: return String(format: NSLocalizedString("Notes from %@ & %@", bundle: bundle, comment: ""), locale: locale, names[0], names[1]) + case 3: return String(format: NSLocalizedString("Notes from %@, %@ & %@", bundle: bundle, comment: ""), locale: locale, names[0], names[1], names[2]) + default: + let format = localizedStringFormat(key: "notes_from_three_and_others", locale: locale) + return String(format: format, locale: locale, pubkeys.count - 3, names[0], names[1], names[2]) + } +}Also applies to: 149-177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift` around lines 101 - 126, Refactor the near-duplicate logic in notesFromAuthorsString and friendsOfFriendsString into a single helper that returns the localized summary string: extract the shared steps (profile lookup via a provided lookup function, name extraction/truncation to 20 chars, and the switch on authors.count that formats 0/1/2/3/+ cases) into a new function (e.g., summarizeAuthors(_:lookup:zeroCaseKey:ndb:locale:) or similar) and have both notesFromAuthorsString and friendsOfFriendsString call it; parameterize the helper with the zero-case localization key/string and a profile-lookup closure (to accommodate ndb.lookup_profile_and_copy vs the inline logic), reuse localizedStringFormat for the default case, and ensure you pass Profile.displayName(...).username.truncate(maxLength: 20) as the name transformation so behavior stays identical.
🤖 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 2040: The PBXFileReference entries for the new GroupedTimeline files
(e.g., GroupedFilterButton.swift) incorrectly include the non-standard attribute
includeInIndex = 1; remove that attribute from each PBXFileReference (the
entries named for GroupedFilterButton.swift and the other GroupedTimeline files)
or, preferably, re-add the files via Xcode so Xcode generates proper
PBXFileReference entries without includeInIndex; update the PBX entries (the
PBXFileReference blocks) to match existing source file references by deleting
includeInIndex = 1.
In `@damus/Features/NIP05/Models/NIP05DomainEventsModel.swift`:
- Around line 44-61: The subscribe(...) method currently resets pagination state
(used_initial_page, has_more, last_loaded_count) even when resetEvents is false,
causing duplicate fetches; change it so that when resetEvents is false (used for
set_friend_filter), you preserve pagination state instead of resetting it—either
skip resetting used_initial_page/has_more/last_loaded_count altogether or set
last_loaded_count to events.count and update a highWaterMark (e.g.,
highestEventTimestamp or similar) so streamItems() can continue from the correct
point; locate the subscribe(...) function and adjust the branch around
events.reset()/events.flush() and the subsequent
used_initial_page/has_more/last_loaded_count assignments so preserved events
aren’t re-fetched unnecessarily.
In `@damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift`:
- Around line 80-88: The tap-target is implemented with .onTapGesture which
isn't exposed as a button to VoiceOver; replace the tap handler with a proper
Button (or, if keeping the visual, wrap the existing VStack inside a Button and
apply .buttonStyle(.plain) / PlainButtonStyle()) and move the guard
!authors.isEmpty and the damus_state.nav.push(Route.NIP05DomainPubkeys(domain:
model.domain, nip05_domain_favicon: nip05_domain_favicon, pubkeys: authors))
into the Button's action so the control is announced as interactive
(alternatively add .accessibilityAddTraits(.isButton) to the VStack, but prefer
using Button to match the title view).
In `@damus/Features/NIP05/Views/NIP05DomainTimelineView.swift`:
- Around line 108-111: NIP05DomainTimelineView uses hard-coded paddings
(.padding(.top, 50) and .padding(.leading, 45)); replace them with
safe-area-aware and measured values: wrap the view in a GeometryReader and use
proxy.safeAreaInsets.top (or
UIApplication.shared.windows.first?.safeAreaInsets.top) instead of the constant
50 for the top offset, and replace the leading magic number by measuring the
back button's frame via a custom PreferenceKey/AnchorPreference (attach a
background GeometryReader to the back button to publish its maxX) and apply that
measured value as the leading padding/alignment; update NIP05DomainTimelineView
to consume those values and remove the hard-coded constants.
- Around line 189-192: The appear handler currently skips subscribing if cached
events exist; update handleOnAppear to always call model.subscribe(...) (using
the same groupedSince) so the relay connection is restored after
handleOnDisappear unsubscribed; mirror the pattern used in
ZapsView.swift/timeline views by removing the guard that checks
model.events.all_events.isEmpty and call model.subscribe(since: groupedSince)
unconditionally (or call subscribe(resetEvents: false, since: groupedSince) if
that overload exists) so cached events are preserved while re-establishing the
live subscription.
- Around line 169-178: The subscribe() method currently assigns a new
loadingTask without canceling any prior task, risking concurrent tasks; update
NIP05DomainTimelineView's model.subscribe implementation to cancel any existing
loadingTask before creating a new one by calling loadingTask?.cancel()
immediately prior to assigning loadingTask = Task { await streamItems() },
following the EventsModel/LiveEventModel pattern so
subscribe(resetEvents:since:) safely replaces the previous task.
In `@damus/Features/Timeline/Views/PostingTimelineView.swift`:
- Around line 224-254: The file is missing docstrings for private helpers and is
recomputing grouping on every SwiftUI render; add concise doc comments to
groupedContentView, flushActiveSource(), backfillForGroupedMode(), and
refreshFromNdb() per project guidelines, and avoid the O(n) grouping in
GroupedListView.body by memoizing the grouped result in PostingTimelineView: add
a `@State` (or `@StateObject`) property to hold the grouped/timestamped groups,
compute it only when events.all_events, groupedFilterSettings.filterValues, or
the passed filter change (use onChange, .task, or a Combine publisher to update
the `@State`), then pass the memoized grouped data into GroupedListView instead of
letting GroupedListView call GroupedTimelineGrouper.group on every render (refer
to groupedContentView, GroupedListView, GroupedTimelineGrouper.group, and
groupedFilterSettings to locate the spots to change).
In `@damus/Shared/Components/GroupedTimeline/GroupedListView.swift`:
- Around line 62-75: The empty-state view is using hardcoded English strings;
update the Text initializers in the emptyStateView to use localized keys instead
(e.g., replace "No matching posts" and "Try adjusting your filters" with
NSLocalizedString or SwiftUI LocalizedStringKey usages) so they integrate with
the app’s i18n system; ensure the conditional message that depends on
hasActiveFilters continues to show the localized secondary string and keep the
same font/foreground modifiers on the localized Text views.
In `@damus/Shared/Components/GroupedTimeline/GroupedTimelineGrouper.swift`:
- Around line 58-61: The subtraction assigning cutoff can underflow when
converting TimeInterval to UInt32; update the calculation in the function that
returns [AuthorGroup] (around parseFilteredWords/values.filteredWords) to
perform a saturating subtraction: compute the epoch seconds as
UInt32(now.timeIntervalSince1970) safely (or cast after checking) and subtract
values.timeRangeSeconds using a safe method (e.g., use max(0, Int64(epoch) -
Int64(values.timeRangeSeconds)) or UInt32.max-limited logic) so cutoff never
underflows; adjust the variable named cutoff to be derived from this saturating
result (keeping its UInt32 type if required).
- Around line 118-138: The isTooShort(_ event: NostrEvent, hideShortNotes: Bool)
function currently rejects any single-word note via the words.count == 1 check,
which hides long single-token posts; change that check so it only flags very
short single-word notes (e.g., require words.count == 1 && content.count < 10)
or remove the single-word check entirely—update the condition using the existing
content/words variables in isTooShort to preserve long single-word notes like
long hashtags.
- Around line 125-128: The current text filtering (creating textWithoutEmojis
from content using !$0.properties.isEmoji) removes digits and certain symbols;
change the predicate to use !$0.properties.isEmojiPresentation so only scalars
that render as emoji by default are stripped, preserving numeric and symbol
characters used in content length validation (update the filter applied to
content.unicodeScalars in the GroupedTimelineGrouper.swift code where
textWithoutEmojis is constructed).
---
Duplicate comments:
In `@damus/Features/NIP05/Views/NIP05DomainTimelineView.swift`:
- Around line 12-27: Add a documentation comment for the localizedTitle computed
property in the NIP05ViewMode enum: describe what localizedTitle returns (a
user-facing localized label for each view mode), mention it's used for tab
labels, and reference that it supports .grouped and .timeline cases; update the
docstring above the var localizedTitle to follow the project's doc comment
style.
In `@damus/Features/Profile/Views/CondensedProfilePicturesView.swift`:
- Around line 19-22: Add a Swift docstring for the initializer of
CondensedProfilePicturesView describing its purpose and parameters; update the
init(state:pubkeys:maxPictures:size:) initializer to include a brief summary
line and `@param` descriptions for state (DamusState), pubkeys ([Pubkey]),
maxPictures (Int) and size (CGFloat, default 32.0), and mention any important
behavior (e.g., that it constructs CondensedProfilePicturesViewModel) to satisfy
documentation guidelines.
In `@damus/Shared/Components/GroupedTimeline/GroupedAuthorRow.swift`:
- Around line 50-65: Add a docstring for the computed view property
postCountBadge describing its purpose and behavior: explain that it returns a
Text view showing the post count with conditional font sizing and padding, the
purple foreground and capsule background/overlay styling, and note the threshold
logic (postCount >= 100) that alters font and padding; place this brief comment
immediately above the postCountBadge property to satisfy documentation
guidelines.
In `@damus/Shared/Components/GroupedTimeline/GroupedFilterSettingsView.swift`:
- Around line 97-104: Update the Picker to iterate over the canonical options:
use GroupedFilterSettings.maxNotesOptions.compactMap({ $0 }) in the ForEach (as
done) and ensure the selection binding settings.maxNotesPerUser and the tag
types match optional Ints (Disabled -> tag(nil as Int?) and each option ->
tag(count as Int?)); no other changes needed beyond keeping the ForEach
reference to GroupedFilterSettings.maxNotesOptions and the tag types consistent
with settings.maxNotesPerUser.
---
Nitpick comments:
In `@damus.xcodeproj/project.pbxproj`:
- Around line 1976-1981: The PBXBuildFile entries for
GroupedTimelineGrouper.swift and GroupedModeQueueManager.swift use hand-crafted
sequential UUIDs (e.g., A1B2C3D4E5F6A7B8C9D0E1F2 and B1B2B3B4B5B6B7B8B9B0C1C2)
which is unconventional; to fix, remove these manual entries and re-add the
files using Xcode's File → Add Files to "damus"… (or close Xcode, run Xcode to
re-generate UUIDs) so Xcode will create standard random UUIDs for
GroupedTimelineGrouper.swift and GroupedModeQueueManager.swift and update the
PBXBuildFile references consistently with the rest of the project.
In `@damus/Features/NIP05/Models/NIP05.swift`:
- Around line 220-244: ordered_domain_authors currently uses NSMutableOrderedSet
which loses Swift type-safety; replace the Obj-C collection with a Swift-native
dedup by iterating the concatenated array (eventPubkeys + (filterAuthors ??
[])), maintaining a Set<Pubkey> seen to skip duplicates while building a new
[Pubkey] orderedUnique, then proceed to filter with
matches_domain(domain:profiles:) and the friend_filter branch
(contacts.is_in_friendosphere) unchanged; ensure Pubkey remains Hashable or use
an appropriate unique key when populating seen so the behavior and order match
the original.
- Around line 184-201: The friends-of-friends branch runs a potentially large
loop on the main actor (iterating contacts.get_friend_of_friends_list() and
calling matches_domain), which can stall the UI; move that work to a background
task similar to the .all case by performing the iteration inside Task.detached
and returning the authors set, or snapshot the minimal profile data
off-main-thread first and make matches_domain operate on that snapshot; update
the .friends_of_friends case to await the detached task and fall back to
validated_authors_for_domain if the result is empty.
In `@damus/Features/NIP05/Models/NIP05DomainEventsModel.swift`:
- Around line 82-132: Add missing Swift docstrings for the listed methods:
streamItems(), load_more(), fetch_older(), add_event(_:), set_friend_filter(_:),
and matches_domain(_:). For each method add a brief triple-slash (///) comment
immediately above the function that states its purpose, describes parameters and
return value (or that it returns Void), notes async behavior or side effects
(e.g., updates state.loading, mutates events, triggers network streams, or
validates NIP-05), and any important invariants (e.g., when since is set use
different limits, or that matches_domain checks NIP-05 domain matching). Follow
the repository’s existing docstring style (brief summary line, parameter tags if
used) and keep comments concise.
In `@damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift`:
- Around line 56-64: The computed property allAuthors and the synchronous
profile lookups (ndb.lookup_profile_and_copy invoked by notesFromAuthorsString)
run on the main actor and iterate model.events.all_events on every view
evaluation; move these expensive DB reads and the O(n) event scan off the main
thread by computing and caching the authors list in the model (e.g., add a
precomputed property or method on the view model that updates on events change)
or in the view using `@State` plus a .task to populate it, and perform profile
lookups from notesFromAuthorsString in the same background task/queue (or expose
an async lookup method on the model) so the view body only reads cheap,
precomputed data (refer to allAuthors, notesFromAuthorsString,
model.events.all_events, and ndb.lookup_profile_and_copy).
- Around line 101-126: Refactor the near-duplicate logic in
notesFromAuthorsString and friendsOfFriendsString into a single helper that
returns the localized summary string: extract the shared steps (profile lookup
via a provided lookup function, name extraction/truncation to 20 chars, and the
switch on authors.count that formats 0/1/2/3/+ cases) into a new function (e.g.,
summarizeAuthors(_:lookup:zeroCaseKey:ndb:locale:) or similar) and have both
notesFromAuthorsString and friendsOfFriendsString call it; parameterize the
helper with the zero-case localization key/string and a profile-lookup closure
(to accommodate ndb.lookup_profile_and_copy vs the inline logic), reuse
localizedStringFormat for the default case, and ensure you pass
Profile.displayName(...).username.truncate(maxLength: 20) as the name
transformation so behavior stays identical.
In `@damus/Features/NIP05/Views/NIP05DomainTimelineView.swift`:
- Around line 56-58: The computed property contentFilters currently creates a
new ContentFilters (and its closures) every time the view re-evaluates; change
it to a stored/cached value to avoid repeated allocation — for example,
initialize a stored property once (e.g., a private let or lazy var
contentFilters) or move it into the view's model/@StateObject so
ContentFilters.defaults(damus_state:) is called only once; update references to
use that stored contentFilters and ensure it is recalculated only when
damus_state actually changes.
In `@damus/Features/Timeline/Models/HomeModel.swift`:
- Around line 766-772: Add docstrings above the three properties describing
their purpose and usage: for backfillTask explain it is a Task<Void, Never>?
that performs relay backfills to fill older events for grouped mode and mention
its lifecycle/cancellation behavior; for ndbRefreshTask explain it is a
Task<Void, Never>? responsible for refreshing the ndb (or network DB)
cache/state and when it runs; for backfillPageLimit (UInt32) document that it is
the maximum number of events fetched per backfill page, include the unit and
default value (5000) and any constraints. Place the comments directly above the
declarations for backfillTask, ndbRefreshTask, and backfillPageLimit and keep
them concise.
- Around line 854-866: The current double-break pattern (a `break` inside the
`switch` for `.eose` and then `if case .eose = item { break }` after the switch)
is fragile; convert the surrounding `for await` loop into a labeled loop (e.g.,
timelineLoop:) and replace both the post-switch `if case .eose` check and the
inner switch `case .eose: break` behavior with a single `break timelineLoop`
from inside the `.eose` switch arm so that `lender.justUseACopy`,
`should_show_event(state: self.damus_state, ev:)`, and appending to `allEvents`
remain unchanged but loop exit is explicit and idiomatic.
- Around line 869-878: The loop is doing sequential main-actor work and then
uses a redundant DispatchQueue.main.async to set self.backfilling; since
HomeModel is `@MainActor` the DispatchQueue hop should be removed and set
backfilling directly (reference HomeModel and backfilling). To avoid UI hitches
when inserting large chunks, change the insertion loop around damus_state.events
/ favoriteEvents.insert(_:)/insert_home_event(_:) to perform batched inserts or
periodically yield to the executor (e.g., process N events then await
Task.yield() or insert in a batch) so the main actor can process UI work between
batches.
- Around line 885-914: The refreshSingleAuthorEvents function spawns a
fire-and-forget Task (via damus_state.nostrNetwork.reader.advancedStream)
without keeping a handle, causing concurrent streams to accumulate; change it to
store the Task in a cancellable property (e.g., singleAuthorRefreshTask) on the
HomeModel, cancel any existing task before creating a new one, and assign the
new Task to that property; ensure you cancel/cleanup this stored task where
other stored tasks are handled (similar to backfillTask and ndbRefreshTask) and
on deinit or when the stream should stop.
In `@damus/Features/Timeline/Views/PostingTimelineView.swift`:
- Around line 256-272: Add docstrings for the three private helper methods to
satisfy docstring coverage: document flushActiveSource() (annotated `@MainActor`)
with a one-line summary of what it flushes and any threading/actor expectations,
document backfillForGroupedMode(showLoading:) describing the purpose, the
meaning and default of the showLoading parameter, how the since timestamp is
computed and that it calls home.backfillForGroupedMode, and document
refreshFromNdb() summarizing the since computation and that it refreshes grouped
data from NDB; keep each docstring concise and follow existing project style
(one-line summary + brief param/side-effect notes).
In `@damus/Shared/Components/GroupedTimeline/GroupedListView.swift`:
- Around line 29-35: GroupedTimelineGrouper.group(...) is being called
synchronously inside GroupedListView.body (using events.all_events, filter,
settings.filterValues, Date()), causing an O(n) regroup on every SwiftUI body
recompute; move this heavy computation out of body into a stored state or
view-model property (e.g., `@State` or `@StateObject/`@ObservedObject) such as a
`grouped` property on GroupedListView or its VM, and update that property only
when inputs change by using .onChange(of:) for events.all_events, filter, and
settings.filterValues (or subscribe to events' publisher via Combine) so renders
that don't change those inputs skip the grouping work.
In `@damus/Shared/Components/GroupedTimeline/GroupedTimelineGrouper.swift`:
- Around line 91-95: Add missing docstrings for the internal filtering helpers
parseFilteredWords, shouldIncludeEvent, containsFilteredWords, and isTooShort:
for each function add a brief Swift doc comment that states the purpose,
describes parameters and types, explains return value, and any important
behavior or side‑effects (e.g., trimming, lowercasing, minimum length). Place
the comments immediately above the function declarations so they follow project
docstring style and cover the functions at/around parseFilteredWords,
shouldIncludeEvent, containsFilteredWords, and isTooShort.
Comment on lines
+108
to
+111
| .padding(.leading, 45) // Align with domain title (after back button width) | ||
| } | ||
| .padding(.horizontal, 16) | ||
| .padding(.top, 50) // Account for status bar / notch |
Contributor
There was a problem hiding this comment.
Hard-coded top padding is fragile across device form factors.
.padding(.top, 50) assumes a fixed status bar / notch height. Devices with Dynamic Island, classic notch, or no notch will have different safe area insets. Similarly, .padding(.leading, 45) on line 108 couples to the back-button width.
Consider using GeometryReader with safeAreaInsets.top for the status bar offset, and computing the leading alignment from the actual button frame rather than a magic number.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus/Features/NIP05/Views/NIP05DomainTimelineView.swift` around lines 108 -
111, NIP05DomainTimelineView uses hard-coded paddings (.padding(.top, 50) and
.padding(.leading, 45)); replace them with safe-area-aware and measured values:
wrap the view in a GeometryReader and use proxy.safeAreaInsets.top (or
UIApplication.shared.windows.first?.safeAreaInsets.top) instead of the constant
50 for the top offset, and replace the leading magic number by measuring the
back button's frame via a custom PreferenceKey/AnchorPreference (attach a
background GeometryReader to the back button to publish its maxX) and apply that
measured value as the leading padding/alignment; update NIP05DomainTimelineView
to consume those values and remove the hard-coded constants.
Comment on lines
+224
to
+254
| private var groupedContentView: some View { | ||
| let eventsSource = timeline_source == .favorites | ||
| ? home.favoriteEvents : home.events | ||
| return ScrollView { | ||
| GroupedListView( | ||
| damus_state: damus_state, | ||
| events: eventsSource, | ||
| filter: content_filter(.posts_and_replies), | ||
| settings: groupedFilterSettings, | ||
| onProfileTapped: { pubkey in | ||
| lastVisitedGroupedPubkey = pubkey | ||
| } | ||
| ) | ||
| .redacted(reason: home.backfilling ? .placeholder : []) | ||
| .shimmer(home.backfilling) | ||
| .padding(.top, headerHeight) | ||
| } | ||
| .onAppear { | ||
| // When returning from a profile, do a targeted refresh for just that | ||
| // author. Single-author queries avoid the dilution problem of querying | ||
| // all 500+ friends at once, ensuring the author's full event history | ||
| // within the time window is fetched. | ||
| if let pubkey = lastVisitedGroupedPubkey { | ||
| lastVisitedGroupedPubkey = nil | ||
| let since = UInt32(Date().timeIntervalSince1970) - groupedFilterSettings.timeRange.seconds | ||
| home.refreshSingleAuthorEvents(pubkey: pubkey, since: since, source: timeline_source) | ||
| } else if !home.backfilling { | ||
| refreshFromNdb() | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find GroupedListView and GroupedTimelineGrouper
fd -e swift | xargs rg -l "GroupedListView\|GroupedTimelineGrouper" | head -20Repository: damus-io/damus
Length of output: 798
🏁 Script executed:
# Search for GroupedListView definition
rg -n "struct GroupedListView\|class GroupedListView" --type=swift -A 30Repository: damus-io/damus
Length of output: 40
🏁 Script executed:
# Search for GroupedTimelineGrouper.group method
rg -n "func group\(" --type=swift -B 2 -A 15 | grep -A 15 "GroupedTimelineGrouper"Repository: damus-io/damus
Length of output: 2176
🏁 Script executed:
# Check PostingTimelineView for docstrings on new methods
rg -n "private var groupedContentView\|private func flushActiveSource\|private func backfillForGroupedMode\|private func refreshFromNdb" --type=swift damus/Features/Timeline/Views/PostingTimelineView.swift -A 2Repository: damus-io/damus
Length of output: 40
🏁 Script executed:
# Find all Swift files in GroupedTimeline directory
find damus -type f -name "*GroupedTimeline*" -o -name "*GroupedList*" | head -20Repository: damus-io/damus
Length of output: 188
🏁 Script executed:
# Look for GroupedListView definition
rg -n "GroupedListView" --type=swift -B 2 -A 20 | head -100Repository: damus-io/damus
Length of output: 9237
🏁 Script executed:
# Check the full GroupedTimelineGrouper implementation
cat damus/Shared/Components/GroupedTimeline/GroupedTimelineGrouper.swiftRepository: damus-io/damus
Length of output: 4937
🏁 Script executed:
# Check PostingTimelineView.swift for the specific methods without docstrings
sed -n '220,280p' damus/Features/Timeline/Views/PostingTimelineView.swiftRepository: damus-io/damus
Length of output: 2449
Add missing docstrings and optimize grouping recomputation.
Two issues in the new grouped mode methods:
-
Missing docstrings: The private methods
groupedContentView,flushActiveSource(),backfillForGroupedMode(), andrefreshFromNdb()lack docstrings. Per coding guidelines, any code added or modified must have documentation. -
Grouping recalculated on every render:
GroupedListView.bodycallsGroupedTimelineGrouper.group(events:filter:values:now:)on every SwiftUI render cycle. With large event sets from 500+ followed authors, this O(n) pass can cause performance issues during scrolling. Consider memoizing the grouped result in a@Stateproperty, recomputing only whenevents.all_events,settings.filterValues, orfilteractually change.
🤖 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 224 -
254, The file is missing docstrings for private helpers and is recomputing
grouping on every SwiftUI render; add concise doc comments to
groupedContentView, flushActiveSource(), backfillForGroupedMode(), and
refreshFromNdb() per project guidelines, and avoid the O(n) grouping in
GroupedListView.body by memoizing the grouped result in PostingTimelineView: add
a `@State` (or `@StateObject`) property to hold the grouped/timestamped groups,
compute it only when events.all_events, groupedFilterSettings.filterValues, or
the passed filter change (use onChange, .task, or a Combine publisher to update
the `@State`), then pass the memoized grouped data into GroupedListView instead of
letting GroupedListView call GroupedTimelineGrouper.group on every render (refer
to groupedContentView, GroupedListView, GroupedTimelineGrouper.group, and
groupedFilterSettings to locate the spots to change).
Comment on lines
+62
to
+75
| private var emptyStateView: some View { | ||
| VStack(spacing: 8) { | ||
| Text("No matching posts") | ||
| .font(.headline) | ||
| .foregroundColor(.gray) | ||
|
|
||
| if hasActiveFilters { | ||
| Text("Try adjusting your filters") | ||
| .font(.subheadline) | ||
| .foregroundColor(.secondary) | ||
| } | ||
| } | ||
| .padding(.top, 40) | ||
| } |
Contributor
There was a problem hiding this comment.
Localize empty-state strings.
"No matching posts" and "Try adjusting your filters" are user-facing strings that should use NSLocalizedString (or SwiftUI's LocalizedStringKey) for i18n consistency with the rest of the codebase.
🌍 Proposed fix
- Text("No matching posts")
+ Text("No matching posts", comment: "Empty state message when no grouped posts match filters")
.font(.headline)
.foregroundColor(.gray)
if hasActiveFilters {
- Text("Try adjusting your filters")
+ Text("Try adjusting your filters", comment: "Hint shown when active filters produce no results")
.font(.subheadline)
.foregroundColor(.secondary)
}📝 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
| private var emptyStateView: some View { | |
| VStack(spacing: 8) { | |
| Text("No matching posts") | |
| .font(.headline) | |
| .foregroundColor(.gray) | |
| if hasActiveFilters { | |
| Text("Try adjusting your filters") | |
| .font(.subheadline) | |
| .foregroundColor(.secondary) | |
| } | |
| } | |
| .padding(.top, 40) | |
| } | |
| private var emptyStateView: some View { | |
| VStack(spacing: 8) { | |
| Text("No matching posts", comment: "Empty state message when no grouped posts match filters") | |
| .font(.headline) | |
| .foregroundColor(.gray) | |
| if hasActiveFilters { | |
| Text("Try adjusting your filters", comment: "Hint shown when active filters produce no results") | |
| .font(.subheadline) | |
| .foregroundColor(.secondary) | |
| } | |
| } | |
| .padding(.top, 40) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus/Shared/Components/GroupedTimeline/GroupedListView.swift` around lines
62 - 75, The empty-state view is using hardcoded English strings; update the
Text initializers in the emptyStateView to use localized keys instead (e.g.,
replace "No matching posts" and "Try adjusting your filters" with
NSLocalizedString or SwiftUI LocalizedStringKey usages) so they integrate with
the app’s i18n system; ensure the conditional message that depends on
hasActiveFilters continues to show the localized secondary string and keep the
same font/foreground modifiers on the localized Text views.
Comment on lines
+118
to
+138
| static func isTooShort(_ event: NostrEvent, hideShortNotes: Bool) -> Bool { | ||
| guard hideShortNotes else { return false } | ||
|
|
||
| let content = event.content.trimmingCharacters(in: .whitespacesAndNewlines) | ||
|
|
||
| if content.count < 10 { return true } | ||
|
|
||
| let textWithoutEmojis = content.unicodeScalars | ||
| .filter { !$0.properties.isEmoji } | ||
| .map { String($0) } | ||
| .joined() | ||
| .replacingOccurrences(of: " ", with: "") | ||
| .trimmingCharacters(in: .whitespacesAndNewlines) | ||
|
|
||
| if textWithoutEmojis.count < 2 { return true } | ||
|
|
||
| let words = content.split(whereSeparator: { $0.isWhitespace }).filter { !$0.isEmpty } | ||
| if words.count == 1 { return true } | ||
|
|
||
| return false | ||
| } |
Contributor
There was a problem hiding this comment.
Single-word notes ≥10 chars are filtered as "too short".
Line 135 rejects any content with exactly one word regardless of length. A note like "cryptocurrency" (14 chars, clearly non-trivial) or a single long hashtag would be hidden. Consider removing the single-word check, or only applying it to very short single words:
🛡️ Possible adjustment
let words = content.split(whereSeparator: { $0.isWhitespace }).filter { !$0.isEmpty }
- if words.count == 1 { return true }
+ if words.count == 1 && content.count < 20 { return true }📝 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
| static func isTooShort(_ event: NostrEvent, hideShortNotes: Bool) -> Bool { | |
| guard hideShortNotes else { return false } | |
| let content = event.content.trimmingCharacters(in: .whitespacesAndNewlines) | |
| if content.count < 10 { return true } | |
| let textWithoutEmojis = content.unicodeScalars | |
| .filter { !$0.properties.isEmoji } | |
| .map { String($0) } | |
| .joined() | |
| .replacingOccurrences(of: " ", with: "") | |
| .trimmingCharacters(in: .whitespacesAndNewlines) | |
| if textWithoutEmojis.count < 2 { return true } | |
| let words = content.split(whereSeparator: { $0.isWhitespace }).filter { !$0.isEmpty } | |
| if words.count == 1 { return true } | |
| return false | |
| } | |
| static func isTooShort(_ event: NostrEvent, hideShortNotes: Bool) -> Bool { | |
| guard hideShortNotes else { return false } | |
| let content = event.content.trimmingCharacters(in: .whitespacesAndNewlines) | |
| if content.count < 10 { return true } | |
| let textWithoutEmojis = content.unicodeScalars | |
| .filter { !$0.properties.isEmoji } | |
| .map { String($0) } | |
| .joined() | |
| .replacingOccurrences(of: " ", with: "") | |
| .trimmingCharacters(in: .whitespacesAndNewlines) | |
| if textWithoutEmojis.count < 2 { return true } | |
| let words = content.split(whereSeparator: { $0.isWhitespace }).filter { !$0.isEmpty } | |
| if words.count == 1 && content.count < 20 { return true } | |
| return false | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus/Shared/Components/GroupedTimeline/GroupedTimelineGrouper.swift` around
lines 118 - 138, The isTooShort(_ event: NostrEvent, hideShortNotes: Bool)
function currently rejects any single-word note via the words.count == 1 check,
which hides long single-token posts; change that check so it only flags very
short single-word notes (e.g., require words.count == 1 && content.count < 10)
or remove the single-word check entirely—update the condition using the existing
content/words variables in isTooShort to preserve long single-word notes like
long hashtags.
651a654 to
a0bc7e2
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift (1)
163-163:⚠️ Potential issue | 🟡 MinorHardcoded English string not wrapped in
NSLocalizedString.The
case 0:path infriendsOfFriendsStringreturns a bare string literal while all other cases useNSLocalizedString. This string will not be translated.🐛 Proposed fix
- case 0: - return "No one in your trusted network is associated with this domain." + case 0: + return NSLocalizedString( + "No one in your trusted network is associated with this domain.", + bundle: bundle, + comment: "Text when no one in the trusted network is associated with this NIP-05 domain." + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift` at line 163, The string returned in friendsOfFriendsString's `case 0:` is hardcoded; replace it with an NSLocalizedString call consistent with the other cases (e.g. NSLocalizedString("No one in your trusted network is associated with this domain.", comment: "")) so the message is localizable, keeping the exact message text and comment format used elsewhere in NIP05DomainTimelineHeaderView.
🧹 Nitpick comments (7)
damus/Shared/Components/GroupedTimeline/GroupedAuthorRow.swift (1)
68-85:eventPreviewTextdoesn't needself— could be a free function orstatic.The method doesn't reference any instance properties. Minor nit, but making it
staticor a module-level helper would clarify that and make it reusable outside this view.More importantly, the regex
nostr:[a-zA-Z0-9]+won't strip references containing underscores or other characters that could appear in NIP-19 TLV-encoded identifiers (e.g.,nprofile,neventbech32 strings). In practice bech32 only uses[a-z0-9], so uppercase is over-broad but harmless, and the pattern should suffice for display-preview stripping.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Shared/Components/GroupedTimeline/GroupedAuthorRow.swift` around lines 68 - 85, Make eventPreviewText a static (or free) function since it doesn't use instance state: change func eventPreviewText(_ event: NostrEvent) to static func eventPreviewText(_ event: NostrEvent) (or move it out of the type) and update references accordingly; also broaden the regex that strips nostr identifiers to include bech32-like characters and underscores by replacing "nostr:[a-zA-Z0-9]+" with something like "nostr:[a-z0-9_]+" (or the precise bech32 charset) and use appropriate regex options (no need for uppercase A-Z) so nprofile/nevent bech32 references are removed correctly.damus/Features/NIP05/Models/NIP05.swift (1)
220-244:ordered_domain_authorsusesNSMutableOrderedSetfor deduplication.This works correctly but is somewhat non-idiomatic Swift. The
as? [Pubkey] ?? []fallback silently produces an empty array if the cast fails (which shouldn't happen in practice, but masks bugs if it does). Consider a Swift-native ordered-dedup approach for clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Models/NIP05.swift` around lines 220 - 244, The function ordered_domain_authors currently uses NSMutableOrderedSet and an unsafe cast (orderedUnique = NSMutableOrderedSet(array: pubkeys).array as? [Pubkey] ?? []) which can silently yield an empty array; replace this with a Swift-native ordered dedup: iterate over pubkeys, keep a Set of seen Pubkey (or seen identifiers if Pubkey is not Hashable) and append each pk to a new array only if not already seen, then use that deduped array for matching; update references to pubkeys and orderedUnique in ordered_domain_authors and ensure Pubkey is Hashable or use a stable unique key (e.g., hex string) for the seen Set.damus/Features/Timeline/Models/HomeModel.swift (2)
845-866: Convoluted double-break to exitfor awaiton EOSE.The
case .eose: breakon line 860 breaks theswitch, then line 865 re-checks the same item tobreakthefor awaitloop. This works but is unnecessarily confusing.A labeled loop is cleaner and idiomatic:
♻️ Proposed simplification
- for await item in damus_state.nostrNetwork.reader.advancedStream(filters: [chunkFilter], streamMode: .ndbAndNetworkParallel(networkOptimization: nil)) { - switch item { - case .event(let lender): - await lender.justUseACopy({ ev in - guard should_show_event(state: self.damus_state, ev: ev) else { return } - allEvents.append(ev) - }) - case .eose: - break - case .ndbEose, .networkEose: - continue - } - if case .eose = item { break } - } + chunkStream: for await item in damus_state.nostrNetwork.reader.advancedStream(filters: [chunkFilter], streamMode: .ndbAndNetworkParallel(networkOptimization: nil)) { + switch item { + case .event(let lender): + await lender.justUseACopy({ ev in + guard should_show_event(state: self.damus_state, ev: ev) else { return } + allEvents.append(ev) + }) + case .eose: + break chunkStream + case .ndbEose, .networkEose: + continue + } + }🤖 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 845 - 866, The code uses a confusing double-break to exit the `for await` loop over `damus_state.nostrNetwork.reader.advancedStream` by breaking the `switch` on `.eose` and then separately checking `if case .eose = item { break }`; replace this with a single labeled loop to clearly break out from inside the `switch`: give the `for await` loop a label (e.g., outerLoop) and in `case .eose` perform `break outerLoop`, removing the later `if case .eose = item { break }` check; apply this change around the loop handling in HomeModel (the `for chunk in chunks { ... for await item in ... { switch item { ... } } }` block) so the intent to exit the stream on EOSE is explicit and not duplicated.
885-914: Fire-and-forget Task with no cancellation handle.Unlike
backfillForGroupedModeandrefreshGroupedFromNdb, this method creates an untrackedTaskwith no stored handle. Rapid calls (e.g., user scrolling through grouped rows) will spawn concurrent network subscriptions that cannot be cancelled, potentially piling up relay requests.Store a task handle and cancel the previous one, consistent with the pattern used by the sibling methods.
♻️ Proposed fix
Add a private task handle:
private var ndbRefreshTask: Task<Void, Never>? +private var singleAuthorRefreshTask: Task<Void, Never>?Then use it:
func refreshSingleAuthorEvents(pubkey: Pubkey, since: UInt32, source: TimelineSource) { + singleAuthorRefreshTask?.cancel() let kinds: [NostrKind] = [.text, .longform, .boost, .highlight] let isFavorites = source == .favorites ... - Task { + singleAuthorRefreshTask = Task { for await item in ...🤖 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 885 - 914, The refreshSingleAuthorEvents function spawns an untracked Task; instead add a private Task? property (e.g., refreshSingleAuthorTask) on HomeModel, cancel any existing task before creating a new one, assign the new Task to that property, run the existing advancedStream logic inside that Task, and ensure you clear the handle (set to nil) when the stream returns or finishes (EOSE/return) so it can be cancelled next time—follow the same cancel-then-replace pattern used by backfillForGroupedMode and refreshGroupedFromNdb and reference damus_state.nostrNetwork.reader.advancedStream and the refreshSingleAuthorEvents method when updating the code.damus/Shared/Components/GroupedTimeline/GroupedTimelineGrouper.swift (1)
91-95: Internal helper methods lack docstrings.
parseFilteredWords,shouldIncludeEvent,containsFilteredWords, andisTooShortare all undocumented. While thegroupmethod has a docstring, these supporting methods would benefit from brief descriptions of their purpose and return semantics.As per coding guidelines, "Ensure docstring coverage for any code added or modified."
Also applies to: 97-110
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Shared/Components/GroupedTimeline/GroupedTimelineGrouper.swift` around lines 91 - 95, Add Swift doc comments (///) for the helper methods parseFilteredWords(_:), shouldIncludeEvent(_:), containsFilteredWords(_:filteredWords:), and isTooShort(_:) describing each method's purpose, parameters and return semantics; for example, state that parseFilteredWords takes a comma-separated raw string and returns a lowercased array of trimmed words (minimum length 2), shouldIncludeEvent decides whether an event belongs in a group and what boolean/criteria it returns, containsFilteredWords checks if a given text contains any of the parsed filtered words, and isTooShort determines if an event's text is too short to include; follow the same docstring style used on group(_:), keep comments concise, and ensure parameter names and return types are documented.damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift (2)
101-126:ndb.lookup_profile_and_copyis called from the SwiftUIbodyon the main thread.
notesFromAuthorsStringis invoked at Line 74 insidebody. It performs up to threendb.lookup_profile_and_copydatabase reads per render. While NDB reads are typically fast (memory-mapped), calling database queries insidebodyis not idiomatic and fires on every@ObservedObjectchange — which can be frequent as events stream in.Consider computing the string in a
@Stateor atask(id:)modifier that runs off the hot render path, or at minimum cache the result with aCombinepipeline attached tomodel.$events.As per coding guidelines: "All network requests, database queries, and expensive computations must run on background threads/queues."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift` around lines 101 - 126, notesFromAuthorsString currently calls ndb.lookup_profile_and_copy from the SwiftUI body (via its call site at line 74), triggering synchronous DB reads on the main thread on every render; move this work off the hot render path by computing and caching the resulting string asynchronously (e.g., create a `@State` String property like notesFromAuthors, populate it in a Task/Task.detached or a task(id: model.$events) modifier that performs ndb.lookup_profile_and_copy on a background queue, or subscribe to model.$events with Combine to update the cached value), and have body simply read the cached `@State` value; update any callers to use the cached variable instead of directly invoking notesFromAuthorsString and ensure thread-safe access to Ndb by performing all lookups off the main thread.
58-64:allAuthorsis re-evaluated on everybodyrender.Each body pass allocates two arrays (
eventAuthors,filterAuthors) and aSet<Pubkey>for deduplication. As the model streams events,@ObservedObjecttriggers frequent re-renders. For large event lists this accumulates avoidable allocations.Memoising via a stored property updated in
add_event(or via a Combine sink) would remove the per-render allocation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift` around lines 58 - 64, The computed property allAuthors currently rebuilds eventAuthors, filterAuthors, and a deduplication Set on every SwiftUI body render; change this to a stored cached authors collection on NIP05DomainTimelineHeaderView that is incrementally updated instead: add a stored var (e.g., cachedAuthors: [Pubkey] or Set<Pubkey>) and update it when new events arrive by handling model.events.all_events changes (hook into add_event or attach a Combine sink to the model's events publisher) and also merge model.filter.authors when the filter changes; then replace the allAuthors computed property to return the cached value to avoid per-render allocations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.beads/issues.jsonl:
- Line 5: The JSON entry with "id":"damus-ir1" is missing the created_by field;
update that object in .beads/issues.jsonl to include a created_by key with the
same format used by other entries (e.g., a string or user identifier), ensuring
the schema matches other records so downstream tooling can rely on a consistent
field presence.
In `@damus.xcodeproj/project.pbxproj`:
- Around line 1976-1981: The PBX project contains hand-crafted sequential UUIDs
for new GroupedTimeline entries (e.g., PBXBuildFile entries referencing
GroupedTimelineGrouper.swift and GroupedModeQueueManager.swift, PBXFileReference
entries, and the GroupedTimeline PBXGroup) which risks collisions and
corruption; remove all manually-added GroupedTimeline-related PBXBuildFile,
PBXFileReference and PBXGroup entries (the ones using patterns like B1B2B3...,
C1C2C3..., A1B2C3..., F2E1D0..., A1A2A3...) from the project.pbxproj and then
re-add the six source files and two test files using Xcode’s “Add Files to…”
dialog so Xcode will generate proper random 24-hex UUIDs and correct attributes
(includeInIndex, etc.). Ensure you delete both the duplicate build-file entries
and their matching fileRef PBXFileReference and PBXGroup nodes before re-adding.
- Around line 6379-6384: The six GroupedTimeline source files
(GroupedFilterButton.swift, GroupedFilterSettingsView.swift,
GroupedAuthorRow.swift, GroupedListView.swift, GroupedTimelineGrouper.swift,
GroupedModeQueueManager.swift) are currently assigned to the ShareExtension and
HighlighterActionExtension targets; open the project.pbxproj and remove their
file reference entries (e.g., CB20AABE3C7B960961C0F914,
CF86249414FD714F8E45AA55, D93D3D7A501A49D1F86669A3, 91E25E8CB81B797E7FCC4D25,
B1B2B3B4B5B6B7B8B9B0C1C2, C1C2C3C4C5C6C7C8C9C0D1D2) from the PBXNativeTarget
build phase sections for ShareExtension and HighlighterActionExtension so they
remain only in the main damus app target; ensure you only remove the target
membership entries and do not delete the file references themselves from the
project if the main app still needs them.
In `@damus/Features/NIP05/Models/NIP05.swift`:
- Around line 180-204: The .friends_of_friends branch in authors_for_domain
blocks the MainActor by calling matches_domain (which does ndb.lookup_profile
via withNdb) for each pubkey; move the entire iteration and domain-matching work
into a background task (use Task.detached) similar to the .all branch, perform
profile lookup and NIP-05 parsing there (either by inlining the logic instead of
calling `@MainActor` matches_domain or by refactoring matches_domain to accept
off-main-thread inputs), collect the filtered Set<Pubkey> inside the detached
task, then await and return the result on the MainActor (e.g. via await
Task.detached { ... }.value and if needed await MainActor.run { authors = result
}) so the heavy loop no longer runs on authors_for_domain's MainActor context;
keep validated_authors_for_domain and scan_all_cached_profiles fallback logic
unchanged.
In `@damus/Features/NIP05/Models/NIP05DomainEventsModel.swift`:
- Around line 69-76: set_friend_filter currently calls subscribe(resetEvents:
false) which causes subscribe to set self.since = since (nil) and drop any
active time-window; change the call so subscribe preserves the existing since
value when toggling filters — either by calling subscribe(resetEvents: false,
since: self.since) or by modifying subscribe to only overwrite self.since when
an explicit non-nil since is provided; update the call in set_friend_filter
and/or subscribe implementation to ensure the grouped 24h/7d since window is
retained across friend_filter changes.
- Around line 83-90: The grouped-mode hardcoded filter.limit = 5000 can cause
memory/relay pressure; change this to a smaller configurable constant (e.g.
GROUPED_MODE_LIMIT = 1000) or a stored property on NIP05DomainEventsModel and
use that instead of the literal, and ensure the grouped view uses load_more() to
fetch more history if needed; update the block that sets filter.since and
filter.limit (the since/used_initial_page/self.limit/self.initial_limit logic)
to reference the new constant/property so the limit is tunable rather than a
magic 5000.
- Around line 134-143: Add triple-slash docstrings for the three new methods:
load_more(), fetch_older(until:), and add_event(_:). For each, provide a
one-line summary of purpose, brief description of parameters (e.g., until:
cutoff timestamp for fetch_older, event param for add_event), mention
concurrency/side-effects (that load_more/ fetch_older run async Tasks and mutate
loading_more/ events), and any return/throw behavior (if none, state “no return
value”). Place the docstrings directly above the corresponding function
declarations (load_more, fetch_older, add_event) using the /// style to satisfy
the coding guidelines.
- Around line 145-173: fetch_older can exit the for-await loop without receiving
an .eose (e.g. timeout, disconnect, cancellation) leaving has_more=true and
causing repeated empty fetches; after the loop completes normally ensure you set
has_more = false (and keep loading_more = false) so the UI won't repeatedly
retry—update the end of fetch_older (after the for-await over
state.nostrNetwork.reader.advancedStream) to set has_more = false (optionally
also update last_loaded_count if you want to prevent future redundant work)
before returning.
In `@damus/Features/NIP05/Views/NIP05DomainTimelineView.swift`:
- Around line 14-27: Remove the unused NIP05ViewMode enum and its localizedTitle
property: locate the enum declaration named NIP05ViewMode and delete it
(including the cases .grouped and .timeline and the var localizedTitle), and
ensure there are no remaining references—note the view currently toggles modes
via filterSettings.enableGroupedMode, so update any references that might have
intended to use NIP05ViewMode to rely on filterSettings.enableGroupedMode
instead if necessary.
In `@damus/Features/Timeline/Models/HomeModel.swift`:
- Around line 869-878: The loop in backfillForGroupedMode causes up to 5000
main-actor switches and unsynchronized access to EventCache: collect allEvents
(or build two arrays: favoritesBatch and homeBatch) on the background task, then
perform a single await MainActor.run { ... } (or call one `@MainActor` method) to
1) insert the batch into favoriteEvents via favoriteEvents.insert(contentsOf:)
or similar and 2) call insert_home_event for the batch; also move the
damus_state.events.insert calls into that same main-actor block (or protect
EventCache by annotating EventCache as `@MainActor` or serializing its mutations
on a dedicated queue) so EventCache mutations are done safely and context
switches are minimized.
In `@damus/Features/Timeline/Views/PostingTimelineView.swift`:
- Around line 179-195: The onAppear setting of
groupedFilterSettings.enableGroupedMode can trigger onChange which calls
flushActiveSource() and backfillForGroupedMode(), and groupedContentView's
refreshFromNdb() may also run causing duplicate work; to fix, add a guard so
refreshFromNdb() checks the same in-flight/backfilling state (e.g., use
home.backfilling or a new isBackfilling flag) and returns early if a grouped
backfill is already running, or alternatively suppress the onChange trigger
during initial onAppear by comparing old vs new and skipping the first synthetic
change; update the logic around groupedFilterSettings.enableGroupedMode,
groupedModeStored, flushActiveSource(), backfillForGroupedMode(), and
groupedContentView.refreshFromNdb() to use the chosen guard so backfill work
runs only once.
---
Outside diff comments:
In `@damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift`:
- Line 163: The string returned in friendsOfFriendsString's `case 0:` is
hardcoded; replace it with an NSLocalizedString call consistent with the other
cases (e.g. NSLocalizedString("No one in your trusted network is associated with
this domain.", comment: "")) so the message is localizable, keeping the exact
message text and comment format used elsewhere in NIP05DomainTimelineHeaderView.
---
Duplicate comments:
In `@damus.xcodeproj/project.pbxproj`:
- Line 2040: The PBXFileReference entries for the new GroupedTimeline source
files (e.g., GroupedFilterButton.swift and the other GroupedTimeline .swift
entries) incorrectly include the non-standard includeInIndex = 1 attribute;
remove that attribute from those PBXFileReference blocks in the project.pbxproj
(or re-add the affected files via Xcode so Xcode regenerates the references
without includeInIndex). Locate the PBXFileReference entries referencing
GroupedFilterButton.swift and the other GroupedTimeline filenames and either
delete the includeInIndex = 1 line from each block or recreate the file
references through Xcode to resolve the inconsistency.
In `@damus/Features/NIP05/Models/NIP05DomainEventsModel.swift`:
- Around line 44-61: In subscribe(resetEvents:since:) the pagination flags
used_initial_page, has_more, and loading_more are being reset unconditionally
which causes duplicate fetches when resetEvents is false; change the logic in
the subscribe method (function subscribe) so that events.reset() and the
pagination state resets (used_initial_page, has_more, loading_more, and any
other initial pagination counters like last_loaded_count if intended) are
performed only inside the if resetEvents { ... } branch, leaving the else {
events.flush() } branch to preserve pagination state when resetEvents is false;
keep loadingTask, loading, and assigning self.since outside the conditional as
currently done.
In `@damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift`:
- Around line 80-88: The VStack's onTapGesture handler (the block that calls
damus_state.nav.push(route: Route.NIP05DomainPubkeys(domain: model.domain,
nip05_domain_favicon: nip05_domain_favicon, pubkeys: authors))) is not exposed
as an accessible control; replace the gesture with an actual Button (wrapping
the VStack content) or add .accessibilityAddTraits(.isButton) to the tappable
view so VoiceOver announces it as interactive, preserving the guard
!authors.isEmpty check and the same navigation call.
In `@damus/Features/NIP05/Views/NIP05DomainTimelineView.swift`:
- Around line 188-192: handleOnAppear currently skips calling model.subscribe if
model.events.all_events is non-empty, which leaves the relay closed after
handleOnDisappear/unsubscribe; remove that guard and always call model.subscribe
when the view appears, but preserve cached events by passing resetEvents: false
(i.e. call model.subscribe(since: groupedSince, resetEvents: false)) so the
relay connection is restored without clearing existing events; this mirrors the
approach used in filterSettingsSheet and ensures re-subscription after
navigation.
- Around line 108-111: The hard-coded .padding(.leading, 45) and .padding(.top,
50) in NIP05DomainTimelineView are brittle; replace them with values derived
from the real back-button width and the view's safe-area inset. Measure the back
button frame (e.g., add a width preference using a custom PreferenceKey on the
BackButton view and read it into a `@State` like backButtonWidth) and use that
state to compute the leading padding, and read the safe area top inset from a
GeometryReader (or the environment safeAreaInsets) to compute the top padding
instead of 50. Update the view modifiers in NIP05DomainTimelineView to use these
dynamic values so alignment adapts to Dynamic Island/notch/device variations.
In `@damus/Features/Timeline/Views/PostingTimelineView.swift`:
- Around line 224-272: Add concise Swift docstrings for the private grouped-mode
members: groupedContentView, flushActiveSource(),
backfillForGroupedMode(showLoading:), and refreshFromNdb(). For each, add a ///
comment immediately above the declaration describing what the property or method
does, its important side effects (e.g. triggers refresh/backfill, clears
lastVisitedGroupedPubkey, uses groupedFilterSettings.timeRange), any parameters
(e.g. showLoading: Bool) and concurrency/actor constraints where relevant (mark
flushActiveSource with its `@MainActor` behavior). Keep comments brief,
imperative, and consistent with project doc style.
In `@damus/Shared/Components/GroupedTimeline/GroupedAuthorRow.swift`:
- Around line 50-65: Add a concise docstring above the computed property
postCountBadge describing its purpose and behavior: state that it returns a
SwiftUI View showing the author's post count as a capsule-styled badge, note how
font size and horizontal padding change when postCount is >= 100, and mention
any important visual choices (foreground/background colors and min frame). Put
the docstring immediately above the postCountBadge declaration in
GroupedAuthorRow (GroupedAuthorRow.swift) so it documents the property for
future readers and satisfies docstring coverage.
In `@damus/Shared/Components/GroupedTimeline/GroupedTimelineGrouper.swift`:
- Around line 125-128: The current filtering that builds textWithoutEmojis uses
content.unicodeScalars.filter { !$0.properties.isEmoji } which also strips
digits and symbols; change the predicate to check isEmojiPresentation instead so
only glyphs explicitly emoji-presented are removed (i.e., replace use of isEmoji
with isEmojiPresentation when building textWithoutEmojis), preserving numeric
and symbol characters for the "too short" check in GroupedTimelineGrouper.
- Around line 58-61: The subtraction for cutoff can underflow: when computing
cutoff in GroupedTimelineGrouper (the function returning [AuthorGroup] that
calls parseFilteredWords), ensure you perform a safe subtraction between
now.timeIntervalSince1970 and values.timeRangeSeconds by doing the math in a
signed type or by checking bounds and clamping to zero before converting to
UInt32; e.g., compute a Double or Int64 delta = now.timeIntervalSince1970 -
Double(values.timeRangeSeconds) and then set cutoff = UInt32(max(0, delta)) (or
guard that now >= timeRangeSeconds and use 0 as the fallback) so the subtraction
cannot trap.
- Around line 134-135: The single-word rejection unconditionally drops notes if
words.count == 1; change this to only filter out single-word notes that are
shorter than the minimum length check used earlier (use the same threshold
referenced around line 123), e.g. replace the unconditional "if words.count == 1
{ return true }" with a check that returns true only when words.count == 1 &&
content.count (or words[0].count) < minLengthThreshold; locate this in
GroupedTimelineGrouper where `content` and `words` are computed and ensure you
reuse the same min length constant/variable used by the earlier length check.
---
Nitpick comments:
In `@damus/Features/NIP05/Models/NIP05.swift`:
- Around line 220-244: The function ordered_domain_authors currently uses
NSMutableOrderedSet and an unsafe cast (orderedUnique =
NSMutableOrderedSet(array: pubkeys).array as? [Pubkey] ?? []) which can silently
yield an empty array; replace this with a Swift-native ordered dedup: iterate
over pubkeys, keep a Set of seen Pubkey (or seen identifiers if Pubkey is not
Hashable) and append each pk to a new array only if not already seen, then use
that deduped array for matching; update references to pubkeys and orderedUnique
in ordered_domain_authors and ensure Pubkey is Hashable or use a stable unique
key (e.g., hex string) for the seen Set.
In `@damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift`:
- Around line 101-126: notesFromAuthorsString currently calls
ndb.lookup_profile_and_copy from the SwiftUI body (via its call site at line
74), triggering synchronous DB reads on the main thread on every render; move
this work off the hot render path by computing and caching the resulting string
asynchronously (e.g., create a `@State` String property like notesFromAuthors,
populate it in a Task/Task.detached or a task(id: model.$events) modifier that
performs ndb.lookup_profile_and_copy on a background queue, or subscribe to
model.$events with Combine to update the cached value), and have body simply
read the cached `@State` value; update any callers to use the cached variable
instead of directly invoking notesFromAuthorsString and ensure thread-safe
access to Ndb by performing all lookups off the main thread.
- Around line 58-64: The computed property allAuthors currently rebuilds
eventAuthors, filterAuthors, and a deduplication Set on every SwiftUI body
render; change this to a stored cached authors collection on
NIP05DomainTimelineHeaderView that is incrementally updated instead: add a
stored var (e.g., cachedAuthors: [Pubkey] or Set<Pubkey>) and update it when new
events arrive by handling model.events.all_events changes (hook into add_event
or attach a Combine sink to the model's events publisher) and also merge
model.filter.authors when the filter changes; then replace the allAuthors
computed property to return the cached value to avoid per-render allocations.
In `@damus/Features/Timeline/Models/HomeModel.swift`:
- Around line 845-866: The code uses a confusing double-break to exit the `for
await` loop over `damus_state.nostrNetwork.reader.advancedStream` by breaking
the `switch` on `.eose` and then separately checking `if case .eose = item {
break }`; replace this with a single labeled loop to clearly break out from
inside the `switch`: give the `for await` loop a label (e.g., outerLoop) and in
`case .eose` perform `break outerLoop`, removing the later `if case .eose = item
{ break }` check; apply this change around the loop handling in HomeModel (the
`for chunk in chunks { ... for await item in ... { switch item { ... } } }`
block) so the intent to exit the stream on EOSE is explicit and not duplicated.
- Around line 885-914: The refreshSingleAuthorEvents function spawns an
untracked Task; instead add a private Task? property (e.g.,
refreshSingleAuthorTask) on HomeModel, cancel any existing task before creating
a new one, assign the new Task to that property, run the existing advancedStream
logic inside that Task, and ensure you clear the handle (set to nil) when the
stream returns or finishes (EOSE/return) so it can be cancelled next time—follow
the same cancel-then-replace pattern used by backfillForGroupedMode and
refreshGroupedFromNdb and reference
damus_state.nostrNetwork.reader.advancedStream and the refreshSingleAuthorEvents
method when updating the code.
In `@damus/Shared/Components/GroupedTimeline/GroupedAuthorRow.swift`:
- Around line 68-85: Make eventPreviewText a static (or free) function since it
doesn't use instance state: change func eventPreviewText(_ event: NostrEvent) to
static func eventPreviewText(_ event: NostrEvent) (or move it out of the type)
and update references accordingly; also broaden the regex that strips nostr
identifiers to include bech32-like characters and underscores by replacing
"nostr:[a-zA-Z0-9]+" with something like "nostr:[a-z0-9_]+" (or the precise
bech32 charset) and use appropriate regex options (no need for uppercase A-Z) so
nprofile/nevent bech32 references are removed correctly.
In `@damus/Shared/Components/GroupedTimeline/GroupedTimelineGrouper.swift`:
- Around line 91-95: Add Swift doc comments (///) for the helper methods
parseFilteredWords(_:), shouldIncludeEvent(_:),
containsFilteredWords(_:filteredWords:), and isTooShort(_:) describing each
method's purpose, parameters and return semantics; for example, state that
parseFilteredWords takes a comma-separated raw string and returns a lowercased
array of trimmed words (minimum length 2), shouldIncludeEvent decides whether an
event belongs in a group and what boolean/criteria it returns,
containsFilteredWords checks if a given text contains any of the parsed filtered
words, and isTooShort determines if an event's text is too short to include;
follow the same docstring style used on group(_:), keep comments concise, and
ensure parameter names and return types are documented.
| {"id":"damus-289","title":"Fix no-op settings, remove dead toggles, extract testable grouping logic","status":"closed","priority":1,"issue_type":"task","created_at":"2026-02-16T16:33:50.98876-06:00","updated_at":"2026-02-16T22:45:43.789297-06:00","closed_at":"2026-02-16T22:45:43.789297-06:00","created_by":"e"} | ||
| {"id":"damus-b9i","title":"Investigate if grouped view makes sense in universe view","description":"Research and evaluate whether the grouped view mode would be useful in the universe view. Consider: volume of posts (universe is high-traffic), UX implications of grouping unknown authors, performance with large datasets, and whether WOT filtering in grouped mode adds value for universe discovery.","status":"closed","priority":3,"issue_type":"task","created_at":"2026-02-16T15:16:47.509285-06:00","updated_at":"2026-02-16T23:13:15.653028-06:00","closed_at":"2026-02-16T23:13:15.653028-06:00","created_by":"e","dependencies":[{"issue_id":"damus-b9i","depends_on_id":"damus-ku4","type":"blocks","created_at":"2026-02-16T15:16:58.815499-06:00","created_by":"daemon"}]} | ||
| {"id":"damus-8wm","title":"Add grouped mode to home timeline (follows and favorites)","status":"closed","priority":1,"issue_type":"task","created_at":"2026-02-16T16:33:53.249035-06:00","updated_at":"2026-02-16T22:57:24.113733-06:00","closed_at":"2026-02-16T22:57:24.113733-06:00","created_by":"e","dependencies":[{"issue_id":"damus-8wm","depends_on_id":"damus-4yl","type":"blocks","created_at":"2026-02-16T16:33:57.822835-06:00","created_by":"daemon"}]} | ||
| {"id":"damus-ir1","title":"fix nip-05","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-04T10:48:44.424341-06:00","updated_at":"2026-02-16T23:12:15.889032-06:00","closed_at":"2026-02-16T23:12:15.889032-06:00"} |
Contributor
There was a problem hiding this comment.
Minor schema inconsistency: created_by field is missing.
Every other entry in this file includes a created_by field, but damus-ir1 omits it. If downstream tooling or queries rely on a consistent schema, this could cause issues.
-{"id":"damus-ir1","title":"fix nip-05","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-04T10:48:44.424341-06:00","updated_at":"2026-02-16T23:12:15.889032-06:00","closed_at":"2026-02-16T23:12:15.889032-06:00"}
+{"id":"damus-ir1","title":"fix nip-05","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-04T10:48:44.424341-06:00","updated_at":"2026-02-16T23:12:15.889032-06:00","closed_at":"2026-02-16T23:12:15.889032-06:00","created_by":"e"}📝 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
| {"id":"damus-ir1","title":"fix nip-05","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-04T10:48:44.424341-06:00","updated_at":"2026-02-16T23:12:15.889032-06:00","closed_at":"2026-02-16T23:12:15.889032-06:00"} | |
| {"id":"damus-ir1","title":"fix nip-05","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-04T10:48:44.424341-06:00","updated_at":"2026-02-16T23:12:15.889032-06:00","closed_at":"2026-02-16T23:12:15.889032-06:00","created_by":"e"} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.beads/issues.jsonl at line 5, The JSON entry with "id":"damus-ir1" is
missing the created_by field; update that object in .beads/issues.jsonl to
include a created_by key with the same format used by other entries (e.g., a
string or user identifier), ensuring the schema matches other records so
downstream tooling can rely on a consistent field presence.
Comment on lines
+1976
to
+1981
| B1B2B3B4B5B6B7B8B9B0C1C2 /* GroupedTimelineGrouper.swift in Sources */ = {isa = PBXBuildFile; fileRef = A1B2C3D4E5F6A7B8C9D0E1F2 /* GroupedTimelineGrouper.swift */; }; | ||
| B2B3B4B5B6B7B8B9B0C1C2C3 /* GroupedTimelineGrouper.swift in Sources */ = {isa = PBXBuildFile; fileRef = A1B2C3D4E5F6A7B8C9D0E1F2 /* GroupedTimelineGrouper.swift */; }; | ||
| B3B4B5B6B7B8B9B0C1C2C3C4 /* GroupedTimelineGrouper.swift in Sources */ = {isa = PBXBuildFile; fileRef = A1B2C3D4E5F6A7B8C9D0E1F2 /* GroupedTimelineGrouper.swift */; }; | ||
| C1C2C3C4C5C6C7C8C9C0D1D2 /* GroupedModeQueueManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2E1D0C9B8A7F6E5D4C3B2A1 /* GroupedModeQueueManager.swift */; }; | ||
| C2C3C4C5C6C7C8C9C0D1D2D3 /* GroupedModeQueueManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2E1D0C9B8A7F6E5D4C3B2A1 /* GroupedModeQueueManager.swift */; }; | ||
| C3C4C5C6C7C8C9C0D1D2D3D4 /* GroupedModeQueueManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2E1D0C9B8A7F6E5D4C3B2A1 /* GroupedModeQueueManager.swift */; }; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python3 - << 'EOF'
import re
from collections import Counter
with open('damus.xcodeproj/project.pbxproj', 'r') as f:
content = f.read()
sequential = [
'A1A2A3A4A5A6A7A8A9A0B1B2',
'A1B2C3D4E5F6A7B8C9D0E1F2',
'F2E1D0C9B8A7F6E5D4C3B2A1',
'B1B2B3B4B5B6B7B8B9B0C1C2', 'B2B3B4B5B6B7B8B9B0C1C2C3', 'B3B4B5B6B7B8B9B0C1C2C3C4',
'C1C2C3C4C5C6C7C8C9C0D1D2', 'C2C3C4C5C6C7C8C9C0D1D2D3', 'C3C4C5C6C7C8C9C0D1D2D3D4',
'D1D2D3D4D5D6D7D8D9D0E1E2', 'D2D3D4D5D6D7D8D9D0E1E2E3',
'E1E2E3E4E5E6E7E8E9E0F1F2', 'E2E3E4E5E6E7E8E9E0F1F2F3',
]
print("=== Sequential UUID occurrence counts ===")
for uuid in sequential:
count = content.count(uuid)
# Each UUID should appear exactly as many times as it is cross-referenced (typically 2-3).
# Any count > expected references suggests a collision with a pre-existing entry.
print(f" {uuid}: {count} occurrence(s)")
# Check for any UUID that appears as a *definition key* (left-hand side of an assignment)
# more than once, which would be a definitive collision.
defs = re.findall(r'^\s+([A-F0-9]{24})\s*/\*', content, re.MULTILINE)
dupes = {k: v for k, v in Counter(defs).items() if v > 1}
if dupes:
print("\n=== COLLISION: UUIDs used as definition keys more than once ===")
for uuid, cnt in dupes.items():
print(f" {uuid}: {cnt} definitions")
else:
print("\nNo definition-key collisions detected (does not rule out cross-reference collisions).")
EOFRepository: damus-io/damus
Length of output: 50371
🏁 Script executed:
# Check actual lines mentioned in the review
echo "=== Lines 1976-1981 ==="
sed -n '1976,1981p' damus.xcodeproj/project.pbxproj
echo ""
echo "=== Lines 138-139 ==="
sed -n '138,139p' damus.xcodeproj/project.pbxproj
echo ""
echo "=== Lines 2231-2232 ==="
sed -n '2231,2232p' damus.xcodeproj/project.pbxproj
echo ""
echo "=== Lines 2949-2950 ==="
sed -n '2949,2950p' damus.xcodeproj/project.pbxproj
echo ""
echo "=== Line 3833 ==="
sed -n '3833p' damus.xcodeproj/project.pbxproj
echo ""
echo "=== Line 5440 ==="
sed -n '5440p' damus.xcodeproj/project.pbxprojRepository: damus-io/damus
Length of output: 2513
Remove hand-crafted sequential UUIDs and re-add all GroupedTimeline files through Xcode
Every UUID assigned to the new GroupedTimeline objects follows an obvious decimal-counting pattern rather than the cryptographically random 24-hex-char identifiers Xcode generates:
| Location | UUID | Entry |
|---|---|---|
| Lines 1976–1978 | B1B2B3…, B2B3B4…, B3B4B5… |
GroupedTimelineGrouper.swift build files |
| Lines 1979–1981 | C1C2C3…, C2C3C4…, C3C4C5… |
GroupedModeQueueManager.swift build files |
| Lines 138–139 | D2D3D4…, E2E3E4… |
test-file build entries |
| Lines 2231–2232 | D1D2D3…, E1E2E3… |
test-file PBXFileReference |
| Lines 2949–2950 | A1B2C3D4E5F6A7B8C9D0E1F2, F2E1D0C9B8A7F6E5D4C3B2A1 |
GroupedTimelineGrouper/GroupedModeQueueManager PBXFileReference (with includeInIndex = 1) |
| Lines 3833 / 5440 | A1A2A3A4A5A6A7A8A9A0B1B2 |
GroupedTimeline PBXGroup |
This hand-authored pattern creates three risks:
- UUID collision — future Xcode-generated entries can randomly collide with these low-entropy values, silently corrupting references.
- Merge fragility — patterned UUIDs are more likely to conflict when rebasing or cherry-picking.
- Xcode corruption — on next project save Xcode may rewrite or deduplicate these, breaking the build.
Remove all manually-added GroupedTimeline entries and re-add the six source files and two test files through Xcode's "Add Files to…" dialog so Xcode generates proper random UUIDs and correct PBXFileReference attributes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus.xcodeproj/project.pbxproj` around lines 1976 - 1981, The PBX project
contains hand-crafted sequential UUIDs for new GroupedTimeline entries (e.g.,
PBXBuildFile entries referencing GroupedTimelineGrouper.swift and
GroupedModeQueueManager.swift, PBXFileReference entries, and the GroupedTimeline
PBXGroup) which risks collisions and corruption; remove all manually-added
GroupedTimeline-related PBXBuildFile, PBXFileReference and PBXGroup entries (the
ones using patterns like B1B2B3..., C1C2C3..., A1B2C3..., F2E1D0..., A1A2A3...)
from the project.pbxproj and then re-add the six source files and two test files
using Xcode’s “Add Files to…” dialog so Xcode will generate proper random 24-hex
UUIDs and correct attributes (includeInIndex, etc.). Ensure you delete both the
duplicate build-file entries and their matching fileRef PBXFileReference and
PBXGroup nodes before re-adding.
Comment on lines
+6379
to
+6384
| CB20AABE3C7B960961C0F914 /* GroupedFilterButton.swift in Sources */, | ||
| CF86249414FD714F8E45AA55 /* GroupedFilterSettingsView.swift in Sources */, | ||
| D93D3D7A501A49D1F86669A3 /* GroupedAuthorRow.swift in Sources */, | ||
| 91E25E8CB81B797E7FCC4D25 /* GroupedListView.swift in Sources */, | ||
| B1B2B3B4B5B6B7B8B9B0C1C2 /* GroupedTimelineGrouper.swift in Sources */, | ||
| C1C2C3C4C5C6C7C8C9C0D1D2 /* GroupedModeQueueManager.swift in Sources */, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
cd damus.xcodeproj && python3 - << 'EOF'
import re
with open('project.pbxproj', 'r') as f:
content = f.read()
# All GroupedTimeline build-file UUIDs added in this PR
grouped_build_uuids = {
# GroupedAuthorRow
'0A4E465299FD22F7FF9ABC4B', '343D40E5239706F50DB4B7A1', 'D93D3D7A501A49D1F86669A3',
# GroupedFilterButton
'15816FA4C2C558EB93B8E06B', 'CB20AABE3C7B960961C0F914', 'D2491E9E616838CC8A66A5A9',
# GroupedFilterSettingsView
'1695A2396ABB4CA359EA5E02', 'CF86249414FD714F8E45AA55', 'E51876D3421296DECB2DEE22',
# GroupedListView
'91E25E8CB81B797E7FCC4D25', 'A3D3CD9B73EB7E2020F93345', 'C49C1BB6E2EE0DC7C7D28FC4',
# GroupedTimelineGrouper (sequential UUIDs)
'B1B2B3B4B5B6B7B8B9B0C1C2', 'B2B3B4B5B6B7B8B9B0C1C2C3', 'B3B4B5B6B7B8B9B0C1C2C3C4',
# GroupedModeQueueManager (sequential UUIDs)
'C1C2C3C4C5C6C7C8C9C0D1D2', 'C2C3C4C5C6C7C8C9C0D1D2D3', 'C3C4C5C6C7C8C9C0D1D2D3D4',
}
# Parse Sources build phases and find those containing any GroupedTimeline UUID
phase_re = re.compile(
r'([A-F0-9]{24})\s*/\* Sources \*/\s*=\s*\{.*?isa\s*=\s*PBXSourcesBuildPhase;.*?files\s*=\s*\(([^)]+)\)',
re.DOTALL)
phase_hits = {}
for m in phase_re.finditer(content):
found = grouped_build_uuids & set(re.findall(r'([A-F0-9]{24})', m.group(2)))
if found:
phase_hits[m.group(1)] = found
# Map each phase back to its native target
target_re = re.compile(
r'([A-F0-9]{24})\s*/\*[^*]*\*/\s*=\s*\{[^{]*?isa\s*=\s*PBXNativeTarget;'
r'[^{]*?name\s*=\s*"?([^";\n]+)"?\s*;.*?buildPhases\s*=\s*\(([^)]+)\)',
re.DOTALL)
print("Native targets that compile GroupedTimeline files:")
for m in target_re.finditer(content):
target_name = m.group(2).strip()
phase_uuids = set(re.findall(r'([A-F0-9]{24})', m.group(3)))
if phase_uuids & set(phase_hits.keys()):
print(f" → {target_name}")
EOFRepository: damus-io/damus
Length of output: 125
🏁 Script executed:
#!/bin/bash
cd damus.xcodeproj && grep -o 'name = "[^"]*"[^{]*isa = PBXNativeTarget' project.pbxproj | head -20Repository: damus-io/damus
Length of output: 40
🏁 Script executed:
#!/bin/bash
cd damus.xcodeproj && python3 << 'EOF'
import re
with open('project.pbxproj', 'r') as f:
content = f.read()
# Find all native targets
target_re = re.compile(
r'([A-F0-9]{24})\s*/\*([^*]+)\s*\*/\s*=\s*\{[^{]*?isa\s*=\s*PBXNativeTarget;',
re.DOTALL)
print("All native targets in project:")
for m in target_re.finditer(content):
print(f" {m.group(2).strip()}")
EOFRepository: damus-io/damus
Length of output: 197
🏁 Script executed:
#!/bin/bash
cd damus.xcodeproj && python3 << 'EOF'
import re
with open('project.pbxproj', 'r') as f:
content = f.read()
grouped_build_uuids = {
# GroupedAuthorRow
'0A4E465299FD22F7FF9ABC4B', '343D40E5239706F50DB4B7A1', 'D93D3D7A501A49D1F86669A3',
# GroupedFilterButton
'15816FA4C2C558EB93B8E06B', 'CB20AABE3C7B960961C0F914', 'D2491E9E616838CC8A66A5A9',
# GroupedFilterSettingsView
'1695A2396ABB4CA359EA5E02', 'CF86249414FD714F8E45AA55', 'E51876D3421296DECB2DEE22',
# GroupedListView
'91E25E8CB81B797E7FCC4D25', 'A3D3CD9B73EB7E2020F93345', 'C49C1BB6E2EE0DC7C7D28FC4',
# GroupedTimelineGrouper
'B1B2B3B4B5B6B7B8B9B0C1C2', 'B2B3B4B5B6B7B8B9B0C1C2C3', 'B3B4B5B6B7B8B9B0C1C2C3C4',
# GroupedModeQueueManager
'C1C2C3C4C5C6C7C8C9C0D1D2', 'C2C3C4C5C6C7C8C9C0D1D2D3', 'C3C4C5C6C7C8C9C0D1D2D3D4',
}
# Parse Sources build phases and find those containing any GroupedTimeline UUID
phase_re = re.compile(
r'([A-F0-9]{24})\s*/\* Sources \*/\s*=\s*\{.*?isa\s*=\s*PBXSourcesBuildPhase;.*?files\s*=\s*\(([^)]+)\)',
re.DOTALL)
phase_hits = {}
for m in phase_re.finditer(content):
found = grouped_build_uuids & set(re.findall(r'([A-F0-9]{24})', m.group(2)))
if found:
phase_hits[m.group(1)] = found
# Map each phase back to its native target
target_re = re.compile(
r'([A-F0-9]{24})\s*/\*([^*]+)\*/\s*=\s*\{[^{]*?isa\s*=\s*PBXNativeTarget;'
r'[^{]*?buildPhases\s*=\s*\(([^)]+)\)',
re.DOTALL)
print("Detailed target analysis:")
for m in target_re.finditer(content):
target_name = m.group(2).strip()
phase_uuids = set(re.findall(r'([A-F0-9]{24})', m.group(3)))
has_grouped = bool(phase_uuids & set(phase_hits.keys()))
status = "✓ HAS GroupedTimeline files" if has_grouped else "✗ NO GroupedTimeline files"
print(f" {target_name}: {status}")
EOFRepository: damus-io/damus
Length of output: 363
Remove GroupedTimeline files from ShareExtension and HighlighterActionExtension targets
Verification confirms the GroupedTimeline source files are incorrectly wired into two app extension targets:
- ShareExtension
- HighlighterActionExtension
These extensions should not include SwiftUI views and filter-settings logic—they will cause linker failures or unnecessarily bloat the extension binaries. Remove the six GroupedTimeline files from these extension targets; only the main damus app target should build them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus.xcodeproj/project.pbxproj` around lines 6379 - 6384, The six
GroupedTimeline source files (GroupedFilterButton.swift,
GroupedFilterSettingsView.swift, GroupedAuthorRow.swift, GroupedListView.swift,
GroupedTimelineGrouper.swift, GroupedModeQueueManager.swift) are currently
assigned to the ShareExtension and HighlighterActionExtension targets; open the
project.pbxproj and remove their file reference entries (e.g.,
CB20AABE3C7B960961C0F914, CF86249414FD714F8E45AA55, D93D3D7A501A49D1F86669A3,
91E25E8CB81B797E7FCC4D25, B1B2B3B4B5B6B7B8B9B0C1C2, C1C2C3C4C5C6C7C8C9C0D1D2)
from the PBXNativeTarget build phase sections for ShareExtension and
HighlighterActionExtension so they remain only in the main damus app target;
ensure you only remove the target membership entries and do not delete the file
references themselves from the project if the main app still needs them.
Comment on lines
+134
to
+143
| func load_more() { | ||
| guard !loading_more, has_more else { return } | ||
| guard let oldest = events.all_events.last?.created_at else { return } | ||
|
|
||
| loading_more = true | ||
| let until = oldest > 0 ? oldest &- 1 : 0 | ||
| Task { | ||
| await self.fetch_older(until: until) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Missing docstrings on load_more, fetch_older, and add_event.
All three methods are added in this PR with no /// documentation. The coding guidelines require docstring coverage for any code added or modified.
♻️ Skeleton to add
+ /// Requests the next page of older events, guarded by `loading_more`/`has_more`.
+ /// Computes `until` from the oldest currently loaded event timestamp.
func load_more() { ... }
+ /// Fetches events older than `until` using a one-shot relay query.
+ /// Updates `has_more` and `last_loaded_count` on EOSE.
+ /// - Parameter until: Upper-bound Unix timestamp (exclusive) for the query.
private func fetch_older(until: UInt32) async { ... }
+ /// Validates and inserts a single relay event into the holder.
+ /// Skips metadata, filter mismatches, and muted/NSFW content.
+ /// - Parameter ev: The Nostr event received from the relay.
func add_event(_ ev: NostrEvent) async { ... }As per coding guidelines: "Ensure docstring coverage for any code added or modified."
📝 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
| func load_more() { | |
| guard !loading_more, has_more else { return } | |
| guard let oldest = events.all_events.last?.created_at else { return } | |
| loading_more = true | |
| let until = oldest > 0 ? oldest &- 1 : 0 | |
| Task { | |
| await self.fetch_older(until: until) | |
| } | |
| } | |
| /// Requests the next page of older events, guarded by `loading_more`/`has_more`. | |
| /// Computes `until` from the oldest currently loaded event timestamp. | |
| func load_more() { | |
| guard !loading_more, has_more else { return } | |
| guard let oldest = events.all_events.last?.created_at else { return } | |
| loading_more = true | |
| let until = oldest > 0 ? oldest &- 1 : 0 | |
| Task { | |
| await self.fetch_older(until: until) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus/Features/NIP05/Models/NIP05DomainEventsModel.swift` around lines 134 -
143, Add triple-slash docstrings for the three new methods: load_more(),
fetch_older(until:), and add_event(_:). For each, provide a one-line summary of
purpose, brief description of parameters (e.g., until: cutoff timestamp for
fetch_older, event param for add_event), mention concurrency/side-effects (that
load_more/ fetch_older run async Tasks and mutate loading_more/ events), and any
return/throw behavior (if none, state “no return value”). Place the docstrings
directly above the corresponding function declarations (load_more, fetch_older,
add_event) using the /// style to satisfy the coding guidelines.
Comment on lines
+14
to
+27
| /// Display modes for the NIP-05 domain timeline. | ||
| enum NIP05ViewMode: String, CaseIterable { | ||
| case grouped // One row per author with post count | ||
| case timeline // Chronological list of all posts | ||
|
|
||
| var localizedTitle: String { | ||
| switch self { | ||
| case .grouped: | ||
| return NSLocalizedString("Grouped", comment: "Tab label for grouped view of posts by author") | ||
| case .timeline: | ||
| return NSLocalizedString("Timeline", comment: "Tab label for chronological timeline view") | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "NIP05ViewMode" --type=swiftRepository: damus-io/damus
Length of output: 160
Remove unused NIP05ViewMode enum.
The enum defines .grouped and .timeline cases with localizedTitle property but is never referenced in the codebase. The view uses filterSettings.enableGroupedMode (a boolean) to toggle between modes instead, making this enum dead code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus/Features/NIP05/Views/NIP05DomainTimelineView.swift` around lines 14 -
27, Remove the unused NIP05ViewMode enum and its localizedTitle property: locate
the enum declaration named NIP05ViewMode and delete it (including the cases
.grouped and .timeline and the var localizedTitle), and ensure there are no
remaining references—note the view currently toggles modes via
filterSettings.enableGroupedMode, so update any references that might have
intended to use NIP05ViewMode to rely on filterSettings.enableGroupedMode
instead if necessary.
Comment on lines
+869
to
+878
| // Insert all events at once, then clear shimmer | ||
| for ev in allEvents { | ||
| self.damus_state.events.insert(ev) | ||
| if isFavorites { | ||
| await self.favoriteEvents.insert(ev) | ||
| } else { | ||
| await self.insert_home_event(ev) | ||
| } | ||
| } | ||
| DispatchQueue.main.async { self.backfilling = false } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the containing method around lines 869-878
fd "HomeModel.swift" --type fRepository: damus-io/damus
Length of output: 155
🏁 Script executed:
# Check the method signature and context around lines 869-878
rg -n "backfillPageLimit\|EventCache\|EventHolder" damus --type swift | head -40Repository: damus-io/damus
Length of output: 40
🏁 Script executed:
# Find the definition of backfillPageLimit
rg -n "backfillPageLimit" damus --type swift -B2 -A2Repository: damus-io/damus
Length of output: 1062
🏁 Script executed:
# Check if the method containing this code has `@MainActor` annotation
sed -n '850,885p' damus/Features/Timeline/Models/HomeModel.swiftRepository: damus-io/damus
Length of output: 1620
🏁 Script executed:
# Look for EventCache.insert implementation
rg -n "class EventCache" damus --type swift -A50 | grep -A20 "func insert"Repository: damus-io/damus
Length of output: 40
🏁 Script executed:
# Look for EventHolder.insert implementation
rg -n "class EventHolder\|struct EventHolder" damus --type swift -A100 | grep -B5 -A10 "func insert"Repository: damus-io/damus
Length of output: 40
🏁 Script executed:
# Get more context to find the method signature
sed -n '820,885p' damus/Features/Timeline/Models/HomeModel.swiftRepository: damus-io/damus
Length of output: 2911
🏁 Script executed:
# Search for damus_state.events definition
rg -n "var events" damus --type swift -B2 -A5Repository: damus-io/damus
Length of output: 21627
🏁 Script executed:
# Search for EventCache class definition
rg -n "class EventCache" damus --type swift -A30Repository: damus-io/damus
Length of output: 2402
🏁 Script executed:
# Search for insert method more broadly
rg -n "\.insert\(" damus/Features/Timeline/Models/HomeModel.swift -B2 -A2Repository: damus-io/damus
Length of output: 2857
🏁 Script executed:
# Get the insert method from EventHolder
rg -n "func insert" damus/Shared/Utilities/EventHolder.swift -B2 -A15Repository: damus-io/damus
Length of output: 1672
🏁 Script executed:
# Check the full EventHolder class to see actor/isolation annotations
sed -n '11,150p' damus/Shared/Utilities/EventHolder.swiftRepository: damus-io/damus
Length of output: 3778
🏁 Script executed:
# Check if insert_home_event and favoriteEvents.insert are on the main thread
grep -n "@MainActor\|class EventHolder\|func insert" damus/Shared/Utilities/EventHolder.swift | head -20Repository: damus-io/damus
Length of output: 515
🏁 Script executed:
# Check if damus_state.events is EventCache or EventHolder
rg -n "damus_state.events" damus/Features/Timeline/Models/HomeModel.swift | head -5Repository: damus-io/damus
Length of output: 442
🏁 Script executed:
# Check what damus_state.events actually is - look at DamusState definition
rg -n "class DamusState\|struct DamusState" damus --type swift -A20 | grep -A20 "var events"Repository: damus-io/damus
Length of output: 40
🏁 Script executed:
# Check if there are any batch or bulk insert methods available
rg -n "func.*insert\|batch\|bulk" damus/Shared/Utilities/EventHolder.swiftRepository: damus-io/damus
Length of output: 40
🏁 Script executed:
# Find DamusState definition
fd "DamusState" damus --type fRepository: damus-io/damus
Length of output: 141
🏁 Script executed:
# Search for DamusState class/struct definition
rg -n "class DamusState\|struct DamusState" damus --type swift -A50 | head -100Repository: damus-io/damus
Length of output: 40
🏁 Script executed:
# Check what type events is in the context
rg -n "var events:" damus --type swift -B3 -A3Repository: damus-io/damus
Length of output: 17350
🏁 Script executed:
# Look at the DamusState definition
head -100 damus/Core/Storage/DamusState.swiftRepository: damus-io/damus
Length of output: 4060
🏁 Script executed:
# Search for events property specifically in DamusState
grep -n "var events" damus/Core/Storage/DamusState.swiftRepository: damus-io/damus
Length of output: 40
🏁 Script executed:
# Get the insert method in EventCache
rg -n "func insert" damus/Shared/Utilities/EventCache.swift -B2 -A15Repository: damus-io/damus
Length of output: 536
🏁 Script executed:
# Check if EventCache is an actor or has MainActor annotation
head -200 damus/Shared/Utilities/EventCache.swift | tail -100Repository: damus-io/damus
Length of output: 3082
🏁 Script executed:
# Check if backfillForGroupedMode has `@MainActor` annotation
rg -n "func backfillForGroupedMode" damus/Features/Timeline/Models/HomeModel.swift -B5Repository: damus-io/damus
Length of output: 371
🏁 Script executed:
# Check the full context of the method to see if it's isolated to MainActor
sed -n '820,825p' damus/Features/Timeline/Models/HomeModel.swiftRepository: damus-io/damus
Length of output: 308
🏁 Script executed:
# Check what actor or MainActor annotation HomeModel has
rg -n "class HomeModel\|@MainActor" damus/Features/Timeline/Models/HomeModel.swift | head -10Repository: damus-io/damus
Length of output: 40
🏁 Script executed:
# Verify if EventCache has any actor or thread-safety annotations
head -105 damus/Shared/Utilities/EventCache.swift | tail -15Repository: damus-io/damus
Length of output: 595
🏁 Script executed:
# Check insert_home_event signature
rg -n "func insert_home_event" damus/Features/Timeline/Models/HomeModel.swift -B2 -A5Repository: damus-io/damus
Length of output: 263
Bulk insertion of up to 5000 events with repeated main-actor context switches.
The backfillForGroupedMode task executes on a background thread and loops through allEvents (up to backfillPageLimit = 5000), calling await self.favoriteEvents.insert(ev) and await self.insert_home_event(ev) for each iteration. Both methods are @MainActor, causing 5000 context switches to the main thread.
Additionally, self.damus_state.events.insert(ev) (EventCache.insert) is called directly from the background thread without thread safety, as EventCache is not thread-isolated and maintains a non-atomic dictionary.
Consider batching EventHolder inserts or restructuring to minimize main-actor context switches. Ensure EventCache modifications are properly synchronized if accessed from multiple threads.
🤖 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 869 - 878, The
loop in backfillForGroupedMode causes up to 5000 main-actor switches and
unsynchronized access to EventCache: collect allEvents (or build two arrays:
favoritesBatch and homeBatch) on the background task, then perform a single
await MainActor.run { ... } (or call one `@MainActor` method) to 1) insert the
batch into favoriteEvents via favoriteEvents.insert(contentsOf:) or similar and
2) call insert_home_event for the batch; also move the damus_state.events.insert
calls into that same main-actor block (or protect EventCache by annotating
EventCache as `@MainActor` or serializing its mutations on a dedicated queue) so
EventCache mutations are done safely and context switches are minimized.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (10)
damus/Features/Timeline/Views/PostingTimelineView.swift (1)
264-272: PotentialUInt32wrapping iftimeRange.secondsexceeds the current timestamp is not guarded.While in practice the current Unix timestamp (~1.7B) vastly exceeds the max
timeRange.seconds(7 days ≈ 604800), a defensivemax(0, ...)orsaturating subtractionwould harden this against future additions of larger time ranges.private func backfillForGroupedMode(showLoading: Bool = true) { - let since = UInt32(Date().timeIntervalSince1970) - groupedFilterSettings.timeRange.seconds + let now = UInt32(Date().timeIntervalSince1970) + let since = now > groupedFilterSettings.timeRange.seconds ? now - groupedFilterSettings.timeRange.seconds : 0 home.backfillForGroupedMode(since: since, source: timeline_source, showLoading: showLoading) } private func refreshFromNdb() { - let since = UInt32(Date().timeIntervalSince1970) - groupedFilterSettings.timeRange.seconds + let now = UInt32(Date().timeIntervalSince1970) + let since = now > groupedFilterSettings.timeRange.seconds ? now - groupedFilterSettings.timeRange.seconds : 0 home.refreshGroupedFromNdb(since: since, source: timeline_source) }🤖 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 264 - 272, The subtraction to produce a UInt32 timestamp in backfillForGroupedMode and refreshFromNdb can wrap if groupedFilterSettings.timeRange.seconds is larger than the current time; guard by computing a non-negative difference before casting: compute let delta = max(0, Int(Date().timeIntervalSince1970) - Int(groupedFilterSettings.timeRange.seconds)) (or use a saturating/subtraction helper) and then pass UInt32(delta) to home.backfillForGroupedMode(...) and home.refreshGroupedFromNdb(...), referencing the functions backfillForGroupedMode, refreshFromNdb and the groupedFilterSettings.timeRange.seconds value to locate the change.damus/Shared/Components/GroupedTimeline/GroupedAuthorRow.swift (1)
68-85:eventPreviewTextregex won't stripnostr:references containing_(e.g., NIP-19 TLV hex segments).The current regex
nostr:[a-zA-Z0-9]+covers standard bech32-encoded references well. However, some clients embed raw hex or extended identifiers that can include underscores or other characters after thenostr:prefix. A slightly broader character class like[a-zA-Z0-9_]+or the commonly used\\S+would be safer:- .replacingOccurrences(of: "nostr:[a-zA-Z0-9]+", with: "", options: .regularExpression) + .replacingOccurrences(of: "nostr:\\S+", with: "", options: .regularExpression)This prevents leftover fragments from partially-matched references appearing in previews.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Shared/Components/GroupedTimeline/GroupedAuthorRow.swift` around lines 68 - 85, The regex in eventPreviewText currently only removes alphanumerics after "nostr:" and can leave fragments (update the replacement to match broader identifiers); change the replacement pattern used on content (the .replacingOccurrences(of: "nostr:[a-zA-Z0-9]+", ...)) to a pattern that captures underscores or any non-whitespace sequence (e.g., "nostr:[a-zA-Z0-9_]+" or "nostr:\\S+"), so the cleaned string removes full nostr: references before trimming and length-checking.damus/Shared/Components/GroupedTimeline/GroupedFilterSettingsView.swift (1)
12-39: Consider annotatingGroupedFilterSettingswith@MainActor.This
ObservableObjectis mutated from SwiftUI views (main thread) and fromonChange/onAppearcallbacks. Adding@MainActorto the class ensures the compiler enforces main-thread-only access, preventing accidental background mutations as the codebase grows.-class GroupedFilterSettings: ObservableObject { +@MainActor +class GroupedFilterSettings: ObservableObject {Also,
reset()(line 31) is missing a docstring per coding guidelines.+ /// Restores all filter properties to their initial defaults. func reset() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Shared/Components/GroupedTimeline/GroupedFilterSettingsView.swift` around lines 12 - 39, Annotate the ObservableObject class GroupedFilterSettings with `@MainActor` to ensure all published-property mutations are main-thread enforced, and add a short docstring above the reset() method describing its purpose (restores defaults) per coding guidelines; update the class declaration (GroupedFilterSettings) and the reset() method doc comment accordingly so the compiler and reviewers can verify main-thread safety and documentation.damus/Shared/Components/GroupedTimeline/GroupedTimelineGrouper.swift (1)
92-139: Internal static methods are missing doc comments — required by coding guidelines.
parseFilteredWords,shouldIncludeEvent,containsFilteredWords, andisTooShorteach lack a doc comment (///), whilegroup()and the types above them are documented. As per coding guidelines, docstring coverage is required for any code added or modified.📝 Suggested doc stubs
+ /// Parses a raw comma-separated filter string into a trimmed, lowercased word list. + /// Words shorter than 2 characters are discarded. static func parseFilteredWords(_ raw: String) -> [String] { + /// Returns `true` if the event passes all active filters and should appear in the grouped view. static func shouldIncludeEvent( + /// Returns `true` if the event's content contains any of the words in `wordsList` (substring match). static func containsFilteredWords(_ event: NostrEvent, wordsList: [String]) -> Bool { + /// Returns `true` when `hideShortNotes` is enabled and the event's non-emoji text content is below + /// the minimum meaningful length threshold. static func isTooShort(_ event: NostrEvent, hideShortNotes: Bool) -> Bool {As per coding guidelines, "Ensure docstring coverage for any code added or modified."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Shared/Components/GroupedTimeline/GroupedTimelineGrouper.swift` around lines 92 - 139, Add missing doc comments for the four internal static methods: parseFilteredWords(_:), shouldIncludeEvent(_:filter:values:wordsList:cutoff:), containsFilteredWords(_:wordsList:), and isTooShort(_:hideShortNotes:). For each method add a /// comment that briefly describes the method's purpose, lists and documents parameters (e.g., raw, event, filter, values, wordsList, cutoff, hideShortNotes) and states the return value (what the Bool or [String] represents); keep comments concise and consistent with the existing style used for group() and surrounding types.damus/Features/NIP05/Views/NIP05DomainTimelineView.swift (1)
183-186:groupedSincesilently returnsnilwhen grouped mode is off, buthandleOnAppearalways passes it.If the user toggles from grouped → timeline while the view is alive, then navigates away and back,
handleOnAppearpassesgroupedSince(nownil) tosubscribe. This is correct for timeline mode, but the interaction is subtle and undocumented. A brief inline comment would help future readers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Views/NIP05DomainTimelineView.swift` around lines 183 - 186, The computed property groupedSince returns nil when filterSettings.enableGroupedMode is false but handleOnAppear always forwards groupedSince into subscribe, which can silently change behavior when the user toggles modes; add a concise inline comment next to the groupedSince property explaining that it intentionally returns nil for timeline mode and that handleOnAppear/subscribe rely on nil to indicate timeline (so toggling grouped→timeline while the view is alive is expected), and mention the interaction between groupedSince, handleOnAppear, and subscribe so future readers understand the intended contract.damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift (2)
58-64:allAuthorsrecomputes a deduplication pass over all events on every SwiftUI render.
model.events.all_events.map { $0.pubkey }allocates a temporary array proportional to the event count (up to 5000 in grouped mode), plus aSetfor dedup, on every body evaluation. Consider caching the ordered unique authors list in the model or in a@State/computed property that only updates whenmodel.eventschanges.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift` around lines 58 - 64, The computed property allAuthors currently maps model.events.all_events to extract pubkeys and deduplicates them on every SwiftUI render, causing large temporary allocations; change this so the ordered unique authors list is cached and only recomputed when events change — either move the deduplication into the backing model (e.g., expose a cached property on the model like uniqueAuthors) or store it in the view as a `@State/`@StateObject/@ObservedObject-backed property that you update in an onReceive or didSet when model.events changes; ensure you use the same ordering (event order + filter authors) and keep references to model.filter.authors and model.events.all_events when updating so UI reads allAuthors cheaply.
103-128:notesFromAuthorsStringandfriendsOfFriendsStringare near-identical — consider consolidating.Both functions share the same structure: look up 3 author names, then switch on count with identical format strings. The only differences are the profile lookup method (
lookup_profile_and_copyvs inlinelookup_profileborrow), and thecase 0string. A single parameterized helper (e.g., accepting anemptyMessageparameter and a name-resolver closure) would eliminate the duplication.♻️ Sketch
-func notesFromAuthorsString(_ authors: [Pubkey], ndb: Ndb, locale: Locale = Locale.current) -> String { +func notesFromAuthorsString(_ authors: [Pubkey], ndb: Ndb, locale: Locale = Locale.current, emptyMessage: String? = nil) -> String { let bundle = bundleForLocale(locale: locale) let names: [String] = authors.prefix(3).map { pk in let profile = try? ndb.lookup_profile_and_copy(pk) return Profile.displayName(profile: profile, pubkey: pk).username.truncate(maxLength: 20) } switch authors.count { case 0: - return NSLocalizedString("No notes yet", bundle: bundle, comment: "...") + return emptyMessage ?? NSLocalizedString("No notes yet", bundle: bundle, comment: "...") // ... rest unchanged } }Then
friendsOfFriendsStringbecomes a thin wrapper callingnotesFromAuthorsStringwith a customemptyMessage.Also applies to: 151-179
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift` around lines 103 - 128, The two near-identical functions notesFromAuthorsString and friendsOfFriendsString should be consolidated into a single parameterized helper: extract the shared logic (fetch up to 3 display names, truncate, and switch on authors.count to format localized strings) into a new function (e.g., authorsSummaryString(authors: [Pubkey], ndb: Ndb, locale: Locale, emptyMessage: String, nameResolver: (Pubkey, Ndb) -> Profile?)), and update notesFromAuthorsString and friendsOfFriendsString to call that helper passing the appropriate emptyMessage and a name-resolver closure that uses either lookup_profile_and_copy or lookup_profile as needed; ensure the helper uses Profile.displayName(...).username.truncate(maxLength: 20) and preserves the localized format keys (including notes_from_three_and_others).damus/Features/Timeline/Models/HomeModel.swift (3)
853-866: Confusing double-check for.eosein the innerfor awaitloop — use a labeled break instead.In Swift,
breakinside aswitchonly exits theswitch, not the enclosing loop. The current code works around this with a post-switchif case .eose = item { break }, while.ndbEose/.networkEoseusecontinueto skip that check. The duplication and the interaction betweenbreak/continueand theifguard are non-obvious.A labeled break removes the ambiguity entirely:
- for await item in damus_state.nostrNetwork.reader.advancedStream(filters: [chunkFilter], streamMode: .ndbAndNetworkParallel(networkOptimization: nil)) { - switch item { - case .event(let lender): - await lender.justUseACopy({ ev in - guard should_show_event(state: self.damus_state, ev: ev) else { return } - allEvents.append(ev) - }) - case .eose: - break - case .ndbEose, .networkEose: - continue - } - if case .eose = item { break } - } + chunkStream: for await item in damus_state.nostrNetwork.reader.advancedStream(filters: [chunkFilter], streamMode: .ndbAndNetworkParallel(networkOptimization: nil)) { + switch item { + case .event(let lender): + await lender.justUseACopy({ ev in + guard should_show_event(state: self.damus_state, ev: ev) else { return } + allEvents.append(ev) + }) + case .eose: + break chunkStream + case .ndbEose, .networkEose: + break // continue waiting for .eose + } + }🤖 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 853 - 866, The inner for-await loop over damus_state.nostrNetwork.reader.advancedStream should use a labeled loop to avoid the confusing post-switch check for .eose; add a label (e.g., streamLoop:) before the for await, then inside the switch case .eose use break streamLoop to exit the loop directly, remove the trailing if case .eose = item { break } check, and keep the existing .event handling (await lender.justUseACopy { ... }) and the .ndbEose/.networkEose continue behavior unchanged so semantics via should_show_event and allEvents remain the same.
766-772: Docstring is placed on thebackfillTaskvariable but describes the backfilling operation.Lines 766–768 read like a method-level docstring ("Backfills the given EventHolder with events from the relay going back to
since…") yet they are attached toprivate var backfillTask: Task<Void, Never>?.ndbRefreshTaskandbackfillPageLimitalso lack any documentation.Move the descriptive comment to
backfillForGroupedMode(which is the method it actually describes) and replace the variable comments with concise property-level docs:-/// Backfills the given EventHolder with events from the relay going back to `since`. -/// Used by grouped mode to ensure the full time window is represented, -/// since the normal subscription only fetches the most recent N events. -private var backfillTask: Task<Void, Never>? - -private var ndbRefreshTask: Task<Void, Never>? -private let backfillPageLimit: UInt32 = 5000 +/// Handle for the active background backfill task; cancel before re-creating. +private var backfillTask: Task<Void, Never>? +/// Handle for the active NDB-only local refresh task; cancel before re-creating. +private var ndbRefreshTask: Task<Void, Never>? +/// Maximum number of events fetched per author-chunk subscription during backfill. +private let backfillPageLimit: UInt32 = 5000As per coding guidelines, docstring coverage is required for any code added or modified.
🤖 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 766 - 772, The comment describing the backfilling behavior should be moved from the property declaration to the actual method it documents: remove the method-level docstring currently above private var backfillTask and paste it above the backfillForGroupedMode(...) function so the description matches the implementation; then add concise property-level docs for private var backfillTask: Task<Void, Never>? (e.g., "Task managing backfill work for grouped mode"), private var ndbRefreshTask: Task<Void, Never>? (e.g., "Task for refreshing ndb state"), and private let backfillPageLimit: UInt32 (e.g., "Maximum events to request per backfill page") to satisfy doc coverage and coding guidelines.
894-913:refreshSingleAuthorEventscreates an unmanaged, non-cancellableTask.Every invocation creates a new background task with no stored handle. Unlike
homeHandlerTask,favoritesHandlerTask,backfillTask, andndbRefreshTask(all cancelled before re-creation), this task cannot be cancelled. Rapid repeated calls — e.g., tapping into then back out of profiles quickly — accumulate concurrent tasks that all write to the sameEventHolder, producing interleaved event insertions and redundant relay subscriptions.♻️ Proposed fix — store and cancel the task handle
Add a stored property near the other task handles:
private var ndbRefreshTask: Task<Void, Never>? +private var singleAuthorRefreshTask: Task<Void, Never>? private let backfillPageLimit: UInt32 = 5000Then cancel before re-creating:
func refreshSingleAuthorEvents(pubkey: Pubkey, since: UInt32, source: TimelineSource) { let kinds: [NostrKind] = [.text, .longform, .boost, .highlight] let isFavorites = source == .favorites var filter = NostrFilter(kinds: kinds) filter.authors = [pubkey] filter.since = since filter.limit = 500 - Task { + singleAuthorRefreshTask?.cancel() + singleAuthorRefreshTask = Task { for await item in damus_state.nostrNetwork.reader.advancedStream(...) {🤖 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 894 - 913, refreshSingleAuthorEvents currently spawns an unmanaged Task that cannot be cancelled; add a stored Task? property (e.g., refreshSingleAuthorTask or authorEventsTask) alongside homeHandlerTask/favoritesHandlerTask, cancel the existing task (.cancel()) before creating a new one, assign the newly created Task to that property, and ensure the Task's loop honors cancellation (check Task.isCancelled or Task.checkCancellation() inside the for-await loop that iterates damus_state.nostrNetwork.reader.advancedStream(filters: [filter], streamMode: .ndbAndNetworkParallel(...)) so it exits promptly when cancelled.
🤖 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/Features/NIP05/Models/NIP05DomainEventsModel.swift`:
- Around line 116-118: The loop in streamItems() that calls
check_nip05_validity(pubkey:damus_state:) for each pubkey from
scan_all_cached_profiles launches uncontrolled Task.detached bursts and must be
throttled: replace the per-pubkey fire-and-forget with a limited-concurrency
executor (e.g., an AsyncSemaphore or an OperationQueue with
maxConcurrentOperationCount) that batches or gates requests (e.g., process
authors in chunks or await semaphore.acquire/release around each validation
Task), deduplicate the authors list before scheduling, and ensure subscribe()
does not re-trigger validations unnecessarily (cache validated pubkeys and
short-circuit in the scheduler). Use the existing check_nip05_validity function
name and the streamItems()/subscribe()/scan_all_cached_profiles call sites to
locate where to implement the semaphore/batching and caching logic.
In `@damus/Shared/Components/GroupedTimeline/GroupedTimelineGrouper.swift`:
- Around line 76-78: The current use of maxNotesPerUser with groupsByAuthor =
groupsByAuthor.filter { $0.value.count <= maxNotes } removes entire author
groups when they exceed the threshold; change this so the setting caps displayed
notes per author instead of excluding them by: when values.maxNotesPerUser is
set, transform groupsByAuthor by truncating each author’s value array to at most
maxNotes (e.g., map each entry to keep only the first N notes) rather than
filtering out authors entirely; alternatively, if the real intention is to
exclude prolific posters, rename maxNotesPerUser to spamThreshold and add a doc
comment on the new name clarifying exclusion semantics so callers aren’t misled.
- Around line 53-88: The grouping work in GroupedTimelineGrouper.group(...)
(which calls parseFilteredWords(...) and containsFilteredWords(...)) is
CPU-heavy and must be moved off the main thread; update GroupedListView.body to
kick off an async background task (Task { } or DispatchQueue.global().async)
that calls GroupedTimelineGrouper.group(events:filter:values:now:), store the
result in a `@State` or `@StateObject` cache (e.g. groupedResults) and then dispatch
the final AuthorGroup array back to the main thread to assign the state for
rendering; ensure you still use values.maxNotesPerUser and other
GroupedFilterValues inside the background task, debounce or re-run the task when
events/filter/values change, and avoid doing lowercasing/splitting/searching on
the main thread by keeping those operations inside group().
---
Duplicate comments:
In @.beads/issues.jsonl:
- Line 9: The JSON object with id "damus-ir1" is missing the required created_by
field; update the record in .beads/issues.jsonl by adding a created_by property
(e.g., a user identifier or system actor) to the object with id "damus-ir1" so
the schema includes created_by alongside existing fields like id, title, status,
priority, issue_type, created_at, updated_at, and closed_at; ensure the new
created_by value follows the same format used elsewhere in the file.
In `@damus.xcodeproj/project.pbxproj`:
- Around line 6379-6384: The six GroupedTimeline source entries
(GroupedFilterButton.swift, GroupedFilterSettingsView.swift,
GroupedAuthorRow.swift, GroupedListView.swift, GroupedTimelineGrouper.swift,
GroupedModeQueueManager.swift) are incorrectly present in the ShareExtension and
HighlighterActionExtension PBXSourcesBuildPhase sections; remove their
PBXBuildFile entries from those extension Sources build phases so only the main
damus target's PBXSourcesBuildPhase contains them, ensuring you delete the
corresponding build-file UUID lines (e.g., CB20AABE3C7B960961C0F914,
CF86249414FD714F8E45AA55, D93D3D7A501A49D1F86669A3, 91E25E8CB81B797E7FCC4D25,
B1B2B3B4B5B6B7B8B9B0C1C2, C1C2C3C4C5C6C7C8C9C0D1D2) from the extension
PBXSourcesBuildPhase blocks and leave them only in the main target's Sources
build phase.
- Around line 138-139: The project.pbxproj contains hand-crafted patterned UUIDs
for GroupedTimelineGrouper.swift, GroupedModeQueueManager.swift and their test
files (and related PBXBuildFile/PBXFileReference/PBXGroup entries like the
B1…/C1…/A1… groups listed), so remove the existing references for
GroupedTimelineGrouper.swift, GroupedModeQueueManager.swift,
GroupedTimelineGrouperTests.swift and GroupedModeQueueManagerTests.swift from
the project (delete their PBXBuildFile and PBXFileReference entries and any
group children entries referencing those IDs), then re-add those four files
using Xcode’s “Add Files to…” dialog so Xcode generates proper random UUIDs;
verify no patterned IDs remain in PBXBuildFile, PBXFileReference, PBXGroup or
PBXSourcesBuildPhase entries (search for the D1D2D3…, D2D3D4…, E1E2E3…, E2E3E4…
style IDs) before committing.
In `@damus/Features/NIP05/Models/NIP05.swift`:
- Around line 184-189: The .friends_of_friends branch is performing per-pubkey
synchronous profile checks on the `@MainActor` via matches_domain (which calls
profiles.is_validated and profiles.lookup); move the expensive per-pubkey work
off the main actor by collecting contacts.get_friend_of_friends_list(), then run
the domain-matching loop inside a Task.detached (or other background Task) that
calls matches_domain for each pubkey and accumulates matching pubkeys, and
finally switch back to the main actor to insert the results into authors (or
call await MainActor.run { authors.formUnion(matching) }); reference
contacts.get_friend_of_friends_list(), the .friends_of_friends switch arm,
matches_domain, profiles.is_validated, profiles.lookup, and
authors.insert(pubkey) when making the change.
In `@damus/Features/NIP05/Models/NIP05DomainEventsModel.swift`:
- Around line 135-144: Add concise docstrings to the methods load_more(),
fetch_older(until:), and add_event(_: ) describing their purpose, parameters,
return behavior, and side effects: for load_more explain it triggers loading
older events when not already loading and has_more is true; for
fetch_older(until:) document the meaning of the until parameter, that it
performs async fetch and updates model state (e.g., loading_more, has_more,
events), and that it runs on Task; for add_event(_: ) describe what kind of
Event it accepts, how it mutates events collection and any threading/ordering
guarantees. Attach these comments directly above each function signature in
NIP05DomainEventsModel so future readers understand inputs, outputs, and state
changes.
- Around line 170-174: fetch_older currently only clears loading_more when
advancedStream terminates, leaving has_more true and causing repeated empty
fetches; modify fetch_older to track whether an .eose was received (e.g., a
local flag) and in the stream termination/finally path set has_more = false if
no .eose was seen (or if the stream ended/cancelled), so that load_more() stops
requesting when the stream closes without EOSE; update references in the
fetch_older function to use this flag and ensure loading_more is still cleared.
- Around line 70-77: set_friend_filter is resetting the active time-window
because it calls subscribe(resetEvents: false) which causes subscribe to set
self.since = nil; update set_friend_filter so it preserves and forwards the
current since to subscribe (e.g. call subscribe(resetEvents: false, since:
self.since) or ensure subscribe(resetEvents: false) does not clear self.since),
referencing set_friend_filter(...) and subscribe(...) and the self.since
property when making the change.
- Around line 44-62: The subscribe(resetEvents: Bool = true, since: UInt32? =
nil) call currently resets pagination state even when resetEvents is false,
causing streamItems() to re-fetch overlapping events that EventHolder.insert
deduplicates; change the logic so that events.reset(), used_initial_page,
has_more, last_loaded_count (and any other pagination flags) are only reset when
resetEvents is true, and when resetEvents is false call events.flush() and
preserve used_initial_page, has_more, last_loaded_count so streamItems()
continues from the current pagination state; keep cancelling loadingTask and
starting a new Task to call streamItems(), but avoid clobbering pagination
variables when resetEvents == false.
- Around line 86-88: The hardcoded "filter.limit = 5000" in
NIP05DomainEventsModel (the block that sets filter.since and filter.limit when
"since" is present) can cause memory pressure for popular domains; change it to
use a configurable constant or adaptive cap (e.g., a new property like
maxEventsPerDomain or fetchBatchSize on NIP05DomainEventsModel or a settings
object) instead of the literal 5000, and/or implement pagination (fetch in
smaller chunks and accumulate) when grouped mode is enabled; update the code
that assigns filter.limit to use that constant or computed value and add tests
or comments where the grouped-mode behavior is handled.
In `@damus/Features/NIP05/Views/NIP05DomainTimelineView.swift`:
- Around line 14-27: The NIP05ViewMode enum is unused because the view mode is
controlled by filterSettings.enableGroupedMode; remove the dead enum
NIP05ViewMode or refactor code to use it consistently: either replace usages of
filterSettings.enableGroupedMode with NIP05ViewMode (e.g., add a property on
filterSettings of type NIP05ViewMode and update view toggles) or delete the
NIP05ViewMode declaration and any references to it to avoid dead code; locate
the enum NIP05ViewMode in NIP05DomainTimelineView.swift and update the
filterSettings usage (enableGroupedMode) or the view toggle code to ensure only
one source of truth remains.
- Around line 189-192: handleOnAppear currently returns early when
model.events.all_events is non-empty, so after handleOnDisappear calls
unsubscribe() the view never re-establishes the relay; remove or change that
guard so handleOnAppear calls model.subscribe(since: groupedSince) whenever the
model is not currently subscribed. Specifically, update handleOnAppear to call
model.subscribe(since: groupedSince) unconditionally or guard on a subscription
flag (e.g., model.isSubscribed / model.isActive) instead of
model.events.all_events, and ensure handleOnDisappear still calls
model.unsubscribe() to cancel the stream.
- Around line 108-111: The hard-coded paddings in NIP05DomainTimelineView
(.padding(.leading, 45) and .padding(.top, 50)) are fragile; replace them with
values derived from the view's safe area insets (use GeometryReader to read
geometry.safeAreaInsets or use
UIApplication.shared.windows.first?.safeAreaInsets) and apply .padding(.leading,
safeAreaInsets.leading + X) and .padding(.top, safeAreaInsets.top + Y) so the
alignment after the back button and top spacing adapt to Dynamic
Island/notch/no-notch devices; update the layout code around the block
containing .padding(.leading, 45) and .padding(.top, 50) accordingly.
In `@damus/Features/Profile/Views/CondensedProfilePicturesView.swift`:
- Around line 19-22: The initializer for CondensedProfilePicturesView
(init(state:pubkeys:maxPictures:size:)) is missing its own triple-slash
docstring; add a /// docstring directly above this init that documents each
parameter (state, pubkeys, maxPictures, size) with brief descriptions and any
defaults/constraints, matching the existing project's docstring style and tone
so the init's parameters are covered per the coding guidelines.
In `@damus/Features/Timeline/Models/HomeModel.swift`:
- Line 846: The task cancellation race occurs in backfillForGroupedMode: after
backfillTask?.cancel() a previous task T1 can resume, hit the guard and break
out of the streaming loop but still proceed to insert allEvents and set
backfilling = false, clearing state while a new task T2 runs; to fix, ensure the
cancelled task aborts before performing any insertion or mutating backfilling by
checking Task.isCancelled (or comparing backfillTask identity) immediately after
collection: if cancelled, return early without touching EventHolder/EventCache
or backfilling. Also minimize actor hops by moving the per-event awaits
(favoriteEvents.insert and insert_home_event) into a single MainActor.run block
that performs batched insertion and then updates backfilling, so heavy work runs
off-main-thread and state mutation happens atomically on the main actor.
In `@damus/Features/Timeline/Views/PostingTimelineView.swift`:
- Around line 222-272: Add concise docstrings for the private grouped-mode
helpers: groupedContentView, flushActiveSource(),
backfillForGroupedMode(showLoading:), and refreshFromNdb(). For
groupedContentView describe that it builds the grouped timeline ScrollView, the
source selection logic (favorites vs home), and the onAppear targeted refresh
behavior; for flushActiveSource explain it calls GroupedModeQueueManager.flush
for the current timeline_source; for backfillForGroupedMode(showLoading:) state
what since is computed from groupedFilterSettings and that it invokes
home.backfillForGroupedMode with the source and showLoading flag; for
refreshFromNdb state that it computes since similarly and calls
home.refreshGroupedFromNdb with the current source. Include `@MainActor` where
present and keep each comment short (one or two sentences) and placed
immediately above the corresponding declaration.
- Around line 224-254: groupedContentView currently passes raw events and filter
into GroupedListView causing GroupedTimelineGrouper.group(...) to run on every
SwiftUI render; memoize the grouped result in this view using a `@State` (or
`@StateObject`) property and only recompute when eventsSource or
groupedFilterSettings change (e.g., compute in onChange or didSet and assign to
the `@State`), then pass the already-grouped collection into GroupedListView so
grouping work occurs only when the input events or filter actually update.
- Around line 179-195: The current onAppear sets
groupedFilterSettings.enableGroupedMode which triggers the onChange handler and
may double-run backfillForGroupedMode; add a short-lived isInitialAppear Bool
(e.g. isInitialAppear = true) and set it true immediately before you change
groupedModeStored/onAppear, then in the onChange(of:
groupedFilterSettings.enableGroupedMode) handler ignore the change if
isInitialAppear is true (and reset isInitialAppear = false) so
flushActiveSource() and backfillForGroupedMode() only run once; reference
groupedFilterSettings.enableGroupedMode, groupedModeStored, onAppear, onChange,
flushActiveSource(), backfillForGroupedMode(), and home.backfilling when
applying this guard.
In `@damus/Shared/Components/GroupedTimeline/GroupedAuthorRow.swift`:
- Around line 50-65: Add a concise docstring above the computed property
postCountBadge describing its purpose and behavior: state that it returns a View
showing the user's post count as a capsule-styled badge, mention that it adapts
font size and horizontal padding based on the postCount value and uses
DamusColors for foreground/background/stroke styling; reference the related
symbol postCount and note that the badge is a Capsule with fill and stroked
overlay so future maintainers understand the visual and conditional logic.
---
Nitpick comments:
In `@damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift`:
- Around line 58-64: The computed property allAuthors currently maps
model.events.all_events to extract pubkeys and deduplicates them on every
SwiftUI render, causing large temporary allocations; change this so the ordered
unique authors list is cached and only recomputed when events change — either
move the deduplication into the backing model (e.g., expose a cached property on
the model like uniqueAuthors) or store it in the view as a
`@State/`@StateObject/@ObservedObject-backed property that you update in an
onReceive or didSet when model.events changes; ensure you use the same ordering
(event order + filter authors) and keep references to model.filter.authors and
model.events.all_events when updating so UI reads allAuthors cheaply.
- Around line 103-128: The two near-identical functions notesFromAuthorsString
and friendsOfFriendsString should be consolidated into a single parameterized
helper: extract the shared logic (fetch up to 3 display names, truncate, and
switch on authors.count to format localized strings) into a new function (e.g.,
authorsSummaryString(authors: [Pubkey], ndb: Ndb, locale: Locale, emptyMessage:
String, nameResolver: (Pubkey, Ndb) -> Profile?)), and update
notesFromAuthorsString and friendsOfFriendsString to call that helper passing
the appropriate emptyMessage and a name-resolver closure that uses either
lookup_profile_and_copy or lookup_profile as needed; ensure the helper uses
Profile.displayName(...).username.truncate(maxLength: 20) and preserves the
localized format keys (including notes_from_three_and_others).
In `@damus/Features/NIP05/Views/NIP05DomainTimelineView.swift`:
- Around line 183-186: The computed property groupedSince returns nil when
filterSettings.enableGroupedMode is false but handleOnAppear always forwards
groupedSince into subscribe, which can silently change behavior when the user
toggles modes; add a concise inline comment next to the groupedSince property
explaining that it intentionally returns nil for timeline mode and that
handleOnAppear/subscribe rely on nil to indicate timeline (so toggling
grouped→timeline while the view is alive is expected), and mention the
interaction between groupedSince, handleOnAppear, and subscribe so future
readers understand the intended contract.
In `@damus/Features/Timeline/Models/HomeModel.swift`:
- Around line 853-866: The inner for-await loop over
damus_state.nostrNetwork.reader.advancedStream should use a labeled loop to
avoid the confusing post-switch check for .eose; add a label (e.g., streamLoop:)
before the for await, then inside the switch case .eose use break streamLoop to
exit the loop directly, remove the trailing if case .eose = item { break }
check, and keep the existing .event handling (await lender.justUseACopy { ... })
and the .ndbEose/.networkEose continue behavior unchanged so semantics via
should_show_event and allEvents remain the same.
- Around line 766-772: The comment describing the backfilling behavior should be
moved from the property declaration to the actual method it documents: remove
the method-level docstring currently above private var backfillTask and paste it
above the backfillForGroupedMode(...) function so the description matches the
implementation; then add concise property-level docs for private var
backfillTask: Task<Void, Never>? (e.g., "Task managing backfill work for grouped
mode"), private var ndbRefreshTask: Task<Void, Never>? (e.g., "Task for
refreshing ndb state"), and private let backfillPageLimit: UInt32 (e.g.,
"Maximum events to request per backfill page") to satisfy doc coverage and
coding guidelines.
- Around line 894-913: refreshSingleAuthorEvents currently spawns an unmanaged
Task that cannot be cancelled; add a stored Task? property (e.g.,
refreshSingleAuthorTask or authorEventsTask) alongside
homeHandlerTask/favoritesHandlerTask, cancel the existing task (.cancel())
before creating a new one, assign the newly created Task to that property, and
ensure the Task's loop honors cancellation (check Task.isCancelled or
Task.checkCancellation() inside the for-await loop that iterates
damus_state.nostrNetwork.reader.advancedStream(filters: [filter], streamMode:
.ndbAndNetworkParallel(...)) so it exits promptly when cancelled.
In `@damus/Features/Timeline/Views/PostingTimelineView.swift`:
- Around line 264-272: The subtraction to produce a UInt32 timestamp in
backfillForGroupedMode and refreshFromNdb can wrap if
groupedFilterSettings.timeRange.seconds is larger than the current time; guard
by computing a non-negative difference before casting: compute let delta =
max(0, Int(Date().timeIntervalSince1970) -
Int(groupedFilterSettings.timeRange.seconds)) (or use a saturating/subtraction
helper) and then pass UInt32(delta) to home.backfillForGroupedMode(...) and
home.refreshGroupedFromNdb(...), referencing the functions
backfillForGroupedMode, refreshFromNdb and the
groupedFilterSettings.timeRange.seconds value to locate the change.
In `@damus/Shared/Components/GroupedTimeline/GroupedAuthorRow.swift`:
- Around line 68-85: The regex in eventPreviewText currently only removes
alphanumerics after "nostr:" and can leave fragments (update the replacement to
match broader identifiers); change the replacement pattern used on content (the
.replacingOccurrences(of: "nostr:[a-zA-Z0-9]+", ...)) to a pattern that captures
underscores or any non-whitespace sequence (e.g., "nostr:[a-zA-Z0-9_]+" or
"nostr:\\S+"), so the cleaned string removes full nostr: references before
trimming and length-checking.
In `@damus/Shared/Components/GroupedTimeline/GroupedFilterSettingsView.swift`:
- Around line 12-39: Annotate the ObservableObject class GroupedFilterSettings
with `@MainActor` to ensure all published-property mutations are main-thread
enforced, and add a short docstring above the reset() method describing its
purpose (restores defaults) per coding guidelines; update the class declaration
(GroupedFilterSettings) and the reset() method doc comment accordingly so the
compiler and reviewers can verify main-thread safety and documentation.
In `@damus/Shared/Components/GroupedTimeline/GroupedTimelineGrouper.swift`:
- Around line 92-139: Add missing doc comments for the four internal static
methods: parseFilteredWords(_:),
shouldIncludeEvent(_:filter:values:wordsList:cutoff:),
containsFilteredWords(_:wordsList:), and isTooShort(_:hideShortNotes:). For each
method add a /// comment that briefly describes the method's purpose, lists and
documents parameters (e.g., raw, event, filter, values, wordsList, cutoff,
hideShortNotes) and states the return value (what the Bool or [String]
represents); keep comments concise and consistent with the existing style used
for group() and surrounding types.
Comment on lines
+53
to
+88
| static func group( | ||
| events: [NostrEvent], | ||
| filter: (NostrEvent) -> Bool, | ||
| values: GroupedFilterValues, | ||
| now: Date = Date() | ||
| ) -> [AuthorGroup] { | ||
| let wordsList = parseFilteredWords(values.filteredWords) | ||
| let epoch = UInt32(now.timeIntervalSince1970) | ||
| let cutoff = epoch > values.timeRangeSeconds ? epoch - values.timeRangeSeconds : 0 | ||
|
|
||
| var groupsByAuthor: [Pubkey: (latest: NostrEvent, count: Int)] = [:] | ||
|
|
||
| for event in events { | ||
| guard shouldIncludeEvent(event, filter: filter, values: values, wordsList: wordsList, cutoff: cutoff) else { continue } | ||
|
|
||
| if let existing = groupsByAuthor[event.pubkey] { | ||
| let newLatest = event.created_at > existing.latest.created_at ? event : existing.latest | ||
| groupsByAuthor[event.pubkey] = (latest: newLatest, count: existing.count + 1) | ||
| } else { | ||
| groupsByAuthor[event.pubkey] = (latest: event, count: 1) | ||
| } | ||
| } | ||
|
|
||
| if let maxNotes = values.maxNotesPerUser { | ||
| groupsByAuthor = groupsByAuthor.filter { $0.value.count <= maxNotes } | ||
| } | ||
|
|
||
| return groupsByAuthor | ||
| .map { AuthorGroup(pubkey: $0.key, latestEvent: $0.value.latest, postCount: $0.value.count) } | ||
| .sorted { | ||
| if $0.latestEvent.created_at != $1.latestEvent.created_at { | ||
| return $0.latestEvent.created_at > $1.latestEvent.created_at | ||
| } | ||
| return $0.pubkey.id.lexicographicallyPrecedes($1.pubkey.id) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all call sites of GroupedTimelineGrouper.group and their surrounding concurrency context
rg -n "GroupedTimelineGrouper\.group\b" --type=swift -B 10 -A 5Repository: damus-io/damus
Length of output: 19771
🏁 Script executed:
#!/bin/bash
# Find shouldIncludeEvent implementation
rg -n "func shouldIncludeEvent" --type=swift -A 20
# Also get more context around GroupedListView line 30
cat -n damus/Shared/Components/GroupedTimeline/GroupedListView.swift | sed -n '15,45p'Repository: damus-io/damus
Length of output: 3807
Move expensive grouping computation off the main thread.
The group() method is called inside GroupedListView.body (line 30), which is main-thread-bound in SwiftUI. With large event sets, the O(n × w × k) string operations—lowercasing event content, splitting filter words, and searching for word matches in containsFilteredWords()—will block the UI. Per coding guidelines, expensive computations must run on background threads using Task {}, DispatchQueue.global(), or Swift concurrency (async/await).
Refactor GroupedListView.body to compute groups asynchronously and cache the result, dispatching the computed groups back to the main thread for rendering.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus/Shared/Components/GroupedTimeline/GroupedTimelineGrouper.swift` around
lines 53 - 88, The grouping work in GroupedTimelineGrouper.group(...) (which
calls parseFilteredWords(...) and containsFilteredWords(...)) is CPU-heavy and
must be moved off the main thread; update GroupedListView.body to kick off an
async background task (Task { } or DispatchQueue.global().async) that calls
GroupedTimelineGrouper.group(events:filter:values:now:), store the result in a
`@State` or `@StateObject` cache (e.g. groupedResults) and then dispatch the final
AuthorGroup array back to the main thread to assign the state for rendering;
ensure you still use values.maxNotesPerUser and other GroupedFilterValues inside
the background task, debounce or re-run the task when events/filter/values
change, and avoid doing lowercasing/splitting/searching on the main thread by
keeping those operations inside group().
Comment on lines
+76
to
+78
| if let maxNotes = values.maxNotesPerUser { | ||
| groupsByAuthor = groupsByAuthor.filter { $0.value.count <= maxNotes } | ||
| } |
Contributor
There was a problem hiding this comment.
maxNotesPerUser filter silently excludes prolific authors rather than capping them.
filter { $0.value.count <= maxNotes } removes the entire author group when their post count exceeds the threshold. A user who sets maxNotesPerUser = 3 expecting to see at most 3 posts per author will instead see zero posts from anyone who posted 4 or more times. The parameter name implies a display cap, not a spam-exclusion gate.
If the intent is spam filtering, rename to spamThreshold (or similar) and add a doc comment clarifying the exclusion semantics. If the intent is a display cap, change the logic to retain the author and truncate the count:
🛠 Proposed display-cap approach
- if let maxNotes = values.maxNotesPerUser {
- groupsByAuthor = groupsByAuthor.filter { $0.value.count <= maxNotes }
- }
+ // Cap displayed post count per author rather than excluding prolific authors.
+ if let maxNotes = values.maxNotesPerUser {
+ groupsByAuthor = groupsByAuthor.mapValues { entry in
+ (latest: entry.latest, count: min(entry.count, maxNotes))
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus/Shared/Components/GroupedTimeline/GroupedTimelineGrouper.swift` around
lines 76 - 78, The current use of maxNotesPerUser with groupsByAuthor =
groupsByAuthor.filter { $0.value.count <= maxNotes } removes entire author
groups when they exceed the threshold; change this so the setting caps displayed
notes per author instead of excluding them by: when values.maxNotesPerUser is
set, transform groupsByAuthor by truncating each author’s value array to at most
maxNotes (e.g., map each entry to keep only the first N notes) rather than
filtering out authors entirely; alternatively, if the real intention is to
exclude prolific posters, rename maxNotesPerUser to spamThreshold and add a doc
comment on the new name clarifying exclusion semantics so callers aren’t misled.
Add ndb_txn_is_valid() helper and validation checks across multiple nostrdb functions to prevent crashes from invalid/null transactions. This adds defensive guards in: - ndb_get_tsid - ndb_lookup_by_key - ndb_get_profile_by_pubkey - ndb_get_note_by_id - ndb_get_indexkey_by_id - ndb_read_last_profile_fetch 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Consolidates duplicate domain-matching logic into NIP05DomainHelpers struct to improve code maintainability and reduce duplication across timeline views. Key additions: - matches_domain: Check if pubkey has matching NIP-05 (validated or raw) - validated_authors_for_domain: Get all validated authors for a domain - scan_all_cached_profiles: Scan nostrdb cache for domain discovery - authors_for_domain: Get authors based on WOT mode (friends vs all) - ordered_domain_authors: Order authors with events first for responsive UI The scan_all_cached_profiles function enables discovery of users by NIP-05 domain without requiring them to be in your social graph. It queries locally cached metadata events (kind:0) which is fast and doesn't require network requests. Two modes of operation: - WOT ON: Filters to friends-of-friends with matching domain - WOT OFF: Scans all cached profiles for discovery Signed-off-by: alltheseas Closes: damus-io#3375 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add a `size` parameter to control profile picture diameter, defaulting to 32pt for backwards compatibility. The horizontal offset between stacked pictures now scales proportionally (62.5% of diameter). This enables compact layouts like the NIP-05 domain header to use smaller avatars (24pt) while maintaining proper overlap and spacing. Closes: damus-io#3379 Signed-off-by: alltheseas <64376233+alltheseas@users.noreply.github.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add filter settings infrastructure for NIP-05 domain feeds: NIP05FilterSettings (ObservableObject): - enableGroupedMode: Toggle between grouped and timeline views - timeRange: 24h or 7d filter window - compactView/showLastNotePreview: Display options - includeReplies: Toggle reply visibility - hideShortNotes: Filter short/emoji-only notes - filteredWords: Comma-separated keyword exclusions - maxNotesPerUser: Limit prolific posters NIP05FilterButton: - Compact pill-style button showing current state (e.g., "Grouped · 24h") - Opens settings sheet on tap - Shows loading indicator during fetches NIP05FilterSettingsView: - Full settings sheet with all filter options - Reset and Apply buttons - Matches fevela-style grouping behavior Closes: damus-io#3379 Signed-off-by: alltheseas <64376233+alltheseas@users.noreply.github.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add views for displaying domain posts grouped by author: NIP05GroupedAuthorRow: - Shows author profile pic, name, and post count badge - Displays preview of most recent post - Relative timestamp for latest activity - Tapping navigates to author's profile NIP05GroupedListView: - Groups all events by author pubkey - Applies filter settings (time range, keywords, short notes) - Shows authors sorted by most recent activity - Empty state with helpful filter adjustment hint Filtering logic (fevela-style): - Time range: 24h or 7d window - Keyword exclusion: Case-insensitive, min 2 chars - Short note detection: <10 chars, emoji-only, single word - Max notes per user: Exclude prolific posters Closes: damus-io#3379 Signed-off-by: alltheseas <64376233+alltheseas@users.noreply.github.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Update timeline view to support grouped mode and filter settings: Layout changes: - Reduce header height from 200pt to 160pt - Move filter button to title row (next to domain name) - Give friends view full width for text display Mode support: - Grouped mode: Shows NIP05GroupedListView with author rows - Timeline mode: Shows chronological TimelineView - Filter settings sheet accessible from header button Code organization: - Extract subviews into private computed properties - Extract event handlers into named functions - Add MARK sections for logical grouping Also adds new Swift files to Xcode project targets. Closes: damus-io#3379 Signed-off-by: alltheseas <64376233+alltheseas@users.noreply.github.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Move and rename NIP05-prefixed grouped view components to damus/Shared/Components/GroupedTimeline/ for reuse across feeds: - NIP05GroupedListView → GroupedListView - NIP05GroupedAuthorRow → GroupedAuthorRow - NIP05FilterButton → GroupedFilterButton - NIP05FilterSettings → GroupedFilterSettings - NIP05FilterSettingsView → GroupedFilterSettingsView - NIP05TimeRange → GroupedTimeRange - Extract GroupedTimelineGrouper into standalone file - Extract GroupedModeQueueManager into standalone file Update NIP05DomainTimelineView to use new shared type names. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add GroupedFilterButton pill in header next to SignalView - Hide Notes/Notes+Replies picker when grouped mode is active - Switch body between TabView (timeline) and GroupedListView (grouped) - Persist grouped mode via @SceneStorage bidirectional sync - Reset headerOffset to 0 when toggling grouped mode - Flush EventHolder queue on mode toggle and source switch - Use .posts_and_replies base filter; reply filtering handled by settings Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
GroupedTimelineGrouperTests (15 tests): - Empty input returns empty groups - Events correctly grouped by author with accurate counts - Groups sorted by most recent activity - Replies excluded when includeReplies is false - Replies included when includeReplies is true - Events outside time range excluded - Events with filtered keywords excluded - Short filter words (< 2 chars) ignored - Max notes per user excludes prolific authors - Short notes excluded/included based on setting - Single-word notes excluded when hideShortNotes enabled - Content filter passthrough blocks events - parseFilteredWords trims, lowercases, and filters short words GroupedModeQueueManagerTests (3 tests): - Flush moves queued events to main list - Flush disables queueing - Flush is idempotent (no crash/duplicates on second call) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
alltheseas
force-pushed
the
nip05-grouped-ui
branch
2 times, most recently
from
1fab4f0 to
2cefb11
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
damusTests/GroupedModeQueueManagerTests.swift (2)
61-77: Expand idempotency coverage — flush on a never-queuedEventHolderis untested.
testFlushIsIdempotentonly tests the "flush-flush" case where the holder already went through one successful flush. The default state ofEventHolderhasshould_queue = falseandincomingempty; callingGroupedModeQueueManager.flush(source:)directly on that state (no priorset_should_queue(true)) is not covered and could surface a different code path inEventHolder.flush().Consider adding a test case like:
func testFlushOnDefaultHolderIsNoOp() { let holder = EventHolder() // should_queue defaults to false; no events added GroupedModeQueueManager.flush(source: holder) XCTAssertEqual(holder.events.count, 0) XCTAssertFalse(holder.should_queue) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damusTests/GroupedModeQueueManagerTests.swift` around lines 61 - 77, Add a new unit test that calls GroupedModeQueueManager.flush(source:) on a fresh EventHolder (without calling set_should_queue or inserting events) to ensure the flush is a no-op; create a test method (e.g., testFlushOnDefaultHolderIsNoOp) that constructs EventHolder, invokes GroupedModeQueueManager.flush(source: holder), then asserts holder.events.count == 0 and holder.should_queue == false to cover the default-holder code path in EventHolder.flush().
16-26: Add docstring coverage tomakeEventper coding guidelines.The helper
makeEventhas no documentation comment. As per coding guidelines, all added or modified code must have docstring coverage.✏️ Proposed docstring
+ /// Creates a signed `NostrEvent` for use in unit tests. + /// - Parameters: + /// - content: The note body; defaults to a string long enough to pass short-note filters. + /// - keypair: Signing keypair; defaults to the shared `test_keypair` fixture. + /// - secondsAgo: How far in the past to backdate `createdAt`; defaults to 100 s. + /// - Returns: A valid `NostrEvent`, force-unwrapped (crashes on signing failure). private func makeEvent(As per coding guidelines: "Ensure docstring coverage for any code added or modified."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damusTests/GroupedModeQueueManagerTests.swift` around lines 16 - 26, The helper function makeEvent lacks a documentation comment; add a concise docstring above makeEvent describing its purpose, parameters (content: String default, keypair: Keypair default test_keypair, secondsAgo: UInt32 default 100) and the return value (NostrEvent), and note that the createdAt uses current time minus secondsAgo; update the docstring format to match project style (brief summary line, parameter descriptions, and return description) for the makeEvent helper.damus/Shared/Components/GroupedTimeline/GroupedAuthorRow.swift (1)
68-68:eventPreviewText(_:)should beprivate.The method is only used within
body; leaving it without an access modifier exposes it asinternalunnecessarily.- func eventPreviewText(_ event: NostrEvent) -> String { + private func eventPreviewText(_ event: NostrEvent) -> String {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Shared/Components/GroupedTimeline/GroupedAuthorRow.swift` at line 68, The method eventPreviewText(_:) is declared without an access modifier and should be made private since it's only used within the view's body; update the declaration of eventPreviewText(_ event: NostrEvent) to be private (e.g., private func eventPreviewText(_ event: NostrEvent) -> String) so the helper is not exposed beyond GroupedAuthorRow and locate it near the body implementation to keep related code together.damus/Features/Timeline/Models/HomeModel.swift (1)
894-913:refreshSingleAuthorEventsspawns an unmanaged, uncancellableTask.The bare
Task { }created here is not stored in any handle. Rapid back-and-forth navigation (profile visit → timeline → profile visit) will stack multiple concurrent subscriptions for the same author, each holding an open relay connection until EOSE arrives.♻️ Proposed fix — store the handle for single-author refresh
+ private var singleAuthorRefreshTask: Task<Void, Never>? + func refreshSingleAuthorEvents(pubkey: Pubkey, since: UInt32, source: TimelineSource) { ... - Task { + singleAuthorRefreshTask?.cancel() + singleAuthorRefreshTask = Task { for await item in ... {🤖 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 894 - 913, The code spawns an unmanaged Task for the single-author subscription; create a cancellable Task handle property on HomeModel (e.g., refreshSingleAuthorTask: Task<Void, Never>?), cancel and nil out any existing handle before assigning the new Task, and inside the Task ensure you clear the handle when you return on .eose or on cancellation; reference the existing subscription creation using damus_state.nostrNetwork.reader.advancedStream(filters: [filter], streamMode: .ndbAndNetworkParallel(...)) and the surrounding logic in refreshSingleAuthorEvents (or the method containing that Task), use weak self to avoid retain cycles and check Task.isCancelled if needed.damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift (2)
58-64:allAuthorsrecomputed on every SwiftUI body evaluation.This computed property iterates all events, allocates a
Set, and deduplicates on every view render triggered by any@Publishedchange onmodel. For domains with many events this could introduce frame drops.Consider caching the result in the model or using a
@State/memoization pattern updated only whenmodel.eventsactually changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift` around lines 58 - 64, The computed property allAuthors iterates model.events, allocates a Set, and deduplicates on every SwiftUI body evaluation which is expensive; move this work out of the view by caching it and only recomputing when events change: either add a cached property/method on the underlying model (e.g., NIP05DomainTimelineModel.cachedAuthors or recomputeAuthors() that updates when model.events mutates) or make the view hold `@State` private var allAuthors and update that state in an onChange(or onReceive) handler watching model.events or model.events.all_events; preserve the current deduplication logic (combining eventAuthors + filter.authors and filtering by seen Set) when populating the cache/state.
103-128:notesFromAuthorsStringandfriendsOfFriendsStringare near-duplicates.The two functions share identical switch logic for cases 1–default, differing only in the ndb lookup API, the zero-case string, and localization comments. This violates DRY and makes future localization changes error-prone.
Additionally,
notesFromAuthorsStringusesndb.lookup_profile_and_copy(allocates a copy) while the olderfriendsOfFriendsStringuses the zero-copyndb.lookup_profile(borrow:)API. Per coding guidelines, prefer nostrdb facilities — the borrow pattern is more efficient and should be used consistently.Consider extracting the name-resolution and switch formatting into a shared helper, passing only the zero-case string as a parameter.
Also applies to: 151-179
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift` around lines 103 - 128, Extract the shared formatting and name-resolution logic from notesFromAuthorsString and friendsOfFriendsString into a single helper (e.g., formatAuthorsString(authors: [Pubkey], ndb: Ndb, zeroCaseKey: String, zeroCaseComment: String, locale: Locale)) that: 1) resolves up to three display names using the zero-copy API ndb.lookup_profile(borrow:) and Profile.displayName, truncates to 20 chars, 2) chooses the correct zero-case localized string passed in (instead of hardcoding), and 3) runs the same switch/format logic including localizedStringFormat for the default case; then update notesFromAuthorsString and friendsOfFriendsString to call this helper (passing their respective zero-case strings/comments) and remove the duplicated switch and the use of ndb.lookup_profile_and_copy.damus/Features/NIP05/Models/NIP05DomainEventsModel.swift (3)
44-62:loading = trueis set afterloadingTaskis created — ordering is safe but non-obvious.Because the class is
@MainActor, theTaskbody won't start until the current synchronous scope yields, soloadingis set totruebeforestreamItems()begins. However, placingloading = truebefore theTaskcreation would make the intent clearer and prevent confusion during future refactors.♻️ Suggested reorder
+ loading = true + last_loaded_count = 0 loadingTask?.cancel() loadingTask = Task { await streamItems() } - loading = true - last_loaded_count = 0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Models/NIP05DomainEventsModel.swift` around lines 44 - 62, Move the assignment of loading to true before creating the asynchronous loadingTask in subscribe so the intent is explicit; specifically, in NIP05DomainEventsModel.subscribe set loading = true (and any related state like last_loaded_count if desired) prior to creating/loadingTask = Task { await streamItems() }, so the visible state already reflects that loading will start and avoids reliance on `@MainActor` scheduling semantics.
79-81: Remove unused private methodmatches_domain.The method at lines 79–81 has no call sites in this file. The
add_eventmethod explicitly skips domain checking (line 192 comment), andstreamItemsusesNIP05DomainHelpers.matches_domaindirectly. Remove to avoid confusion.Suggested removal
- private func matches_domain(_ pubkey: Pubkey) -> Bool { - NIP05DomainHelpers.matches_domain(pubkey, domain: domain, profiles: state.profiles) - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Models/NIP05DomainEventsModel.swift` around lines 79 - 81, The private helper function matches_domain(_ pubkey: Pubkey) is unused and causes confusion; remove the entire method declaration from NIP05DomainEventsModel (the unused matches_domain function) and ensure any callers use NIP05DomainHelpers.matches_domain directly (note add_event intentionally skips domain checking and streamItems already calls NIP05DomainHelpers.matches_domain), then run tests/compile to confirm no references remain.
116-118: Remove redundant NIP-05 validation at line 190.
check_nip05_validityalready validates all authors upfront at lines 116–118 before streaming events. The function has built-in deduplication (it returns early if a pubkey is already validated viaprofiles.is_validated(pubkey) == nil). The per-event call at line 190 inadd_eventis redundant since all events in the stream are guaranteed to come from the pre-validated authors, causing the function to exit early each time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@damus/Features/NIP05/Models/NIP05DomainEventsModel.swift` around lines 116 - 118, The loop in the initializer already calls check_nip05_validity(pubkey:pubkey, damus_state:state) for each author (with dedup via profiles.is_validated(pubkey)), so remove the redundant per-event validation inside add_event (the call to check_nip05_validity at the noted location) — either delete that call or replace it with a short comment noting validation is pre-done; keep all other logic in add_event unchanged.
🤖 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/Shared/Components/GroupedTimeline/GroupedTimelineGrouper.swift`:
- Around line 135-136: The single-word check in
GroupedTimelineGrouper.isTooShort currently uses "if words.count == 1 &&
content.count < 20 { return true }", which lets very long single words pass;
change it so any single-word note is considered too short when hideShortNotes is
enabled. Locate the isTooShort (or equivalent) method in GroupedTimelineGrouper
and replace the conditional with an unconditional single-word check (e.g., if
words.count == 1 { return true }) so single-word content is rejected regardless
of character count.
In `@damusTests/GroupedModeQueueManagerTests.swift`:
- Around line 11-12: Add missing Swift docstrings for the test helper and tests:
add a /// doc comment above makeEvent(content:keypair:secondsAgo:) describing
its purpose and documenting parameters and return value, and add /// doc
comments to each test method inside GroupedModeQueueManagerTests (the three test
methods present) briefly stating the scenario being tested and expected outcome;
keep wording concise and follow the project's docstring style (short summary
line, `@param` for parameters if used, and `@return` if applicable).
In `@damusTests/GroupedTimelineGrouperTests.swift`:
- Around line 232-243: The test testSingleWordExcludedWhenHideShortEnabled fails
because GroupedTimelineGrouper.isTooShort currently checks both words.count == 1
and content.count < 20, allowing long single-word notes (like
"Supercalifragilisticexpialidocious") to slip through; change isTooShort to
treat any single-word note as too short when hideShortNotes is enabled by
removing the content.count < 20 guard (i.e., return true when words.count == 1),
so GroupedTimelineGrouper.group will exclude single-word events as the test
expects.
---
Duplicate comments:
In `@damus.xcodeproj/project.pbxproj`:
- Around line 134-135: The PBX project contains hand-crafted, patterned UUIDs
for GroupedTimeline source entries (e.g. the GroupedTimeline file UUIDs like
A1A2A3A4…/A1B2C3D4…/B1B2B3B4…/C1C2C3C4…/F2E1D0C9…) and newly added test
build-file refs (e.g. fileRef identifiers referencing
GroupedTimelineGrouperTests.swift, GroupedModeQueueManagerTests.swift, etc. such
as D1D2D3D4…, D2D3D4D5…, E1E2E3E4…, E2E3E4E5…), which must be replaced with
Xcode-generated UUIDs; to fix, remove the hand-edited PBX entries for those
GroupedTimeline source files and their test build-file entries and re-add the
files using Xcode’s “Add Files to…” dialog so Xcode will generate proper random
24-hex identifiers for the items (ensure you update any references that pointed
to the old patterned UUID symbols like GroupedTimelineGrouperTests.swift and
GroupedModeQueueManagerTests.swift).
- Around line 6367-6372: The six GroupedTimeline Swift files
(GroupedFilterButton.swift, GroupedFilterSettingsView.swift,
GroupedAuthorRow.swift, GroupedListView.swift, GroupedTimelineGrouper.swift,
GroupedModeQueueManager.swift) are still listed in the Sources build phases for
extension targets; remove their PBXBuildFile entries from the
PBXSourcesBuildPhase blocks of the ShareExtension and HighlighterActionExtension
PBXNativeTarget definitions so the extensions no longer compile them, but keep
their PBXFileReference entries (and the PBXBuildFile entries) only in the main
damus app target’s Sources build phase; locate the PBXSourcesBuildPhase blocks
that contain the listed UUID lines and delete those specific file UUID lines
(and corresponding PBXBuildFile entries that reference those UUIDs under the
extension targets) to avoid linker/bloat issues, then re-run the verification
script to confirm only the app target compiles them.
In `@damus/Features/NIP05/Models/NIP05.swift`:
- Around line 185-189: The .friends_of_friends scan is running synchronously on
the MainActor and calls matches_domain (which triggers profiles.lookup →
ndb.lookup_profile), so offload the whole scan to a background task like the
.all case: call Task.detached to iterate contacts.get_friend_of_friends_list(),
perform matches_domain checks and build a local Set of pubkeys off-main-thread,
then switch back to the MainActor only once to merge that local set into the
actor-isolated authors (e.g., via await MainActor.run {
authors.formUnion(localSet) }); ensure matches_domain/profile lookups run
off-main-thread and avoid calling authors.insert from the background task.
In `@damus/Features/NIP05/Models/NIP05DomainEventsModel.swift`:
- Around line 70-77: Add missing docstrings for the modified/added methods:
set_friend_filter(_:), streamItems, load_more, fetch_older, and add_event, and
complete the partial docstring on subscribe to cover all parameters and
behavior; each docstring should briefly describe the method purpose, parameters
(including types like FriendFilter and since), return value or async behavior,
and notable side-effects (e.g., unsubscribe/subscribe, whether events are reset,
pagination behavior), matching the project's docstring style and placed
immediately above the corresponding function declarations.
- Around line 83-91: Replace the magic number 5000 in streamItems() with a named
constant to clarify intent and make it configurable: define a constant (e.g.
GROUPED_MODE_LIMIT or maxGroupedLimit) near the top of the file or in a relevant
constants/Config scope and use that constant instead of hardcoding 5000 when
setting filter.limit; keep the existing behavior where when let since is present
you set filter.since = since and filter.limit = GROUPED_MODE_LIMIT, and leave
the else branch using used_initial_page / initial_limit unchanged.
In `@damus/Features/NIP05/Views/NIP05DomainTimelineView.swift`:
- Around line 91-94: The view NIP05DomainTimelineView currently uses magic
numbers .padding(.top, 50) and .padding(.leading, 45); replace these with
computed values so layouts adapt to Dynamic Island/notch devices: wrap the
content in a GeometryReader (or use proxy in the parent) to read safe area
insets and compute topPadding = geometry.safeAreaInsets.top +
desiredExtraSpacing, then replace .padding(.top, 50) with .padding(.top,
topPadding); for the leading alignment replace the hard-coded 45 by either using
a named constant/backButtonWidth (match the actual back button frame) or an
alignmentGuide on the back button and title so the content aligns automatically
instead of .padding(.leading, 45). Ensure you update the references to
.padding(.top, 50) and .padding(.leading, 45) in NIP05DomainTimelineView to use
the computed values.
- Around line 172-175: handleOnAppear currently returns early when
model.events.all_events is non-empty, which prevents re-establishing the live
subscription after handleOnDisappear cancelled the streaming task; change the
logic so handleOnAppear calls model.subscribe(since: groupedSince) when the
streaming subscription is not active (e.g., check the model/owner's subscription
state or streamingTask is nil/cancelled) instead of basing it solely on
model.events.all_events, ensuring you reference and use the existing
model.subscribe(since:) and the cancellation in handleOnDisappear/streamingTask
to decide when to resubscribe.
In `@damus/Features/Profile/Views/CondensedProfilePicturesView.swift`:
- Around line 19-22: Add a /// docstring to the CondensedProfilePicturesView
initializer describing its purpose and each parameter: state (DamusState),
pubkeys ([Pubkey]), maxPictures (Int), and size (CGFloat, default 32.0); ensure
the comment explains what the initializer configures (e.g., creates a
CondensedProfilePicturesViewModel with given pubkeys and limits) and documents
the default value for size so it satisfies the project's docstring coverage
guideline.
In `@damus/Features/Timeline/Models/HomeModel.swift`:
- Around line 869-878: The loop does per-event awaits (await
self.favoriteEvents.insert(ev), await self.insert_home_event(ev)) and unsafely
mutates self.damus_state.events from outside an actor, causing many suspension
points and race conditions; fix by batching: collect allEvents into an array,
add a batch insertion API (e.g., insert_home_events(_: [Event]) and a bulk
insert on favoriteEvents like insertMany(_:)), call those once (await once)
instead of per-event awaits, and perform the UI/state mutation to
damus_state.events inside actor isolation (e.g., await MainActor.run {
self.damus_state.events.formUnion(allEvents) } or implement a MainActor-isolated
method) and only set backfilling = false after the batched awaits complete.
In `@damus/Features/Timeline/Views/PostingTimelineView.swift`:
- Around line 224-272: Add concise docstrings for the private helpers
groupedContentView, flushActiveSource(), backfillForGroupedMode(showLoading:),
and refreshFromNdb() describing their purpose and behavior: for
groupedContentView note it builds the ScrollView of grouped events, source
selection (timeline_source vs favorites), redaction/shimmer behavior and the
onAppear refresh logic for single-author vs full refresh; for
flushActiveSource() state that it forces GroupedModeQueueManager.flush for the
current source and note the `@MainActor` context; for
backfillForGroupedMode(showLoading:) explain it computes the since timestamp
from groupedFilterSettings and delegates to home.backfillForGroupedMode with the
source and showLoading flag; for refreshFromNdb() explain it computes the same
since and calls home.refreshGroupedFromNdb with the source — keep each docstring
short (1–2 lines) and placed immediately above each symbol.
In `@damus/Shared/Components/GroupedTimeline/GroupedAuthorRow.swift`:
- Around line 50-65: The computed view property postCountBadge lacks a
docstring; add a concise Swift documentation comment immediately above the var
postCountBadge describing its purpose (renders a capsule badge showing
postCount), any important behavior (uses different Font sizes when postCount >=
100, applies padding, colors from DamusColors, and the fixed
minWidth/minHeight), and note expected input/state (relies on postCount). Keep
it brief (one-two sentences) and use Swift doc comment format (///) so IDEs and
generated docs pick it up.
In `@damus/Shared/Components/GroupedTimeline/GroupedTimelineGrouper.swift`:
- Around line 76-78: The current logic using maxNotesPerUser on groupsByAuthor
filters out entire authors when they exceed the limit; instead, cap each
author's visible posts to maxNotesPerUser so prolific authors remain but with
their post list truncated. Locate the maxNotesPerUser check and replace the
filter on groupsByAuthor with logic that maps/truncates each group's array to at
most maxNotesPerUser items (use groupsByAuthor's values/arrays, e.g.
prefix/truncate), preserving the author keys while limiting their note arrays.
---
Nitpick comments:
In `@damus/Features/NIP05/Models/NIP05DomainEventsModel.swift`:
- Around line 44-62: Move the assignment of loading to true before creating the
asynchronous loadingTask in subscribe so the intent is explicit; specifically,
in NIP05DomainEventsModel.subscribe set loading = true (and any related state
like last_loaded_count if desired) prior to creating/loadingTask = Task { await
streamItems() }, so the visible state already reflects that loading will start
and avoids reliance on `@MainActor` scheduling semantics.
- Around line 79-81: The private helper function matches_domain(_ pubkey:
Pubkey) is unused and causes confusion; remove the entire method declaration
from NIP05DomainEventsModel (the unused matches_domain function) and ensure any
callers use NIP05DomainHelpers.matches_domain directly (note add_event
intentionally skips domain checking and streamItems already calls
NIP05DomainHelpers.matches_domain), then run tests/compile to confirm no
references remain.
- Around line 116-118: The loop in the initializer already calls
check_nip05_validity(pubkey:pubkey, damus_state:state) for each author (with
dedup via profiles.is_validated(pubkey)), so remove the redundant per-event
validation inside add_event (the call to check_nip05_validity at the noted
location) — either delete that call or replace it with a short comment noting
validation is pre-done; keep all other logic in add_event unchanged.
In `@damus/Features/NIP05/Views/NIP05DomainTimelineHeaderView.swift`:
- Around line 58-64: The computed property allAuthors iterates model.events,
allocates a Set, and deduplicates on every SwiftUI body evaluation which is
expensive; move this work out of the view by caching it and only recomputing
when events change: either add a cached property/method on the underlying model
(e.g., NIP05DomainTimelineModel.cachedAuthors or recomputeAuthors() that updates
when model.events mutates) or make the view hold `@State` private var allAuthors
and update that state in an onChange(or onReceive) handler watching model.events
or model.events.all_events; preserve the current deduplication logic (combining
eventAuthors + filter.authors and filtering by seen Set) when populating the
cache/state.
- Around line 103-128: Extract the shared formatting and name-resolution logic
from notesFromAuthorsString and friendsOfFriendsString into a single helper
(e.g., formatAuthorsString(authors: [Pubkey], ndb: Ndb, zeroCaseKey: String,
zeroCaseComment: String, locale: Locale)) that: 1) resolves up to three display
names using the zero-copy API ndb.lookup_profile(borrow:) and
Profile.displayName, truncates to 20 chars, 2) chooses the correct zero-case
localized string passed in (instead of hardcoding), and 3) runs the same
switch/format logic including localizedStringFormat for the default case; then
update notesFromAuthorsString and friendsOfFriendsString to call this helper
(passing their respective zero-case strings/comments) and remove the duplicated
switch and the use of ndb.lookup_profile_and_copy.
In `@damus/Features/Timeline/Models/HomeModel.swift`:
- Around line 894-913: The code spawns an unmanaged Task for the single-author
subscription; create a cancellable Task handle property on HomeModel (e.g.,
refreshSingleAuthorTask: Task<Void, Never>?), cancel and nil out any existing
handle before assigning the new Task, and inside the Task ensure you clear the
handle when you return on .eose or on cancellation; reference the existing
subscription creation using
damus_state.nostrNetwork.reader.advancedStream(filters: [filter], streamMode:
.ndbAndNetworkParallel(...)) and the surrounding logic in
refreshSingleAuthorEvents (or the method containing that Task), use weak self to
avoid retain cycles and check Task.isCancelled if needed.
In `@damus/Shared/Components/GroupedTimeline/GroupedAuthorRow.swift`:
- Line 68: The method eventPreviewText(_:) is declared without an access
modifier and should be made private since it's only used within the view's body;
update the declaration of eventPreviewText(_ event: NostrEvent) to be private
(e.g., private func eventPreviewText(_ event: NostrEvent) -> String) so the
helper is not exposed beyond GroupedAuthorRow and locate it near the body
implementation to keep related code together.
In `@damusTests/GroupedModeQueueManagerTests.swift`:
- Around line 61-77: Add a new unit test that calls
GroupedModeQueueManager.flush(source:) on a fresh EventHolder (without calling
set_should_queue or inserting events) to ensure the flush is a no-op; create a
test method (e.g., testFlushOnDefaultHolderIsNoOp) that constructs EventHolder,
invokes GroupedModeQueueManager.flush(source: holder), then asserts
holder.events.count == 0 and holder.should_queue == false to cover the
default-holder code path in EventHolder.flush().
- Around line 16-26: The helper function makeEvent lacks a documentation
comment; add a concise docstring above makeEvent describing its purpose,
parameters (content: String default, keypair: Keypair default test_keypair,
secondsAgo: UInt32 default 100) and the return value (NostrEvent), and note that
the createdAt uses current time minus secondsAgo; update the docstring format to
match project style (brief summary line, parameter descriptions, and return
description) for the makeEvent helper.
Comment on lines
+135
to
+136
| let words = content.split(whereSeparator: { $0.isWhitespace }).filter { !$0.isEmpty } | ||
| if words.count == 1 && content.count < 20 { return true } |
Contributor
There was a problem hiding this comment.
content.count < 20 threshold is inconsistent with the test expectation for single-word notes.
isTooShort returns false for any single-word note whose character count is ≥ 20. "Supercalifragilisticexpialidocious" (34 chars) will therefore pass the filter even with hideShortNotes: true. See testSingleWordExcludedWhenHideShortEnabled in GroupedTimelineGrouperTests.swift — that test will fail against the current threshold.
The original behavior (reject every single-word note when hideShortNotes is set) aligns with both the test's assertion and typical user expectations.
🛡️ Proposed fix
- if words.count == 1 && content.count < 20 { return true }
+ if words.count == 1 { return true }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damus/Shared/Components/GroupedTimeline/GroupedTimelineGrouper.swift` around
lines 135 - 136, The single-word check in GroupedTimelineGrouper.isTooShort
currently uses "if words.count == 1 && content.count < 20 { return true }",
which lets very long single words pass; change it so any single-word note is
considered too short when hideShortNotes is enabled. Locate the isTooShort (or
equivalent) method in GroupedTimelineGrouper and replace the conditional with an
unconditional single-word check (e.g., if words.count == 1 { return true }) so
single-word content is rejected regardless of character count.
Comment on lines
+11
to
+12
| @MainActor | ||
| final class GroupedModeQueueManagerTests: XCTestCase { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find EventHolder and GroupedModeQueueManager definitions
echo "=== Searching for EventHolder definition ==="
rg -n "class EventHolder|struct EventHolder" --type swift -A3
echo ""
echo "=== Searching for GroupedModeQueueManager definition ==="
rg -n "class GroupedModeQueueManager|struct GroupedModeQueueManager" --type swift -A3
echo ""
echo "=== Searching for `@MainActor` on EventHolder ==="
rg -B2 "class EventHolder|struct EventHolder" --type swift | head -20
echo ""
echo "=== Searching for `@MainActor` on GroupedModeQueueManager ==="
rg -B2 "class GroupedModeQueueManager|struct GroupedModeQueueManager" --type swift | head -20Repository: damus-io/damus
Length of output: 2184
🏁 Script executed:
#!/bin/bash
# Get the full test file to review structure
echo "=== Full content of GroupedModeQueueManagerTests.swift ==="
cat -n damusTests/GroupedModeQueueManagerTests.swiftRepository: damus-io/damus
Length of output: 2986
🏁 Script executed:
#!/bin/bash
# Search for ObservableObject or other concurrency markers on these types
echo "=== Checking for ObservableObject or other markers ==="
rg -A10 "class EventHolder|struct EventHolder" --type swift
echo ""
rg -A10 "class GroupedModeQueueManager|struct GroupedModeQueueManager" --type swiftRepository: damus-io/damus
Length of output: 2600
Add docstrings to makeEvent() and all test methods.
The test helper makeEvent(content:keypair:secondsAgo:) and the three test methods lack docstrings as required by the coding guidelines for all code added or modified.
The @MainActor annotation on the test class is correct and necessary: GroupedModeQueueManager.flush(source:) is explicitly marked @MainActor, so the test class must run on the main thread to satisfy this constraint. No thread-safety concern exists here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damusTests/GroupedModeQueueManagerTests.swift` around lines 11 - 12, Add
missing Swift docstrings for the test helper and tests: add a /// doc comment
above makeEvent(content:keypair:secondsAgo:) describing its purpose and
documenting parameters and return value, and add /// doc comments to each test
method inside GroupedModeQueueManagerTests (the three test methods present)
briefly stating the scenario being tested and expected outcome; keep wording
concise and follow the project's docstring style (short summary line, `@param` for
parameters if used, and `@return` if applicable).
Comment on lines
+232
to
+243
| func testSingleWordExcludedWhenHideShortEnabled() { | ||
| let singleWordEvent = makeEvent(content: "Supercalifragilisticexpialidocious", keypair: test_keypair) | ||
|
|
||
| let groups = GroupedTimelineGrouper.group( | ||
| events: [singleWordEvent], | ||
| filter: { _ in true }, | ||
| values: defaultValues(hideShortNotes: true), | ||
| now: now | ||
| ) | ||
|
|
||
| XCTAssertTrue(groups.isEmpty, "Single-word note should be excluded") | ||
| } |
Contributor
There was a problem hiding this comment.
testSingleWordExcludedWhenHideShortEnabled will fail against the current implementation.
"Supercalifragilisticexpialidocious" is 34 characters. isTooShort applies words.count == 1 && content.count < 20; since 34 ≥ 20, it returns false and the event is included in groups. XCTAssertTrue(groups.isEmpty) will therefore fail.
The fix belongs in GroupedTimelineGrouper.isTooShort — remove the content.count < 20 guard for the single-word case (see comment on GroupedTimelineGrouper.swift line 135–136). Alternatively, use content shorter than 20 characters in this test if the intent is to only exclude genuinely short single-word notes:
- let singleWordEvent = makeEvent(content: "Supercalifragilisticexpialidocious", keypair: test_keypair)
-
- ...
- XCTAssertTrue(groups.isEmpty, "Single-word note should be excluded")
+ let singleWordEvent = makeEvent(content: "gm", keypair: test_keypair)
+
+ ...
+ XCTAssertTrue(groups.isEmpty, "Single-word short note should be excluded")(Prefer fixing the implementation rather than weakening the test so that long single-word notes are also excluded.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@damusTests/GroupedTimelineGrouperTests.swift` around lines 232 - 243, The
test testSingleWordExcludedWhenHideShortEnabled fails because
GroupedTimelineGrouper.isTooShort currently checks both words.count == 1 and
content.count < 20, allowing long single-word notes (like
"Supercalifragilisticexpialidocious") to slip through; change isTooShort to
treat any single-word note as too short when hideShortNotes is enabled by
removing the content.count < 20 guard (i.e., return true when words.count == 1),
so GroupedTimelineGrouper.group will exclude single-word events as the test
expects.
danieldaquino
requested changes
Apr 1, 2026
Collaborator
danieldaquino
left a comment
danieldaquino
left a comment
There was a problem hiding this comment.
Thank you @alltheseas! Given this is a new feature and the size of the code changes, can we please place this behind a Damus Purple feature flag and minimize changes to existing code as much as possible (to reduce regression risk and make review faster)?
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
2 participants
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 a fevela-style grouped view mode to NIP-05 domain feeds, allowing users to see posts organized by author rather than chronologically.
Depends on: #3376 (Fix nip05 discovery and double filtering)
Closes: #3379
Features
Commits
Add configurable size parameter to CondensedProfilePicturesView
Add NIP-05 filter settings UI with grouped mode support
Add grouped list view for NIP-05 domain feeds
Improve NIP-05 header layout with stacked avatars
Integrate grouped mode into NIP-05 domain timeline
Standard PR Checklist
Closes:orFixes:tags in the commit messages wherever applicableTest report
@damus.io)Results:
Screenshots
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests