Skip to content

Feat/nostr emoji tags - #144

Merged
uakihir0 merged 3 commits into
mainfrom
feat/nostr-emoji-tags
Jul 27, 2026
Merged

Feat/nostr emoji tags#144
uakihir0 merged 3 commits into
mainfrom
feat/nostr-emoji-tags

Conversation

@uakihir0

Copy link
Copy Markdown
Owner

No description provided.

uakihir0 added 2 commits July 26, 2026 18:54
Parse NIP-30 emoji tags from Nostr events and attach Emoji models
to AttributedString via addEmojiElement(). Add extractEmojis()
internal helper and NOSTR_EMOJI_SHORTCODE regex for validation.
Add 7 test cases covering emoji tag parsing, duplicate shortcodes,
malformed tags, and interaction with hashtag/quote references.
Modify note() helper to accept optional tags parameter.
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@uakihir0, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 6 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0a8937d9-8148-4dc6-8ada-b52966254d83

📥 Commits

Reviewing files that changed from the base of the PR and between f01bb84 and 0ff7891.

📒 Files selected for processing (3)
  • nostr/src/commonMain/kotlin/work/socialhub/planetlink/nostr/action/NostrCommentUpdateStream.kt
  • nostr/src/commonMain/kotlin/work/socialhub/planetlink/nostr/action/NostrMapper.kt
  • nostr/src/commonTest/kotlin/work/socialhub/planetlink/nostr/action/NostrMapperTest.kt
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/nostr-emoji-tags

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@uakihir0 uakihir0 assigned uakihir0 and unassigned uakihir0 Jul 27, 2026
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add NIP-30 emoji tag parsing to Nostr note mapping

✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Parse NIP-30 emoji tags and attach Emoji metadata to mapped comment text.
• Validate shortcodes and ignore malformed/duplicate emoji tags to avoid bad rendering.
• Add unit tests covering emoji parsing, edge cases, and coexistence with hashtags/quotes.
Diagram

graph TD
  A(["NostrMapper.comment()"]) --> B(["extractEmojis()"]) --> C(("Emoji list")) --> D(["AttributedString.addEmojiElement()"]) --> E(("Comment.text elements"))
  F(("NostrEvent.tags")) --> B
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Adopt a dedicated NIP-30 tag parser (library/shared module)
  • ➕ Keeps NIP parsing logic centralized and reusable across features
  • ➕ Easier to evolve if NIP-30 handling grows (e.g., url validation, media fetching rules)
  • ➖ May be overkill for a single tag type
  • ➖ Adds dependency/abstraction overhead if no existing parser exists
2. Build a shortcode→url map and resolve during element parsing
  • ➕ Single pass over text to resolve only referenced shortcodes
  • ➕ Potentially avoids constructing Emoji objects for unused tags
  • ➖ More invasive change to AttributedString parsing pipeline
  • ➖ Harder to keep consistent with existing addEmojiElement() behavior

Recommendation: Current approach is appropriate: extractEmojis() is small, validates inputs (shortcode format, blank URL, duplicates), and integrates via the existing addEmojiElement() hook without disrupting other attributed-element logic (hashtags/quotes). Consider a shared NIP-30 parser only if additional NIP-30 features are planned.

Files changed (2) +180 / -2

Enhancement (1) +27 / -1
NostrMapper.ktParse NIP-30 emoji tags and attach EMOJI attributed elements +27/-1

Parse NIP-30 emoji tags and attach EMOJI attributed elements

• Adds validation for Nostr emoji shortcodes and introduces extractEmojis() to parse 'emoji' tags from events. The mapped AttributedString now receives emoji metadata via addEmojiElement(), ignoring malformed tags and duplicate shortcodes.

nostr/src/commonMain/kotlin/work/socialhub/planetlink/nostr/action/NostrMapper.kt

Tests (1) +153 / -1
NostrMapperTest.ktAdd test coverage for emoji tag mapping and edge cases +153/-1

Add test coverage for emoji tag mapping and edge cases

• Introduces 7 tests validating emoji tag parsing, repeated occurrences, multiple emoji types, malformed/empty tags, duplicate shortcode handling, and interaction with hashtags and resolved quotes. Updates the note() test helper to accept tags for constructing events.

nostr/src/commonTest/kotlin/work/socialhub/planetlink/nostr/action/NostrMapperTest.kt

@qodo-code-review

qodo-code-review Bot commented Jul 27, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 21 rules

Grey Divider


Action required

1. Emoji dropped on updates ✓ Resolved 🐞 Bug ≡ Correctness
Description
When a quote is resolved later, NostrCommentUpdateStream.applyNote replaces the parent comment’s
attributed text with AttributedString.plain(...), which discards the EMOJI elements added by
NostrMapper.attributedText. This causes emojis (and other attributes) to disappear after deferred
quote resolution.
Code

nostr/src/commonMain/kotlin/work/socialhub/planetlink/nostr/action/NostrMapper.kt[R207-209]

+        return AttributedString(validated).also {
+            it.addEmojiElement(extractEmojis(note))
+        }
Relevance

⭐⭐⭐ High

Accepted precedent: deferred quote resolution must re-run mapping/update text; otherwise mapped
attributes become stale/lost.

PR-#142

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR adds emoji attribution during mapping, but the deferred-quote update path overwrites the text
with a newly scanned AttributedString that never calls addEmojiElement, so EMOJI elements are lost
after the update.

nostr/src/commonMain/kotlin/work/socialhub/planetlink/nostr/action/NostrMapper.kt[188-210]
nostr/src/commonMain/kotlin/work/socialhub/planetlink/nostr/action/NostrCommentUpdateStream.kt[219-230]
core/src/commonMain/kotlin/work/socialhub/planetlink/model/common/AttributedString.kt[52-73]
core/src/commonMain/kotlin/work/socialhub/planetlink/model/common/AttributedString.kt[151-207]
PR-#142

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`NostrMapper.attributedText()` now adds emoji attributes via `addEmojiElement(extractEmojis(note))`, but when a quoted note arrives later, `NostrCommentUpdateStream.applyNote()` rebuilds the parent comment text using `AttributedString.plain(...)`. That rebuild does not re-add emoji elements, so the newly introduced emoji rendering can disappear after deferred quote resolution.

### Issue Context
This happens specifically in the `quotedEventId == note.event.id` branch (quote resolution path), where the code strips the `nostr:` reference from the *existing* text and then overwrites `text`.

### Fix Focus Areas
- nostr/src/commonMain/kotlin/work/socialhub/planetlink/nostr/action/NostrCommentUpdateStream.kt[219-230]
- nostr/src/commonMain/kotlin/work/socialhub/planetlink/nostr/action/NostrMapper.kt[188-210]
- core/src/commonMain/kotlin/work/socialhub/planetlink/model/common/AttributedString.kt[52-73]

### Suggested fix
When stripping the quote reference in `applyNote`, preserve/reapply the existing attributes:
- Capture the existing emoji definitions from the current `text` (e.g., existing `EMOJI` elements’ `displayText`/`expandedText`), rebuild the stripped `AttributedString`, and call `addEmojiElement(...)` again.
- (Optional but safer) similarly preserve the current validated hashtags set and downgrade any newly-scanned hashtags not present before.

Add a unit test covering: comment initially mapped with unresolved quote + emoji shortcode, then `applyNote(quotedNote)` is applied, and emoji elements remain present.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Unbounded emoji shortcode work ✓ Resolved 🐞 Bug ☼ Reliability
Description
extractEmojis accepts any-length alphanumeric/underscore shortcode from untrusted Nostr tags, and
each accepted shortcode is interpolated into a per-emoji regex during addEmojiElement scanning.
Extremely long shortcodes or many emoji tags can cause unbounded CPU/allocation work during mapping.
Code

nostr/src/commonMain/kotlin/work/socialhub/planetlink/nostr/action/NostrMapper.kt[R212-223]

+    internal fun extractEmojis(note: NostrNote): List<Emoji> {
+        val shortCodes = mutableSetOf<String>()
+        return note.event.tags.mapNotNull { tag ->
+            if (tag.size < 3 || tag[0] != "emoji") return@mapNotNull null
+
+            val shortCode = tag[1]
+            val imageUrl = tag[2]
+            if (!NOSTR_EMOJI_SHORTCODE.matches(shortCode) ||
+                imageUrl.isBlank() ||
+                !shortCodes.add(shortCode)
+            ) {
+                return@mapNotNull null
Relevance

⭐⭐ Medium

Team often accepts untrusted-input hardening, but no close precedent for emoji-regex length caps
specifically.

PR-#53
PR-#143

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new code accepts attacker-controlled shortcodes with no size bound, and the downstream emoji
scanning compiles a regex from each shortcode, making mapping cost depend directly on untrusted
input size/count.

nostr/src/commonMain/kotlin/work/socialhub/planetlink/nostr/action/NostrMapper.kt[30-34]
nostr/src/commonMain/kotlin/work/socialhub/planetlink/nostr/action/NostrMapper.kt[212-230]
core/src/commonMain/kotlin/work/socialhub/planetlink/model/common/AttributedString.kt[151-207]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`extractEmojis()` validates shortcode characters but not length or count. Since each accepted shortcode is later converted into a regex pattern in `AttributedString.scanEmojis`, hostile inputs can increase mapping work via very long shortcodes and/or many emoji tags.

### Issue Context
- `NOSTR_EMOJI_SHORTCODE = Regex("[A-Za-z0-9_]+")` allows arbitrary length.
- `AttributedString.scanEmojis` compiles `":${emoji.shortCode}:".toRegex()` per emoji and scans recursively.

### Fix Focus Areas
- nostr/src/commonMain/kotlin/work/socialhub/planetlink/nostr/action/NostrMapper.kt[33-34]
- nostr/src/commonMain/kotlin/work/socialhub/planetlink/nostr/action/NostrMapper.kt[212-230]
- core/src/commonMain/kotlin/work/socialhub/planetlink/model/common/AttributedString.kt[151-207]

### Suggested fix
Add defensive bounds in `extractEmojis`, e.g.:
- Reject shortcodes outside a reasonable length range (for example 1..64).
- Optionally cap the number of emoji tags processed per note.

(Optionally, longer-term) avoid regex compilation for emoji matching (use `indexOf`/string search) or ensure the pattern uses `Regex.escape(...)` if you ever broaden allowed shortcode characters.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

- Replace AttributedString.plain() with stripQuotePreservingAttributes()
  to preserve EMOJI, HASH_TAG, and other attributed elements when
  stripping the quote reference from parent text on deferred resolution
- Add length bounds (1-64) and count cap (64) to extractEmojis for
  defense against untrusted tag input
- Add stripQuotePreservingAttributes helper to NostrMapper for
  testability
@uakihir0

uakihir0 commented Jul 27, 2026

Copy link
Copy Markdown
Owner Author

🤖 PR Review Auto-Check

Last checked: 2026-07-27 05:29 UTC

No new review comments since the last check.

What was done

Summary

Fixed

Issue 1 — Emoji dropped on updates (NostrCommentUpdateStream.applyNote):

  • Root cause: When resolving a deferred quote, applyNote rebuilt the parent comment's text using AttributedString.plain(strippedContent), which discarded all attributed elements (EMOJI, HASH_TAG, LINK, etc.).
  • Fix: Added NostrMapper.stripQuotePreservingAttributes() which iterates the existing element list and only modifies PLAIN elements (stripping the nostr: reference). All other elements (EMOJI, HASH_TAG, etc.) are preserved unchanged.
  • NostrCommentUpdateStream.applyNote now calls this helper instead of creating a new AttributedString.plain(...).

Issue 2 — Unbounded emoji shortcode work (NostrMapper.extractEmojis):

  • Root cause: NOSTR_EMOJI_SHORTCODE regex [A-Za-z0-9_]+ allowed arbitrary-length shortcodes, which would be compiled into per-emoji regex patterns downstream.
  • Fix: Changed regex to {1,64} and added a cap of 64 emoji tags processed per note.

Tests added

  • preservesEmojiAfterStripQuotePreservingAttributes — verifies emoji elements survive quote stripping
  • preservesEmojiAndHashtagAfterStripQuotePreservingAttributes — verifies both emoji and hashtag attributes survive
  • limitsEmojiTagCount — verifies at most 64 emoji tags are processed

Skipped

  • Hashtag re-validation after quote resolution (optional per review, would require persisting the validated hashtag set)
  • Regex.escape() in scanEmojis (review explicitly said "optionally, longer-term")

@uakihir0
uakihir0 merged commit 98a767d into main Jul 27, 2026
4 checks passed
@uakihir0
uakihir0 deleted the feat/nostr-emoji-tags branch July 27, 2026 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant