Fix: Attach location to activity claim #85
Conversation
…-get-linked-to-hypercert
🦋 Changeset detectedLatest commit: c7040b0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughThe PR refactors evidence and location handling in the Hypercerts SDK core. Evidence is now saved as separate records with subject linking instead of being attached directly to hypercerts. New structured parameter types ( Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant SDK as HypercertOps
participant Validation as Lexicon<br/>Validator
participant BlobStore as Blob Storage
participant Records as Record DB
Client->>SDK: addEvidence(CreateHypercertEvidenceParams)
SDK->>SDK: Resolve subject URI
alt Content is Blob
SDK->>BlobStore: Upload blob
BlobStore-->>SDK: Blob reference
SDK->>SDK: Wrap in smallBlob
else Content is URI
SDK->>SDK: Use URI directly
end
SDK->>Validation: Validate evidence record
Validation-->>SDK: Validation result
alt Validation passes
SDK->>Records: Create evidence record
Records-->>SDK: Record URI + CID
SDK->>SDK: Emit evidenceAdded event
SDK-->>Client: Return UpdateResult
else Validation fails
SDK-->>Client: Throw ValidationError
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/sdk-core/src/repository/interfaces.ts (1)
745-754: Outdated JSDoc - parameters no longer match the interface.The JSDoc references
location.valueandlocation.geojson(lines 747, 751), butAttachLocationParamsuseslocation.location(for the data) and doesn't have ageojsonproperty. Update the documentation to match the new interface:📝 Suggested documentation fix
/** * Attaches a location to an existing hypercert. * * `@param` uri - AT-URI of the hypercert * `@param` location - Location data - * `@param` location.value - Location value (address, coordinates, or description) + * `@param` location.lpVersion - Location Protocol version + * `@param` location.locationType - Format identifier for the location data + * `@param` location.location - Location data as URI string or GeoJSON Blob * `@param` location.srs - Spatial Reference System (required). Use 'EPSG:4326' for WGS84 lat/lon coordinates. * `@param` location.name - Optional human-readable location name * `@param` location.description - Optional description of the location - * `@param` location.geojson - Optional GeoJSON blob for precise boundaries * `@returns` Promise resolving to location record result */packages/sdk-core/tests/repository/HypercertOperationsImpl.test.ts (1)
92-112: Test uses old evidence structure - may fail with new API.This test at lines 92-112 constructs evidence with the old structure (
$type,content.uri,content.$type,createdAt) and passes it directly tocreate(). Based on the PR changes,CreateHypercertParams.evidencenow expectsArray<Omit<CreateHypercertEvidenceParams, "subjectUri">>, which usescontent: string | Blobnot the nested object structure.Consider updating this test to use the new evidence format:
🔧 Suggested fix
it("should include evidence when provided", async () => { const evidence = [ { - $type: "org.hypercerts.claim.evidence" as const, - content: { - uri: "https://example.com/evidence", - $type: "org.hypercerts.defs#uri" as const, - }, title: "Evidence", - createdAt: new Date().toISOString(), + content: "https://example.com/evidence", }, ]; await hypercertOps.create({ ...validParams, evidence, });packages/sdk-core/src/repository/HypercertOperationsImpl.ts (1)
813-822: Bug:lpVersionfrom params is ignored; hardcoded to"1.0".The
AttachLocationParamsinterface defineslpVersion: stringas required, but line 815 hardcodes"1.0"instead of usinglocation.lpVersion.🐛 Proposed fix
const locationRecord: HypercertLocation = { $type: HYPERCERT_COLLECTIONS.LOCATION, - lpVersion: "1.0", + lpVersion: location.lpVersion, srs: location.srs, locationType: location.locationType, location: locationData, createdAt, name: location.name, description: location.description, };
🤖 Fix all issues with AI agents
In `@packages/sdk-core/src/repository/interfaces.ts`:
- Around line 756-762: The addEvidence method currently declares a return type
of UpdateResult but actually creates a new evidence record; change its return
type to CreateResult and update any related typings/usages accordingly (refer to
addEvidence, CreateHypercertEvidenceParams, createRecord, and compare with
attachLocation/addContribution which return CreateResult) so the signature
accurately reflects a creation operation.
🧹 Nitpick comments (3)
.changeset/fix-add-evidence.md (1)
1-6: Consider documenting the breaking change more explicitly.The changeset description mentions the internal change but doesn't clearly state this is a breaking API change. The
addEvidencemethod signature changed from(hypercertUri: string, evidence: HypercertEvidence[])to(evidence: CreateHypercertEvidenceParams). Users upgrading will need to update their code.Consider expanding the description:
**BREAKING**: `addEvidence` now accepts a single `CreateHypercertEvidenceParams` object instead of `(hypercertUri, evidence[])`. Evidence is saved as a separate record with a subject reference to the hypercert, rather than being embedded directly.packages/sdk-core/tests/repository/HypercertOperationsImpl.test.ts (1)
479-563: Good test coverage for attachLocation flows.The test suite covers both key paths:
- String URI → wrapped with
org.hypercerts.defs#uri- GeoJSON Blob → uploaded and wrapped with
org.hypercerts.defs#smallBlobConsider adding error handling tests for completeness:
- Blob upload failure scenario
- Invalid hypercert URI scenario
- Network error during location record creation
packages/sdk-core/src/repository/HypercertOperationsImpl.ts (1)
858-866: Minor: JSDoc param description doesn't match actual type.The
@paramdescription referencesHypercertEvidenceInputbut the actual parameter type isCreateHypercertEvidenceParams.📝 Suggested fix
/** * Adds evidence to any subject via the subject ref. * - * `@param` evidence - HypercertEvidenceInput + * `@param` evidence - Evidence creation parameters including subject URI and content * `@returns` Promise resolving to update result * `@throws` {`@link` ValidationError} if validation fails * `@throws` {`@link` NetworkError} if the operation fails */
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/fix-add-evidence.mdpackages/sdk-core/src/index.tspackages/sdk-core/src/repository/HypercertOperationsImpl.tspackages/sdk-core/src/repository/interfaces.tspackages/sdk-core/src/repository/types.tspackages/sdk-core/tests/repository/HypercertOperationsImpl.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use strict TypeScript with strict: true configuration
Avoid any types (warns in ESLint)
Prefix unused variables with underscore (_)
Use Zod schemas for runtime validation
Use one class/module per file
Classes should use PascalCase naming (e.g., OAuthClient, Repository)
Files should use PascalCase for classes and camelCase for utilities
Types and Interfaces should use PascalCase with descriptive names
Constants should use UPPER_SNAKE_CASE
Use the error hierarchy from sdk-core/src/core/errors.ts with base HypercertsError and specific error classes (ValidationError, AuthenticationError, NetworkError, NotFoundError, SDSRequiredError)
Files:
packages/sdk-core/src/index.tspackages/sdk-core/tests/repository/HypercertOperationsImpl.test.tspackages/sdk-core/src/repository/types.tspackages/sdk-core/src/repository/HypercertOperationsImpl.tspackages/sdk-core/src/repository/interfaces.ts
packages/sdk-*/src/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
Export types from dedicated entrypoints
Files:
packages/sdk-core/src/index.ts
**/tests/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/tests/**/*.test.ts: Test files should mirror source structure and be placed in tests/ directory with *.test.ts extension
Minimum test coverage: 70% for lines, functions, and statements; 60% for branches
Use mocks from src/testing/ for unit tests
Use fixtures from tests/utils/fixtures.ts in unit tests
Files:
packages/sdk-core/tests/repository/HypercertOperationsImpl.test.ts
🧠 Learnings (4)
📚 Learning: 2026-01-06T17:56:38.894Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-sdk PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T17:56:38.894Z
Learning: Applies to packages/sdk-*/src/index.ts : Export types from dedicated entrypoints
Applied to files:
packages/sdk-core/src/index.tspackages/sdk-core/src/repository/types.ts
📚 Learning: 2026-01-06T17:56:38.894Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-sdk PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T17:56:38.894Z
Learning: One record type requires: types in services/<domain>/types.ts, Zod schemas in services/<domain>/schemas.ts, lexicon registration in lexicons/<domain>/index.ts, operations in repository/<Domain>OperationsImpl.ts, tests in tests/repository/, and optional React hook in sdk-react/src/hooks/
Applied to files:
packages/sdk-core/src/index.tspackages/sdk-core/src/repository/interfaces.ts
📚 Learning: 2026-01-06T17:56:38.894Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-sdk PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T17:56:38.894Z
Learning: Applies to **/tests/**/*.test.ts : Use mocks from src/testing/ for unit tests
Applied to files:
packages/sdk-core/tests/repository/HypercertOperationsImpl.test.ts
📚 Learning: 2026-01-06T17:56:38.894Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-sdk PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T17:56:38.894Z
Learning: Applies to **/*.{ts,tsx} : Use the error hierarchy from sdk-core/src/core/errors.ts with base HypercertsError and specific error classes (ValidationError, AuthenticationError, NetworkError, NotFoundError, SDSRequiredError)
Applied to files:
packages/sdk-core/tests/repository/HypercertOperationsImpl.test.tspackages/sdk-core/src/repository/types.tspackages/sdk-core/src/repository/HypercertOperationsImpl.tspackages/sdk-core/src/repository/interfaces.ts
🧬 Code graph analysis (3)
packages/sdk-core/tests/repository/HypercertOperationsImpl.test.ts (1)
packages/sdk-core/src/auth/permissions.ts (1)
blob(598-606)
packages/sdk-core/src/repository/HypercertOperationsImpl.ts (5)
packages/sdk-core/src/repository/interfaces.ts (2)
AttachLocationParams(309-322)CreateHypercertEvidenceParams(241-278)packages/sdk-core/src/repository/types.ts (2)
CreateResult(26-29)UpdateResult(34-37)packages/sdk-core/src/services/hypercerts/types.ts (4)
HypercertLocation(87-87)HYPERCERT_COLLECTIONS(42-42)HypercertEvidence(83-83)validate(31-31)packages/sdk-core/src/core/errors.ts (2)
NetworkError(209-220)ValidationError(170-181)packages/sdk-core/src/lexicons.ts (1)
HYPERCERT_COLLECTIONS(125-190)
packages/sdk-core/src/repository/interfaces.ts (2)
packages/sdk-core/src/index.ts (4)
AttachLocationParams(40-40)CreateHypercertEvidenceParams(41-41)CreateResult(20-20)UpdateResult(21-21)packages/sdk-core/src/types.ts (2)
CreateResult(46-46)UpdateResult(47-47)
🔇 Additional comments (10)
packages/sdk-core/src/repository/types.ts (1)
114-117: LGTM!The
HypercertEvidenceInputtype is well-designed. UsingOmitto exclude SDK-computed fields ($type,createdAt,subject,content) and replacing them with user-facing inputs (subjectUri,content: string | Blob) provides a clean API boundary.packages/sdk-core/src/repository/interfaces.ts (2)
309-322: LGTM!The
AttachLocationParamsinterface is well-designed with:
- Clear field documentation
- Comprehensive JSDoc examples for both URI-based and GeoJSON Blob usage patterns
- Appropriate use of the
(string & {})pattern for extensible union types
241-278: The[k: string]: unknownindex signature is a valid TypeScript pattern in strict mode and does not conflict with explicit properties—all property types are assignable tounknown. The destructuring usage inaddEvidence(const { subjectUri, content, ...rest } = evidence) confirms this is an intentional design choice to capture additional custom fields. No type safety concerns exist; this pattern is appropriate when extensibility is needed.packages/sdk-core/src/index.ts (1)
28-43: LGTM!New public types are correctly exported from their dedicated source files, following the coding guideline to "Export types from dedicated entrypoints." The exports are logically grouped with related types from the same modules.
packages/sdk-core/tests/repository/HypercertOperationsImpl.test.ts (1)
642-722: LGTM!Comprehensive test coverage for the new
addEvidencemethod:
- URI string content path verified
- Blob upload path with
smallBlobwrapping verified- Event emission (
evidenceAdded) properly testedThe test structure is consistent with
attachLocationtests, maintaining good code organization.packages/sdk-core/src/repository/HypercertOperationsImpl.ts (5)
12-36: LGTM!Import updates correctly introduce the new parameter types (
AttachLocationParams,CreateHypercertEvidenceParams) andvalidatefunction to support the refactored methods.
294-313: LGTM!Parameter type update is consistent with the refactored
attachLocationmethod signature.
839-848: LGTM - Core fix for PR objective.The strong reference linking correctly attaches the location record to the hypercert, fulfilling the PR objective of "attaching location to activity claim." The AT Protocol
strongRefpattern is used correctly.
790-811: LGTM!Content type discrimination and handling is well-implemented. String content correctly creates a URI reference, while Blob content is uploaded and stored as a smallBlob reference with appropriate error handling.
866-921: LGTM - Evidence refactored to use separate records with subject linking.The refactored
addEvidencemethod correctly:
- Handles both URI and Blob content with the same pattern as
attachLocation- Creates a standalone evidence record with a subject reference
- Validates against the lexicon before creation
- Emits the
evidenceAddedevent on successThe use of object spread for passing through additional properties is appropriate since the lexicon validation will catch any invalid fields.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| /** | ||
| * Adds evidence to an existing hypercert. | ||
| * | ||
| * @param uri - AT-URI of the hypercert | ||
| * @param evidence - Array of evidence items to add | ||
| * @param evidence - Evidence item to add | ||
| * @returns Promise resolving to update result | ||
| */ | ||
| addEvidence(uri: string, evidence: HypercertEvidence[]): Promise<UpdateResult>; | ||
| addEvidence(evidence: CreateHypercertEvidenceParams): Promise<UpdateResult>; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the implementation of addEvidence in HypercertOperationsImpl
find . -name "*Impl.ts" -o -name "*Implementation.ts" | head -20Repository: hypercerts-org/hypercerts-sdk
Length of output: 440
🏁 Script executed:
#!/bin/bash
# Search for addEvidence method implementation
rg "addEvidence" -A 10 -B 2 --type tsRepository: hypercerts-org/hypercerts-sdk
Length of output: 8937
🏁 Script executed:
#!/bin/bash
# Check for attachLocation and addContribution signatures to compare
rg "attachLocation|addContribution" -B 2 -A 3 --type ts | head -60Repository: hypercerts-org/hypercerts-sdk
Length of output: 5362
🏁 Script executed:
#!/bin/bash
# Check the type definitions for UpdateResult and CreateResult
rg "type UpdateResult|type CreateResult|interface UpdateResult|interface CreateResult" --type ts -A 3Repository: hypercerts-org/hypercerts-sdk
Length of output: 539
🏁 Script executed:
#!/bin/bash
# Check attachLocation and addContribution return types in interfaces.ts
rg "attachLocation|addContribution" -B 5 -A 1 packages/sdk-core/src/repository/interfaces.tsRepository: hypercerts-org/hypercerts-sdk
Length of output: 649
🏁 Script executed:
#!/bin/bash
# Check implementation of attachLocation and addContribution to see what they actually create
rg "async attachLocation|async addContribution" -A 20 packages/sdk-core/src/repository/HypercertOperationsImpl.ts | head -80Repository: hypercerts-org/hypercerts-sdk
Length of output: 2400
Change return type from UpdateResult to CreateResult.
The method creates a new evidence record (via createRecord), not updating an existing one. The return type should be CreateResult to match the semantic meaning and maintain consistency with similar methods like attachLocation and addContribution, which also create records and return CreateResult.
🤖 Prompt for AI Agents
In `@packages/sdk-core/src/repository/interfaces.ts` around lines 756 - 762, The
addEvidence method currently declares a return type of UpdateResult but actually
creates a new evidence record; change its return type to CreateResult and update
any related typings/usages accordingly (refer to addEvidence,
CreateHypercertEvidenceParams, createRecord, and compare with
attachLocation/addContribution which return CreateResult) so the signature
accurately reflects a creation operation.
1 participant
Fixes #77
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.