Add contributors into the activity claim#115
Conversation
🦋 Changeset detectedLatest commit: 0c93e39 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 |
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughReplaces per-contribution Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HypercertOps as HypercertOperationsImpl
participant ContributorSvc as Contributor Service / Records
participant HypercertStore as Hypercert Record Store
User->>HypercertOps: createHypercert(params with contributions)
HypercertOps->>HypercertOps: processContributors(contributions)
loop per contribution
HypercertOps->>HypercertOps: resolveContributorIdentity(identity)
alt identity needs creation
HypercertOps->>ContributorSvc: addContributorInformation(params)
ContributorSvc-->>HypercertOps: { uri, cid }
HypercertOps->>User: emit contributorCreated event
else use existing identity ref
end
HypercertOps->>HypercertOps: resolveContributionDetails(details)
alt details need creation
HypercertOps->>ContributorSvc: createContributionDetails(params)
ContributorSvc-->>HypercertOps: { uri, cid }
end
HypercertOps->>HypercertOps: assemble resolved entry (identity, contributionWeight, contributionDetails)
end
HypercertOps->>HypercertStore: createHypercertRecord(resolved contributors, rights, image, locations)
HypercertStore-->>HypercertOps: { uri, cid }
HypercertOps-->>User: { uri, cid }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
aspiers
force-pushed
the
activity-claim-rkey
branch
from
7cf81f5 to
3b58749
Compare
s-adamantine
force-pushed
the
activity-claim-rkey
branch
from
750cd44 to
3bece2e
Compare
aspiers
force-pushed
the
activity-claim-rkey
branch
from
3bece2e to
fb29a89
Compare
aspiers
force-pushed
the
feat/contribution-weight
branch
from
c9a5e0d to
b707c27
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: 3
🤖 Fix all issues with AI agents
In @.changeset/feat-contribution-weight.md:
- Around line 5-19: Update the changeset to explicitly state this is a breaking
change in `@hypercerts-org/sdk-core` and replace the old example to use the new
contributionDetails structure (not role) showing the new contributionWeight
field; specifically, mention that consumers must now pass contributionDetails
(array/object) with properties like contributors and contributionWeight (or
weight if that's the renamed prop in the API) instead of the former role field,
and update the example snippet to reflect the exact API shape used by the
hypercerts.create method so readers know the breaking change and correct usage.
In `@packages/sdk-core/src/repository/HypercertOperationsImpl.ts`:
- Around line 1219-1322: Replace the generic throws in
resolveContributionDetails and resolveContributorIdentity with the SDK
validation error type: instead of throw new Error("Invalid contributionDetails
format") and throw new Error("Invalid contributorIdentity format"), throw new
ValidationError("Invalid contributionDetails format") and throw new
ValidationError("Invalid contributorIdentity format") (and import/ensure
ValidationError from the sdk-core error hierarchy), so invalid input is
classified as a validation failure rather than a NetworkError.
- Around line 1353-1373: The public HypercertOperations.addContribution
signature in the repository interface is out of sync with the implementation:
update the HypercertOperations (or relevant repository interface) declaration of
addContribution to match the implementation (accept optional hypercertUri?:
string, contributors?: string[], role: string, description?: string, startDate?:
string, endDate?: string, and an index signature [key: string]: unknown) and
ensure the return type remains Promise<CreateResult> so the interface and the
implementation types align.
🧹 Nitpick comments (1)
packages/sdk-core/src/repository/HypercertOperationsImpl.ts (1)
435-452: Avoid acceptingweightwithout using it increateContributionsWithProgress.
Right now the type suggests support, but the value is silently ignored. Either thread it through or remove it to prevent confusion.♻️ Optional cleanup
contributions: Array<{ contributors: Array<string | { uri: string; cid: string }>; role: string; description?: string; - weight?: string; }>,
Adds support for the contributionWeight field in the SDK contributor structure, aligning with the updated org.hypercerts.claim.activity lexicon. Changes: - Add weight?: string to CreateHypercertParams.contributions - Thread contributionWeight through processContributors() and createHypercertRecord() - Update createContributionsWithProgress() type signature - Add 3 test cases for weight handling The field is optional and stored as a string to avoid float precision issues.
Adds support for contributor identity to be either a string DID or a
StrongRef to a contributor info record, aligning with the lexicon union type.
Changes:
- Update CreateHypercertParams.contributions.contributors type to
Array<string | { uri, cid }>
- Thread union type through processContributors and createHypercertRecord
- Filter to strings only when calling addContribution (legacy method)
- Add 2 test cases for StrongRef identity (106 tests passing)
Adds support for passing an existing contributionDetails StrongRef directly instead of creating a new record. Useful when the details record already exists. Priority order: 1. contributionDetailsRef (use existing StrongRef directly) 2. description (create new contributionDetails record) 3. role (use as inline string) 107 tests passing.
Adds contribution-specific timeframe fields that are included in the contributionDetails record when created. These should be a subset of the hypercert's overall timeframe. 108 tests passing.
Extra fields on contributions (beyond role, description, startDate, endDate, weight, contributionDetailsRef, contributors) are now spread onto the contributionDetails record when created. 124 tests passing.
BREAKING: contributions now use `contributionDetails` field instead of
flat role/description fields. Following AGENTS.md pattern.
contributionDetails accepts:
- string: inline role (e.g., "Developer")
- { uri, cid }: StrongRef to existing record
- CreateContributionDetailsParams: auto-create record
This unifies the interface for referencing contribution details.
124 tests passing.
aspiers
force-pushed
the
feat/contribution-weight
branch
from
b707c27 to
fde309e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/sdk-core/src/repository/HypercertOperationsImpl.ts (1)
315-323: Preserve explicit zero contribution weightsThe truthy check drops
"0"(or"0.0") weights if callers provide them. Prefer an undefined check.🛠️ Proposed fix
- if (c.contributionWeight) { + if (c.contributionWeight !== undefined) { contributor.contributionWeight = c.contributionWeight; }
♻️ Duplicate comments (2)
packages/sdk-core/src/repository/interfaces.ts (1)
951-956: SyncHypercertOperations.addContributionsignature with implementationThe interface still requires
contributorsand omitsstartDate/endDateand extra props, but the implementation now accepts them. This is a public typing mismatch.🛠️ Proposed interface update
addContribution(params: { hypercertUri?: string; - contributors: string[]; + contributors?: string[]; role: string; description?: string; + startDate?: string; + endDate?: string; + [key: string]: unknown; }): Promise<CreateResult>;packages/sdk-core/src/repository/HypercertOperationsImpl.ts (1)
1219-1322: UseValidationErrorfor invalid contributionDetails / contributorIdentityThese are input-validation failures; throwing a generic
Errorcauses them to be wrapped asNetworkErrorincreate(). UseValidationErrordirectly.As per coding guidelines, please use the sdk-core error hierarchy for validation failures.✅ Suggested fix
- throw new Error("Invalid contributionDetails format"); + throw new ValidationError("Invalid contributionDetails format"); ... - throw new Error("Invalid contributorIdentity format"); + throw new ValidationError("Invalid contributorIdentity format");
s-adamantine
changed the title
Kzoeps
force-pushed
the
feat/contribution-weight
branch
from
fde309e to
1baf9f3
Compare
Contributors now support:
- string: DID (e.g., "did:plc:abc123")
- { uri, cid }: StrongRef to existing contributorInformation record
- CreateContributorInformationParams: auto-create record
Added:
- CreateContributorInformationParams interface
- ContributorIdentityParams + ResolvedContributorIdentity types
- addContributorInformation() method
- resolveContributorIdentity() helper
- contributorCreated event
124 tests passing.
Kzoeps
force-pushed
the
feat/contribution-weight
branch
from
1baf9f3 to
1d45d7f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/sdk-core/src/repository/interfaces.ts (1)
186-199: Outdated docstring example for contributions.The example in the docstring still uses
roleanddescriptionfields which have been replaced bycontributionDetails. This will confuse developers reading the documentation.📝 Suggested docstring update
* contributions: [ * { * contributors: ["did:plc:lead-org"], -* role: "coordinator", -* description: "Project coordination and funding", +* contributionDetails: "coordinator", * }, * { * contributors: ["did:plc:local-partner"], -* role: "implementer", -* description: "On-ground planting and monitoring", +* contributionDetails: { +* role: "implementer", +* contributionDescription: "On-ground planting and monitoring", +* }, * }, * ],
🧹 Nitpick comments (2)
packages/sdk-core/src/repository/HypercertOperationsImpl.ts (2)
435-468:createContributionsWithProgresssilently discards non-string contributors.Line 451 filters to only string contributors, silently dropping any
StrongRefcontributors. This behavior is documented in the PR summary ("contribution creation ignores non-string contributor refs"), but consumers may not expect this silent data loss.Consider either:
- Logging a warning when StrongRef contributors are dropped, or
- Documenting this limitation in the method's JSDoc
🔧 Optional: Add warning for dropped contributors
for (const contrib of contributions) { + const stringContributors = contrib.contributors.filter((c): c is string => typeof c === "string"); + const droppedCount = contrib.contributors.length - stringContributors.length; + if (droppedCount > 0) { + this.logger?.warn(`createContributionsWithProgress: ${droppedCount} non-string contributor(s) ignored`); + } const contribResult = await this.addContribution({ hypercertUri, - contributors: contrib.contributors.filter((c): c is string => typeof c === "string"), + contributors: stringContributors, role: contrib.role, description: contrib.description, });
1443-1455: Consider using a helper or type guard for image resolution.The inline type assertions for
resolvedImagework correctly but could be extracted to improve readability, especially since similar logic exists inresolveCollectionImageInput. This is a minor suggestion for consistency.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on February 19
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| const contribResult = await this.addContribution({ | ||
| hypercertUri, | ||
| contributors: contrib.contributors, | ||
| contributors: contrib.contributors.filter((c): c is string => typeof c === "string"), |
There was a problem hiding this comment.
StrongRef contributors silently dropped in contribution creation
Medium Severity
The createContributionsWithProgress method's type signature accepts contributors as Array<string | { uri: string; cid: string }>, but the implementation at line 451 filters to only keep string entries: contrib.contributors.filter((c): c is string => typeof c === "string"). This silently discards any StrongRef contributor objects passed to the method. Additionally, the weight parameter is accepted in the type signature but never passed to addContribution, causing weight data to be silently ignored.
2 participants
Summary
Aligns SDK contributor structure with AGENTS.md patterns and
org.hypercerts.claim.activitylexicon.Fixes #100
BREAKING: New Contributions Interface
ContributionDetailsParams (union type)
string"Developer"{ uri, cid }{ uri: "at://...", cid: "..." }CreateContributionDetailsParams{ role, contributionDescription, startDate, endDate, ... }Example Usage
Tests
124 tests passing
Note
BREAKING: align contributions with lexicon beta.7
role/descriptionwith unifiedcontributionDetails(string | StrongRef | create-params); support optional proportionalweightContributorIdentityParams) and details (ContributionDetailsParams) with auto-creation of records when neededcontributorIdentity,contributionDetails, andcontributionWeight; adjust legacy contribution flowaddContributionto accept extra fields (startDate,endDate, etc.); introduceaddContributorInformationAPI and export new typescontributorCreatedevent; update interfaces/exports accordinglyWritten by Cursor Bugbot for commit 0c93e39. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.