test: kind-1 schema validation for Damus events#3714
Closed
alltheseas wants to merge 1 commit into
Closed
Conversation
Add a standalone Swift package in schema-validation/ that validates kind-1 Nostr events against nostrability JSON schema definitions. Uses schemata-validator-swift as a test dependency. Tests: valid event, wrong kind routing, missing fields, real Damus event. Includes GitHub Actions workflow for CI on push/PR to master. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: alltheseas <alltheseas@users.noreply.github.com>
Contributor
📝 WalkthroughWalkthroughA new schema validation module is introduced with a Swift package configuration, GitHub Actions workflow, and comprehensive test suite to validate Kind-1 events against the Damus schema using the SchemataValidator framework. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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
marked this pull request as draft
March 23, 2026 18:53
![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: 2
🧹 Nitpick comments (3)
schema-validation/Tests/Kind1ValidationTests/Kind1ValidationTests.swift (1)
4-78: Consider adding docstrings to test methods.The test methods use
// MARK:comments for organization, but per coding guidelines, docstrings should be added for code changes. Brief docstrings describing what each test validates would improve maintainability.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 `@schema-validation/Tests/Kind1ValidationTests/Kind1ValidationTests.swift` around lines 4 - 78, The test methods (testValidKind1Event, testInvalidKind1WrongKind, testInvalidKind1MissingFields, testDamusRealEvent) lack docstrings; add a short Swift doc comment (/// ...) above each function describing what the test verifies (e.g., "Validates a canonical kind-1 event", "Ensures kind routing rejects kind:0 when validating kind1Schema", "Verifies validation fails when required fields are missing", "Validates a real Damus kind-1 event"), keeping the comments concise and matching existing MARK sections to satisfy docstring coverage guidelines..github/workflows/schema-validation.yml (1)
3-7: Consider adding path filters to reduce unnecessary CI runs.The workflow currently runs on every push/PR to master. Since this only tests the schema-validation package, consider scoping it to relevant paths to avoid unnecessary CI runs.
♻️ Suggested improvement
on: push: branches: [master] + paths: + - 'schema-validation/**' + - '.github/workflows/schema-validation.yml' pull_request: branches: [master] + paths: + - 'schema-validation/**' + - '.github/workflows/schema-validation.yml'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/schema-validation.yml around lines 3 - 7, The workflow triggers currently run on every push/PR to master; narrow them by adding path filters under the on: push and on: pull_request blocks (the existing push and pull_request keys) so the job only runs when relevant files change (e.g., add paths: with patterns like packages/schema-validation/**, schema-validation/**, or other repo-specific paths); update both the push and pull_request blocks to include these path patterns so unrelated changes no longer trigger this CI workflow.schema-validation/Package.resolved (1)
3-46: Transitive dependencies pinned to branches due to upstream package stability.The
schemata-validator-swiftdependency (declared inPackage.swift) is pinned to themainbranch. This brings in transitive dependencies includingjsonschema.swift(master branch),schemata-swift(main branch), and others also pinned to branch heads.For a test-only package this is acceptable, but note that
jsonschema.swifthas stable releases available (v0.6.0+), whileschemata-swiftandschemata-validator-swifthave no tagged releases yet. Once the latter two packages publish releases, consider updating to pinned versions for better reproducibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema-validation/Package.resolved` around lines 3 - 46, The Package.resolved shows transitive dependencies pinned to branch heads (schemata-validator-swift, schemata-swift, jsonschema.swift); to improve reproducibility, change the dependency declaration in Package.swift for jsonschema.swift to a released semantic version (e.g., "0.6.0") and then run swift package update / resolve to regenerate Package.resolved, and when schemata-swift and schemata-validator-swift publish tagged releases replace their branch-based requirements with exact version requirements for schemata-swift and schemata-validator-swift and re-run swift package update to lock reproducible revisions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/schema-validation.yml:
- Around line 14-15: The GitHub Actions step named "Run kind-1 schema validation
tests" uses the wrong working directory string; update that step's run command
so it changes into the repository-root schema-validation directory before
running the tests (replace the current "cd damus/schema-validation && swift
test" value with a command that cd's into "schema-validation" then runs "swift
test"). Ensure the step's run field reflects the corrected directory name so the
CI executes tests from the actual package location.
In `@schema-validation/Tests/Kind1ValidationTests/Kind1ValidationTests.swift`:
- Around line 25-46: The test currently asserts XCTAssertNotNil(result) which
doesn't verify routing or schema rejection; update testInvalidKind1WrongKind to
directly validate the kind:0 event against kind1Schema (use the schema validator
API — e.g., call kind1Schema.validate(event) or
SchemataValidator.validate(event, using: kind1Schema) or the equivalent method
available) and assert that validation fails (expect error/false/nil as the
validator returns on invalid input), or if routing is internal and not testable
here, remove this test; ensure you reference the existing symbols kind1Schema,
SchemataValidator.getSchema, and validateNote when making the change.
---
Nitpick comments:
In @.github/workflows/schema-validation.yml:
- Around line 3-7: The workflow triggers currently run on every push/PR to
master; narrow them by adding path filters under the on: push and on:
pull_request blocks (the existing push and pull_request keys) so the job only
runs when relevant files change (e.g., add paths: with patterns like
packages/schema-validation/**, schema-validation/**, or other repo-specific
paths); update both the push and pull_request blocks to include these path
patterns so unrelated changes no longer trigger this CI workflow.
In `@schema-validation/Package.resolved`:
- Around line 3-46: The Package.resolved shows transitive dependencies pinned to
branch heads (schemata-validator-swift, schemata-swift, jsonschema.swift); to
improve reproducibility, change the dependency declaration in Package.swift for
jsonschema.swift to a released semantic version (e.g., "0.6.0") and then run
swift package update / resolve to regenerate Package.resolved, and when
schemata-swift and schemata-validator-swift publish tagged releases replace
their branch-based requirements with exact version requirements for
schemata-swift and schemata-validator-swift and re-run swift package update to
lock reproducible revisions.
In `@schema-validation/Tests/Kind1ValidationTests/Kind1ValidationTests.swift`:
- Around line 4-78: The test methods (testValidKind1Event,
testInvalidKind1WrongKind, testInvalidKind1MissingFields, testDamusRealEvent)
lack docstrings; add a short Swift doc comment (/// ...) above each function
describing what the test verifies (e.g., "Validates a canonical kind-1 event",
"Ensures kind routing rejects kind:0 when validating kind1Schema", "Verifies
validation fails when required fields are missing", "Validates a real Damus
kind-1 event"), keeping the comments concise and matching existing MARK sections
to satisfy docstring coverage guidelines.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 23e2d73e-131d-44ff-bc12-5ce610338e11
📒 Files selected for processing (4)
.github/workflows/schema-validation.ymlschema-validation/Package.resolvedschema-validation/Package.swiftschema-validation/Tests/Kind1ValidationTests/Kind1ValidationTests.swift
Comment on lines
+14
to
+15
| - name: Run kind-1 schema validation tests | ||
| run: cd damus/schema-validation && swift test |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find the actual location of the schema-validation Package.swift
fd -t f "Package.swift" | grep -i schemaRepository: damus-io/damus
Length of output: 89
Incorrect working directory path will cause CI failure.
The workflow attempts to run swift test in damus/schema-validation, but the schema-validation package is located at schema-validation/ in the repository root. This path mismatch will cause the workflow to fail.
Proposed fix
- name: Run kind-1 schema validation tests
- run: cd damus/schema-validation && swift test
+ run: cd schema-validation && swift test📝 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
| - name: Run kind-1 schema validation tests | |
| run: cd damus/schema-validation && swift test | |
| - name: Run kind-1 schema validation tests | |
| run: cd schema-validation && swift test |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/schema-validation.yml around lines 14 - 15, The GitHub
Actions step named "Run kind-1 schema validation tests" uses the wrong working
directory string; update that step's run command so it changes into the
repository-root schema-validation directory before running the tests (replace
the current "cd damus/schema-validation && swift test" value with a command that
cd's into "schema-validation" then runs "swift test"). Ensure the step's run
field reflects the corrected directory name so the CI executes tests from the
actual package location.
Comment on lines
+25
to
+46
| func testInvalidKind1WrongKind() { | ||
| // validateNote routes by kind field, so kind:0 validates against kind0Schema. | ||
| // Verify kind1Schema exists and rejects kind:0 when validated directly. | ||
| let kind1Schema = SchemataValidator.getSchema("kind1Schema") | ||
| XCTAssertNotNil(kind1Schema, "kind1Schema should exist in the registry") | ||
|
|
||
| // A kind:0 event routed through validateNote goes to kind0Schema, not kind1Schema. | ||
| // This test verifies the routing: kind:0 is not validated as kind:1. | ||
| let event: [String: Any] = [ | ||
| "id": String(repeating: "a", count: 64), | ||
| "pubkey": String(repeating: "b", count: 64), | ||
| "created_at": 1670000000, | ||
| "kind": 0, | ||
| "tags": [] as [[String]], | ||
| "content": "Hello, Nostr!", | ||
| "sig": String(repeating: "e", count: 64) + String(repeating: "f", count: 64) | ||
| ] | ||
|
|
||
| let result = SchemataValidator.validateNote(event) | ||
| // kind:0 routes to kind0Schema — the event is validated against a different schema | ||
| XCTAssertNotNil(result, "kind:0 should be handled by validateNote") | ||
| } |
Contributor
There was a problem hiding this comment.
Test assertion doesn't verify the intended behavior.
The test is named testInvalidKind1WrongKind and the comments describe testing "routing," but XCTAssertNotNil(result) will always pass since validateNote returns a result struct regardless of schema routing. This test doesn't actually verify that a kind:0 event is rejected when validated against the kind-1 schema.
Consider either:
- Directly validating the kind:0 event against
kind1Schemaand asserting it fails, or - Removing this test if the routing behavior is internal to
SchemataValidatorand not meant to be tested here.
💡 Suggested approach to test schema-specific validation
func testInvalidKind1WrongKind() {
- // validateNote routes by kind field, so kind:0 validates against kind0Schema.
- // Verify kind1Schema exists and rejects kind:0 when validated directly.
- let kind1Schema = SchemataValidator.getSchema("kind1Schema")
- XCTAssertNotNil(kind1Schema, "kind1Schema should exist in the registry")
-
- // A kind:0 event routed through validateNote goes to kind0Schema, not kind1Schema.
- // This test verifies the routing: kind:0 is not validated as kind:1.
+ // Verify that a kind:0 event fails validation against the kind-1 schema directly
let event: [String: Any] = [
"id": String(repeating: "a", count: 64),
"pubkey": String(repeating: "b", count: 64),
"created_at": 1670000000,
"kind": 0,
"tags": [] as [[String]],
"content": "Hello, Nostr!",
"sig": String(repeating: "e", count: 64) + String(repeating: "f", count: 64)
]
- let result = SchemataValidator.validateNote(event)
- // kind:0 routes to kind0Schema — the event is validated against a different schema
- XCTAssertNotNil(result, "kind:0 should be handled by validateNote")
+ // If SchemataValidator supports direct schema validation, use it:
+ // let result = SchemataValidator.validate(event, against: "kind1Schema")
+ // XCTAssertFalse(result.valid, "kind:0 event should fail kind-1 schema validation")
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@schema-validation/Tests/Kind1ValidationTests/Kind1ValidationTests.swift`
around lines 25 - 46, The test currently asserts XCTAssertNotNil(result) which
doesn't verify routing or schema rejection; update testInvalidKind1WrongKind to
directly validate the kind:0 event against kind1Schema (use the schema validator
API — e.g., call kind1Schema.validate(event) or
SchemataValidator.validate(event, using: kind1Schema) or the equivalent method
available) and assert that validation fails (expect error/false/nil as the
validator returns on invalid input), or if routing is internal and not testable
here, remove this test; ensure you reference the existing symbols kind1Schema,
SchemataValidator.getSchema, and validateNote when making the change.
Collaborator
Author
|
reworking this. |
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
NostrEventserialization produces NIP-01 compliant JSONschemata-validator-swiftto validateevent_to_json()output against the kind-1 JSON schemaNostrEventinstances (not dummy JSON), proving the serialization pipeline is spec-compliantTests
NostrEvent(content:keypair:kind:), serializes, validatestest_notefixture from existing test dataChanges
damusTests/SchemaValidationTests.swift— new test filedamus.xcodeproj/project.pbxproj— add SchemataValidator as test dependencyPackage.resolved— updated package pinsNotes
xcodebuild test, not at runtime in the appTest plan
xcodebuild test -scheme damus -only-testing:damusTests/SchemaValidationTests— 4/4 pass🤖 Generated with Claude Code