feat: display direct replies below notes - #59
Open
alltheseas wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughAdds HTML note-preview rendering with mention/hashtag resolution and bounded truncation; expands background fetch to collect and fetch missing parent and reply-content unknowns, waits for profile readiness and reply indexing before proceeding. Changes
Sequence Diagram(s)sequenceDiagram
participant Main as fetch_note_secondary_data()
participant Render as render module
participant RelayPool as RelayPool
participant Ndb as Ndb
participant Network as Relays
Main->>Render: collect_parent_unknowns(ndb, note_rd)
Render->>Ndb: load parent note & collect unknowns
Render-->>Main: UnknownIds (parent)
Main->>RelayPool: fetch_unknowns(relay_pool, ndb, unknowns)
RelayPool->>Ndb: subscribe to filters (needed_profiles)
RelayPool->>Network: request events
Network-->>RelayPool: events
RelayPool->>Ndb: ingest events
loop Wait for Profiles
RelayPool->>Ndb: profiles_ready(needed_profiles)?
alt ready
Ndb-->>RelayPool: yes
RelayPool-->>Main: done
else not ready
RelayPool->>Ndb: poll subscription stream
end
end
Main->>Render: collect_reply_content_unknowns(ndb, note_rd)
Render->>Ndb: query replies & collect unknowns
Render-->>Main: UnknownIds (replies)
Main->>RelayPool: fetch_unknowns(relay_pool, ndb, unknowns)
RelayPool->>Ndb: subscribe to reply filters
RelayPool->>Network: request replies
Network-->>RelayPool: reply events
RelayPool->>Ndb: ingest replies
loop Wait for Indexing
RelayPool->>Ndb: check indexed batches vs ingested reply count
alt indexed
Ndb-->>RelayPool: ready
RelayPool-->>Main: done
else wait
RelayPool->>Ndb: poll subscription stream
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
alltheseas
force-pushed
the
fix/nprofile-mention-rendering
branch
2 times, most recently
from
March 26, 2026 02:26
6a02a72 to
eb13ad4
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/render.rs (1)
733-748:⚠️ Potential issue | 🟠 MajorWait for fetched notes too, not just fetched profiles.
ingestedis counted here, but the readiness gate only checksprofiles_ready. If a batch contains both a missing parent/quote note and profile events, the first profile notification can satisfy this loop while the note is still not queryable, so the next render pass still misses that preview on a cold cache.As per coding guidelines,
src/render.rs:RenderData::completepairs nostrdb subscriptions with network fetches—keep it resilient to partially missing data.Also applies to: 752-759
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/render.rs` around lines 733 - 748, The loop increments a single ingested counter when events are processed (via ndb.process_event_with) but the readiness gate only checks profiles_ready, so fetched notes (parents/quotes) can be missed; modify the logic in RenderData::complete to track note-specific ingestion (e.g., maintain a notes_ingested counter or a notes_ready flag when processing events from nostr_filters/relay_pool.stream_events) and ensure the readiness condition requires both profiles_ready and notes_ready (or notes_ingested > 0) before marking completion; update the event processing block around nostr_filters, IngestMetadata creation, and ndb.process_event_with to set that notes flag/counter when the event type is a note/parent/quote so the render pass waits for fetched notes as well as profiles.src/html.rs (1)
1301-1306:⚠️ Potential issue | 🟠 MajorApply the reply cap after the
is_directcheck.This query is limited to
DIRECT_REPLY_LIMITbefore non-direct matches are discarded, so deep thread replies can consume the first 50 rows and hide real direct replies. The same pre-filtered cap is mirrored incollect_reply_content_unknowns, so mention-prefetch will miss those replies too unless both call sites overfetch or scan until they accumulateDIRECT_REPLY_LIMITdirect replies.Also applies to: 1326-1335
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/html.rs` around lines 1301 - 1306, The query currently applies DIRECT_REPLY_LIMIT before filtering by is_direct, letting deep-thread non-direct replies crowd out true direct replies; update the logic in the blocks using Filter::new() + .kinds([1]).event(note.id()).build() and the corresponding app.ndb.query call (and the similar call in collect_reply_content_unknowns) to overfetch (e.g., request more than DIRECT_REPLY_LIMIT) or loop/scan results until you've accumulated DIRECT_REPLY_LIMIT items that pass is_direct, i.e., remove the pre-filter cap as the final limiting step and only enforce DIRECT_REPLY_LIMIT after applying the is_direct check so you always return up to DIRECT_REPLY_LIMIT direct replies for mention-prefetch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/render.rs`:
- Around line 700-707: The current logic builds relay_targets from
unknowns.relay_hints() and drops relay_pool.default_relays() whenever
relay_hints is non-empty, which causes hintless profile fetches (needed_profiles
from UnknownIds) to miss default relays; change the construction of
relay_targets so that if relay_hints is non-empty you merge
relay_pool.default_relays() with relay_hints (deduplicated) rather than
replacing them — update the code around relay_hints, needed_profiles, and
relay_targets to collect both sets (e.g., extend or union the iterators) so
kind-0/profile requests still go to default_relays() as well as any hinted
relays.
- Around line 782-795: In collect_parent_unknowns, after calling
unknowns.collect_from_blocks(ndb, &txn, &parent_note) also ensure the parent's
author profile is queued for the second-pass by adding a call to enqueue the
parent pubkey into UnknownIds (e.g. unknowns.collect_profile(ndb, &txn,
&parent_note.pubkey()) or the project-equivalent method that adds a
pubkey/profile to UnknownIds); this will ensure the parent author's profile
(used by the thread header) is fetched when the second pass runs.
---
Outside diff comments:
In `@src/html.rs`:
- Around line 1301-1306: The query currently applies DIRECT_REPLY_LIMIT before
filtering by is_direct, letting deep-thread non-direct replies crowd out true
direct replies; update the logic in the blocks using Filter::new() +
.kinds([1]).event(note.id()).build() and the corresponding app.ndb.query call
(and the similar call in collect_reply_content_unknowns) to overfetch (e.g.,
request more than DIRECT_REPLY_LIMIT) or loop/scan results until you've
accumulated DIRECT_REPLY_LIMIT items that pass is_direct, i.e., remove the
pre-filter cap as the final limiting step and only enforce DIRECT_REPLY_LIMIT
after applying the is_direct check so you always return up to DIRECT_REPLY_LIMIT
direct replies for mention-prefetch.
In `@src/render.rs`:
- Around line 733-748: The loop increments a single ingested counter when events
are processed (via ndb.process_event_with) but the readiness gate only checks
profiles_ready, so fetched notes (parents/quotes) can be missed; modify the
logic in RenderData::complete to track note-specific ingestion (e.g., maintain a
notes_ingested counter or a notes_ready flag when processing events from
nostr_filters/relay_pool.stream_events) and ensure the readiness condition
requires both profiles_ready and notes_ready (or notes_ingested > 0) before
marking completion; update the event processing block around nostr_filters,
IngestMetadata creation, and ndb.process_event_with to set that notes
flag/counter when the event type is a note/parent/quote so the render pass waits
for fetched notes as well as profiles.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a9d50dff-e53c-4b35-b926-492c544bcce3
📒 Files selected for processing (4)
src/html.rssrc/main.rssrc/render.rssrc/unknowns.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main.rs
alltheseas
force-pushed
the
fix/nprofile-mention-rendering
branch
from
March 26, 2026 02:38
eb13ad4 to
c27fb43
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/render.rs (1)
939-953: Consider reducingnotes_per_awaitbatch size for reply waiting.With
notes_per_await(10)at line 895, the stream waits for up to 10 notes before yielding. When fewer replies are ingested (common case), this may cause unnecessary waiting until internal timeouts. A smaller batch size like 2-4 would yield faster for typical reply counts while still batching effectively.Suggested change
let sub_id = ndb.subscribe(&[reply_filter])?; - let stream = sub_id.stream(ndb).notes_per_await(10); + let stream = sub_id.stream(ndb).notes_per_await(4);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/render.rs` around lines 939 - 953, Reduce the batching size for reply_stream to avoid waiting for large batches when few replies arrive: locate where notes_per_await is set (notes_per_await currently 10) and change it to a smaller value (2–4), keeping reply_stream, reply_ingested, and the existing timeout loop unchanged so the waiting loop (which uses timeout(wait_for, reply_stream.next())) yields more promptly for typical reply counts; adjust and run tests/local runs to confirm fewer unnecessary timeouts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/render.rs`:
- Around line 939-953: Reduce the batching size for reply_stream to avoid
waiting for large batches when few replies arrive: locate where notes_per_await
is set (notes_per_await currently 10) and change it to a smaller value (2–4),
keeping reply_stream, reply_ingested, and the existing timeout loop unchanged so
the waiting loop (which uses timeout(wait_for, reply_stream.next())) yields more
promptly for typical reply counts; adjust and run tests/local runs to confirm
fewer unnecessary timeouts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: af82b765-97ff-4a4e-8018-eaf26c3a5f63
📒 Files selected for processing (4)
src/html.rssrc/main.rssrc/render.rssrc/unknowns.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main.rs
- src/unknowns.rs
Closes damus-io#58 Signed-off-by: alltheseas Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
alltheseas
force-pushed
the
fix/nprofile-mention-rendering
branch
from
March 26, 2026 03:46
c27fb43 to
ade0fde
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/html.rs`:
- Line 1194: The profile page is rendering recent notes via
build_note_content_html and calling render_note_preview_html without performing
the name-prefetch/readiness that the note route does, so parent/reply previews
may show bech32 instead of resolved display names; update serve_profile_html
(the handler that builds recent-note HTML) to mirror the note-route prefetch
step: after fetching the recent notes list, collect all author/prefetch keys
from those notes (including parents/replies), perform the same batched ndb
fetch/readiness call used by the note route, await its completion, then call
build_note_content_html/render_note_preview_html so names are resolved; ensure
you reference the same readiness/fetch helper used by the note route to keep
behavior consistent and batch across the recent-note query to avoid N+1 fetches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e4a944f1-348c-4378-8dae-03b4da18a946
📒 Files selected for processing (4)
src/html.rssrc/main.rssrc/render.rssrc/unknowns.rs
✅ Files skipped from review due to trivial changes (1)
- src/unknowns.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main.rs
- src/render.rs
| )) | ||
| .into_owned(), | ||
| content_html: format!("{}{}", html_escape::encode_text(content), ellipsis), | ||
| content_html: render_note_preview_html(&parent_note, ndb, txn, 200), |
There was a problem hiding this comment.
These preview call sites still miss the prefetch contract on profile pages.
Line 1194 and Line 1352 now depend on render_note_preview_html, which can only resolve names that are already in NDB. The note route satisfies that in src/main.rs (Line 412-Line 455), but serve_profile_html renders recent notes through build_note_content_html in this file (Line 2169-Line 2176) without an equivalent fetch stage. Recent notes on profile pages can therefore still fall back to abbreviated bech32 in parent/reply previews instead of resolved display names. Please mirror that prefetch/readiness step there, ideally batched across the recent-note query.
Also applies to: 1352-1352
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/html.rs` at line 1194, The profile page is rendering recent notes via
build_note_content_html and calling render_note_preview_html without performing
the name-prefetch/readiness that the note route does, so parent/reply previews
may show bech32 instead of resolved display names; update serve_profile_html
(the handler that builds recent-note HTML) to mirror the note-route prefetch
step: after fetching the recent notes list, collect all author/prefetch keys
from those notes (including parents/replies), perform the same batched ndb
fetch/readiness call used by the note route, await its completion, then call
build_note_content_html/render_note_preview_html so names are resolved; ensure
you reference the same readiness/fetch helper used by the note route to keep
behavior consistent and batch across the recent-note query to avoid N+1 fetches.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@nprofile/@npubmentions to display names in reply and parent previewsnevent1(with relay hints) instead ofnote1for parent note linksTest plan
@nprofilementions — verify display names render, not raw bech32nevent1with relay hint, notnote1cargo clippy -- -D warningspassescargo testpassesCloses #58
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements