-
Notifications
You must be signed in to change notification settings - Fork 0
feat(nip-25): add reaction schemas #41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds JSON schema definitions for NIP-25 reaction events, supporting both reactions to nostr events (kind 7) and reactions to external content (kind 17). It also introduces support for custom emoji tags and external content tags from NIP-73.
- Add kind 7 and kind 17 reaction event schemas with proper validation
- Implement custom emoji tag schema for reactions with shortcode and URL validation
- Add external content identifier (i) and type (k) tag schemas from NIP-73
- Create reusable alias files for cross-schema tag references
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| nips/nip-78/kind-30078/schema.yaml | Defines schema for arbitrary custom app data events |
| nips/nip-73/tag/k/schema.yaml | Validates external content type identifier tags |
| nips/nip-73/tag/i/schema.yaml | Validates external content identifier tags |
| nips/nip-25/tag/emoji/schema.yaml | Validates custom emoji tags for reactions |
| nips/nip-25/kind-7/schema.yaml | Validates reactions to nostr events with emoji support |
| nips/nip-25/kind-17/schema.yaml | Validates reactions to external content |
| @/tag/*.yaml | Creates reusable aliases for tag schemas |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| allOf: | ||
| - if: | ||
| contains: | ||
| type: array | ||
| minItems: 1 | ||
| items: | ||
| - const: "emoji" | ||
| then: | ||
| contains: | ||
| $ref: "@/tag/emoji.yaml" |
Copilot
AI
Sep 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This validation logic is redundant. The emoji tag validation is already handled in the items section (lines 17-25). This duplication could cause confusion and maintenance issues.
| allOf: | |
| - if: | |
| contains: | |
| type: array | |
| minItems: 1 | |
| items: | |
| - const: "emoji" | |
| then: | |
| contains: | |
| $ref: "@/tag/emoji.yaml" |
nips/nip-25/kind-7/schema.yaml
Outdated
| contains: | ||
| type: array | ||
| minItems: 2 | ||
| maxItems: 4 | ||
| items: | ||
| - const: "e" | ||
| - type: string | ||
| pattern: '^[a-f0-9]{64}$' | ||
| - type: string | ||
| pattern: '^(ws://|wss://).+$' | ||
| - allOf: | ||
| - $ref: "@/secp256k1.yaml" | ||
| additionalItems: false |
Copilot
AI
Sep 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] This inline e-tag validation should be extracted to a separate schema file for reusability, following the pattern established for other tag types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -0,0 +1,2 @@ | |||
| allOf: | |||
Copilot
AI
Sep 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the trailing space after the colon on line 1.
| allOf: | |
| allOf: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| errorMessage: | ||
| contains: "tags must include at least one k tag describing the external content type" | ||
| - contains: | ||
| $ref: "@/tag/external-content-i.yaml" | ||
| errorMessage: | ||
| contains: "tags must include at least one i tag referencing the external content" |
Copilot
AI
Oct 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error messages are nested under individual 'contains' constraints, but this structure may not work as expected in JSON Schema. Consider moving the errorMessage to the top level of the allOf block for proper error handling.
| errorMessage: | |
| contains: "tags must include at least one k tag describing the external content type" | |
| - contains: | |
| $ref: "@/tag/external-content-i.yaml" | |
| errorMessage: | |
| contains: "tags must include at least one i tag referencing the external content" | |
| - contains: | |
| $ref: "@/tag/external-content-i.yaml" | |
| errorMessage: | |
| allOf: | |
| - "tags must include at least one k tag describing the external content type" | |
| - "tags must include at least one i tag referencing the external content" |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
7bbbc48 to
02e182e
Compare
Summary
Testing