Conversation
🦋 Changeset detectedLatest commit: 1e5e282 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 |
|
Important Review skippedThis PR was authored by the user configured for CodeRabbit reviews. By default, CodeRabbit skips reviewing PRs authored by this user. It's recommended to use a dedicated user account to post CodeRabbit review feedback. To trigger a single review, invoke the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughProfile storage moved from app.bsky.actor.profile to app.certified.profile; profiles now support pronouns and use Hypercert image records for avatar/banner. Blob URL utilities and CID extraction added. Blob upload API now returns BlobRef. ProfileOperations and related types/methods were split into Bluesky vs Certified flows and tests updated. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 2
🤖 Fix all issues with AI agents
In `@packages/sdk-core/package.json`:
- Line 91: Replace the local filesystem dependency for "@hypercerts-org/lexicon"
in package.json with a published npm version; specifically change the value that
currently points to "file:/home/kzoeps/Projects/gainforest/hypercerts-lexicon"
to a semver string (e.g., "^0.10.0-beta.13") so CI and other developers can
install from the registry—if the required changes aren’t yet published, publish
the package first and then update the "@hypercerts-org/lexicon" entry to the
released version.
In `@packages/sdk-core/src/repository/ProfileOperationsImpl.ts`:
- Around line 245-247: The catch in ProfileOperationsImpl (around the getRecord
call) is too broad and swallows all errors; change it to only handle the "record
not found" case (e.g., check error.status === 404 or error.name/type/message
that your PDS client uses for not-found) and in that case fall through to return
just the handle, but for any other error rethrow (or log and rethrow) so
network/auth/server errors aren't hidden; update the error check around
getRecord in ProfileOperationsImpl to discriminate and only suppress the
specific not-found error.
🧹 Nitpick comments (1)
packages/sdk-core/tests/repository/ProfileOperationsImpl.test.ts (1)
8-8: Consider importingPROFILE_COLLECTIONfrom the implementation.The constant
PROFILE_COLLECTIONis duplicated here. If the value changes in the implementation but not in tests, tests could pass with incorrect assertions. Consider exporting the constant fromProfileOperationsImpl.tsor a shared constants file.
Kzoeps
force-pushed
the
profile
branch
5 times, most recently
from
a244549 to
ae706d9
Compare
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 (1)
packages/sdk-core/src/repository/ProfileOperationsImpl.ts (1)
294-303: Replace ProfileParams with CreateHypercertProfileParams and UpdateHypercertProfileParams.The
ProfileOperationsinterface usesProfileParamsfor bothcreate()andupdate()methods, but this violates SDK-core conventions. The correct types already exist:
create(params: ProfileParams)should becreate(params: CreateHypercertProfileParams)update(params: ProfileParams)should beupdate(params: UpdateHypercertProfileParams)
UpdateHypercertProfileParamscorrectly implements the convention pattern (Omit<Partial<CreateHypercertProfileParams>, ...>with nullable image fields), so update the interface signatures and remove the unusedProfileParamsdefinition.
🤖 Fix all issues with AI agents
In `@packages/sdk-core/src/services/hypercerts/types.ts`:
- Around line 163-223: Add a missing HypercertProfileParams union type for the
profile variants (e.g., type HypercertProfileParams =
CreateHypercertProfileParams | UpdateHypercertProfileParams) and refactor
UpdateHypercertProfileParams to use OverrideProperties for consistency: replace
the current Omit<Partial<CreateHypercertProfileParams>, "avatar" | "banner"> & {
avatar?: HypercertImage | null; banner?: HypercertImage | null; } with
OverrideProperties<Partial<CreateHypercertProfileParams>, { avatar?:
HypercertImage | null; banner?: HypercertImage | null; }>, keeping the existing
CreateHypercertProfileParams and HypercertImage symbols intact.
🧹 Nitpick comments (1)
.changeset/profile-certified-lexicon.md (1)
1-25: Add a pre‑1.0 semver caveat for the minor bump.The changeset already lists breaking changes, but a short note that these packages are still 0.x will better set expectations for consumers.
✏️ Suggested wording
**Breaking Changes:** +_Note: These packages are still pre‑1.0; minor bumps may include breaking changes._ - `profile.avatar` and `profile.banner` are now `HypercertImageRecord | undefined` instead of `string | undefined` - Profile data is stored in a different collection (`app.certified.profile` instead of `app.bsky.actor.profile`)Based on learnings: For pre-1.0 libraries (0.x), minor version bumps can introduce breaking changes. In changeset files, note that breaking changes may occur with minor bumps and clearly document any API changes, removal or behavioral changes, and compatibility caveats.
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)
1798-1807: Critical: evaluator shape mismatches type/schema (build failure).
HypercertEvaluation.evaluatorsis currently typed asstring[](TS2322 in CI), but the record now writes{ did }[]. This will also fail lexicon validation if the schema still expects strings. Please align the SDK type and lexicon schema, or revert to the string array until the lexicon is updated.If the lexicon still expects strings, a quick fix is:
🔧 Minimal fix if schema expects string[]
- evaluators: params.evaluators.map((evaluator) => ({ did: evaluator })), + evaluators: params.evaluators,#!/bin/bash # Verify the lexicon schema and generated types for evaluators shape. rg -n "evaluators" node_modules/@hypercerts-org/lexicon -g '*.json' -g '*.d.ts' -g '*.ts'
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/sdk-core/src/repository/HypercertOperationsImpl.ts`:
- Around line 1803-1809: Add a new CreateEvaluationParams type (and the required
Main/alias/Update/union types for evaluation) to
packages/sdk-core/src/services/hypercerts/types.ts, then update the
HypercertOperationsImpl.addEvaluation method signature to async
addEvaluation(params: CreateEvaluationParams): Promise<CreateResult>; inside
addEvaluation build the HypercertEvaluation using params and preserve
caller-provided $type and createdAt via nullish coalescing (e.g. $type:
params.$type ?? HYPERCERT_COLLECTIONS.EVALUATION and createdAt: params.createdAt
?? new Date().toISOString()), and keep mapping evaluators from params.evaluators
to { did } objects before returning the CreateResult.
🧹 Nitpick comments (1)
packages/sdk-core/src/repository/ProfileOperationsImpl.ts (1)
231-240: Consider runtime validation for record fields.The type assertions (
as string | undefined,as HypercertImageRecord | undefined) are unsafe casts fromRecord<string, unknown>. If the PDS returns unexpected data shapes, these could lead to runtime issues downstream.Per coding guidelines, consider using Zod schemas for runtime validation of the profile record structure.
| const evaluationRecord: HypercertEvaluation = { | ||
| $type: HYPERCERT_COLLECTIONS.EVALUATION, | ||
| subject: { uri: subject.uri, cid: subject.cid }, | ||
| evaluators: params.evaluators, | ||
| evaluators: params.evaluators.map((evaluator) => ({ did: evaluator })), | ||
| summary: params.summary, | ||
| createdAt, | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's see the actual code at lines 1803-1809
echo "=== Code at lines 1803-1809 ==="
sed -n '1800,1815p' packages/sdk-core/src/repository/HypercertOperationsImpl.ts
# Find the HypercertEvaluation type definition
echo -e "\n=== HypercertEvaluation type definition ==="
rg -n "type HypercertEvaluation|interface HypercertEvaluation" packages/sdk-core/src/
# Check evaluators field definition
echo -e "\n=== Evaluators field in types ==="
rg -n "evaluators" packages/sdk-core/src/services/hypercerts/types.ts -A 2 -B 2Repository: hypercerts-org/hypercerts-sdk
Length of output: 1047
🏁 Script executed:
#!/bin/bash
# Check the lexicon definition for the Evaluation record
echo "=== Searching lexicon for Evaluation ==="
find . -name "*.ts" -o -name "*.json" | xargs grep -l "Evaluation" | grep -i lexicon | head -5
# Look for OrgHypercertsEvaluation in lexicon
echo -e "\n=== OrgHypercerts Evaluation type ==="
rg -n "OrgHypercertsEvaluation" packages/sdk-core/src/ -A 5 -B 1Repository: hypercerts-org/hypercerts-sdk
Length of output: 431
🏁 Script executed:
#!/bin/bash
# Check how evaluators is used elsewhere in the file
echo "=== All evaluators references in HypercertOperationsImpl ==="
rg -n "evaluators" packages/sdk-core/src/repository/HypercertOperationsImpl.ts -B 2 -A 2Repository: hypercerts-org/hypercerts-sdk
Length of output: 1190
🏁 Script executed:
#!/bin/bash
# Find OrgHypercertsClaimEvaluation definition
echo "=== Looking for OrgHypercertsClaimEvaluation definition ==="
rg -n "OrgHypercertsClaimEvaluation" packages/sdk-core/src/ -A 10 | head -50
# Check the main lexicon definitions
echo -e "\n=== Checking for lexicon files ==="
find packages/sdk-core/src -name "*lexicon*" -type fRepository: hypercerts-org/hypercerts-sdk
Length of output: 3924
🏁 Script executed:
#!/bin/bash
# Look for where OrgHypercertsClaimEvaluation comes from
echo "=== Imports of OrgHypercertsClaimEvaluation ==="
rg "import.*OrgHypercertsClaimEvaluation" packages/sdk-core/src/
# Check the schema files
echo -e "\n=== Schema and definition files ==="
find packages/sdk-core/src -name "*.ts" | xargs grep -l "evaluators.*did" | head -10Repository: hypercerts-org/hypercerts-sdk
Length of output: 217
🏁 Script executed:
#!/bin/bash
# Look at the types file more carefully
echo "=== Full types.ts file (first 200 lines) ==="
head -200 packages/sdk-core/src/services/hypercerts/types.tsRepository: hypercerts-org/hypercerts-sdk
Length of output: 7180
🏁 Script executed:
#!/bin/bash
# Check what `@hypercerts-org/lexicon` exports for OrgHypercertsClaimEvaluation
echo "=== Checking lexicon package ==="
# Since it's from `@hypercerts-org/lexicon`, let's see if there's a local copy or we need web search
find packages -name "lexicon*" -type d
# Check the lexicons.ts file
echo -e "\n=== Content of lexicons.ts ==="
cat packages/sdk-core/src/lexicons.tsRepository: hypercerts-org/hypercerts-sdk
Length of output: 5715
🌐 Web query:
@hypercerts-org/lexicon OrgHypercertsClaimEvaluation evaluators field type definition
💡 Result:
I couldn’t confirm the evaluators field’s type for OrgHypercertsClaimEvaluation from publicly accessible schema text.
What I was able to verify is that @hypercerts-org/lexicon is generated from the hypercerts-lexicon repository’s ATProto lexicon JSON schemas, but the repo pages that would show the actual JSON for the org.hypercerts.claim.evaluation record (where evaluators would be defined) weren’t retrievable in a way that exposed the file contents. [1]
The quickest way to get the exact type definition is to open the lexicon JSON for the claim evaluation record (e.g., org.hypercerts.claim.evaluation) and look at record.properties.evaluators (and any referenced defs). If the schema is published to ATProto, Lexicon Garden also suggests resolving it directly by NSID via https://lexicon.garden/nsid/<nsid>. [2]
Sources: [1], [2]
🏁 Script executed:
#!/bin/bash
# Find where EVALUATION_LEXICON_JSON comes from - it's from `@hypercerts-org/lexicon`
# Let's check if there's a local definition or if we need to check node_modules structure
echo "=== Searching for evaluation lexicon JSON files ==="
find packages -name "*evaluation*" -type f | grep -E "\.(json|ts)$"
# Look for lexicon JSON in a common location
echo -e "\n=== Checking node_modules structure ==="
find node_modules/@hypercerts-org/lexicon -name "*evaluation*" 2>/dev/null | head -20Repository: hypercerts-org/hypercerts-sdk
Length of output: 165
🏁 Script executed:
#!/bin/bash
# Let's check the generated types from the lexicon package
# Look for the type definition in `@hypercerts-org/lexicon`
echo "=== Looking for OrgHypercertsClaimEvaluation in node_modules ==="
find node_modules/@hypercerts-org/lexicon -name "*.d.ts" 2>/dev/null | xargs grep -l "OrgHypercertsClaimEvaluation" 2>/dev/null | head -5
# And check for the actual evaluation schema
echo -e "\n=== Looking for evaluation schema ==="
find node_modules/@hypercerts-org/lexicon -name "*.json" 2>/dev/null | xargs grep -l "evaluation" 2>/dev/null | head -5Repository: hypercerts-org/hypercerts-sdk
Length of output: 176
🏁 Script executed:
#!/bin/bash
# Check if nullish coalescing is used for $type and createdAt in the current code
echo "=== Checking for nullish coalescing pattern at lines 1803-1809 ==="
sed -n '1800,1815p' packages/sdk-core/src/repository/HypercertOperationsImpl.ts
# Look for similar patterns in other record creation methods
echo -e "\n=== Looking for other record creation methods with nullish coalescing ==="
rg -n "buildAttachmentRecord|createLocationRecord" packages/sdk-core/src/repository/ -A 10 | head -50Repository: hypercerts-org/hypercerts-sdk
Length of output: 5608
🏁 Script executed:
#!/bin/bash
# Get the full buildAttachmentRecord method to see nullish coalescing pattern
echo "=== Full buildAttachmentRecord method ==="
sed -n '1131,1155p' packages/sdk-core/src/repository/HypercertOperationsImpl.ts
# Now let's check the buildLocationRecord or createLocationRecord
echo -e "\n=== Looking for createLocationRecord ==="
rg -n "createLocationRecord|buildLocationRecord" packages/sdk-core/src/repository/HypercertOperationsImpl.ts -A 15 | head -60Repository: hypercerts-org/hypercerts-sdk
Length of output: 3233
🏁 Script executed:
#!/bin/bash
# Search for all nullish coalescing usage for $type and createdAt in the file
echo "=== Nullish coalescing for \$type and createdAt patterns ==="
rg -n "\$\?|createdAt.*\?\?" packages/sdk-core/src/repository/HypercertOperationsImpl.ts | head -20Repository: hypercerts-org/hypercerts-sdk
Length of output: 244
🏁 Script executed:
#!/bin/bash
# Let's try running TypeScript compiler to see if there's actually a TS error
echo "=== Checking if there's a TS compiler available ==="
which tsc
# Try to run tsc on just the specific file
echo -e "\n=== Running TypeScript check ==="
cd packages/sdk-core && npx tsc --noEmit src/repository/HypercertOperationsImpl.ts 2>&1 | grep -A 5 -B 5 "1806\|evaluators" | head -30Repository: hypercerts-org/hypercerts-sdk
Length of output: 219
🏁 Script executed:
#!/bin/bash
# Check if CreateEvaluationParams or similar type is defined
echo "=== Looking for CreateEvaluationParams type ==="
rg -n "CreateEvaluationParams|CreateHypercertEvaluationParams" packages/sdk-core/src/
# Check what types are defined in types.ts for other entities
echo -e "\n=== Checking what Create*Params types exist ==="
rg -n "^export type Create" packages/sdk-core/src/services/hypercerts/types.ts | head -20Repository: hypercerts-org/hypercerts-sdk
Length of output: 646
🏁 Script executed:
#!/bin/bash
# Check the full method signature and see if there's a params type
echo "=== Full addEvaluation method signature and start ==="
sed -n '1798,1815p' packages/sdk-core/src/repository/HypercertOperationsImpl.ts
# Look for other similar add* methods to see the pattern
echo -e "\n=== Other add* methods ==="
rg -n "async add\w+\(" packages/sdk-core/src/repository/HypercertOperationsImpl.ts | head -10Repository: hypercerts-org/hypercerts-sdk
Length of output: 1315
🏁 Script executed:
#!/bin/bash
# Check how measurement was created (similar pattern)
echo "=== Looking for addMeasurement method ==="
rg -n "async addMeasurement" packages/sdk-core/src/repository/HypercertOperationsImpl.ts -A 20 | head -40Repository: hypercerts-org/hypercerts-sdk
Length of output: 1031
🏁 Script executed:
#!/bin/bash
# Check the complete types.ts to see all Create*Params patterns
echo "=== All Create*Params and Update*Params types ==="
rg -n "export type (Create|Update)" packages/sdk-core/src/services/hypercerts/types.ts
# Look for EvaluationParams specifically
echo -e "\n=== Any Evaluation*Params types ==="
rg -n "Evaluation" packages/sdk-core/src/services/hypercerts/types.tsRepository: hypercerts-org/hypercerts-sdk
Length of output: 1194
🏁 Script executed:
#!/bin/bash
# Check CreateMeasurementParams to see the pattern
echo "=== CreateMeasurementParams definition ==="
rg -n "CreateMeasurementParams" packages/sdk-core/src/services/hypercerts/types.ts -A 10
# Check CreateAttachmentParams pattern
echo -e "\n=== CreateAttachmentParams definition ==="
rg -n "CreateAttachmentParams" packages/sdk-core/src/services/hypercerts/types.ts -A 10Repository: hypercerts-org/hypercerts-sdk
Length of output: 4769
🏁 Script executed:
#!/bin/bash
# Look at the addEvaluation method more carefully - check if $type uses nullish coalescing
echo "=== Complete addEvaluation method ==="
sed -n '1798,1825p' packages/sdk-core/src/repository/HypercertOperationsImpl.ts
# Compare with addMeasurement to see the pattern
echo -e "\n=== addMeasurement for comparison ==="
sed -n '1657,1690p' packages/sdk-core/src/repository/HypercertOperationsImpl.tsRepository: hypercerts-org/hypercerts-sdk
Length of output: 2579
🏁 Script executed:
#!/bin/bash
# Let's verify if there's supposed to be a CreateEvaluationParams type
# by checking the learnings guideline more carefully
echo "=== Looking for any UpdateEvaluationParams pattern ==="
rg -n "UpdateMeasurementParams|UpdateAttachmentParams" packages/sdk-core/src/services/hypercerts/types.ts
# Check if evaluation should have both Create and Update params
echo -e "\n=== Pattern: every entity should have Create/Update params ==="
rg -n "^export type (Create|Update)" packages/sdk-core/src/services/hypercerts/types.ts | grep -E "(Measurement|Attachment|Evaluation)"
# Look at the complete addMeasurement to see if it uses nullish coalescing
echo -e "\n=== Full measurementRecord creation with nullish coalescing check ==="
sed -n '1664,1677p' packages/sdk-core/src/repository/HypercertOperationsImpl.tsRepository: hypercerts-org/hypercerts-sdk
Length of output: 1359
🏁 Script executed:
#!/bin/bash
# Check if there's a pattern for params.$type and params.createdAt
echo "=== Looking for params.$type or params.createdAt patterns ==="
rg -n "params\.\$type|params\.createdAt" packages/sdk-core/src/repository/HypercertOperationsImpl.ts | head -20Repository: hypercerts-org/hypercerts-sdk
Length of output: 129
🏁 Script executed:
#!/bin/bash
# Let's check the entire types.ts structure around evaluation
echo "=== Context around HypercertEvaluation in types.ts ==="
sed -n '150,170p' packages/sdk-core/src/services/hypercerts/types.tsRepository: hypercerts-org/hypercerts-sdk
Length of output: 1121
Define CreateEvaluationParams type and refactor addEvaluation to follow SDK patterns.
The addEvaluation method violates the SDK's type definition guidelines:
-
Missing type definitions: Unlike
CreateMeasurementParamsandCreateAttachmentParams, there is noCreateEvaluationParamstype. According to guidelines, each lexicon entity should define five types: Main, HypercertXxx alias, CreateXxxParams, UpdateXxxParams, and XxxParams union type. Add these missing types topackages/sdk-core/src/services/hypercerts/types.ts. -
Incorrect method signature: The method uses an inline params object instead of
CreateEvaluationParams. Update the signature to:async addEvaluation(params: CreateEvaluationParams): Promise<CreateResult>
-
Missing nullish coalescing: Per guidelines, use nullish coalescing to preserve caller-supplied
$typeandcreatedAtfields when provided:$type: params.$type ?? HYPERCERT_COLLECTIONS.EVALUATION, createdAt: params.createdAt ?? new Date().toISOString(),
🧰 Tools
🪛 GitHub Actions: Test
[error] 1806-1806: TS2322: Type '{ did: string; }[]' is not assignable to type 'string[]'.
🤖 Prompt for AI Agents
In `@packages/sdk-core/src/repository/HypercertOperationsImpl.ts` around lines
1803 - 1809, Add a new CreateEvaluationParams type (and the required
Main/alias/Update/union types for evaluation) to
packages/sdk-core/src/services/hypercerts/types.ts, then update the
HypercertOperationsImpl.addEvaluation method signature to async
addEvaluation(params: CreateEvaluationParams): Promise<CreateResult>; inside
addEvaluation build the HypercertEvaluation using params and preserve
caller-provided $type and createdAt via nullish coalescing (e.g. $type:
params.$type ?? HYPERCERT_COLLECTIONS.EVALUATION and createdAt: params.createdAt
?? new Date().toISOString()), and keep mapping evaluators from params.evaluators
to { did } objects before returning the CreateResult.
There was a problem hiding this comment.
@Kzoeps I agree with @coderabbitai here - that's why I documented that pattern in AGENTS.md for agents to follow. That said, we need a whole bunch of cleanup on that front anyway, so we could also do it in a follow-up.
There was a problem hiding this comment.
@aspiers, understood! That makes sense given the broader cleanup work needed. Would you like me to create a follow-up issue to track the refactoring of addEvaluation to follow the CreateXxxParams pattern (along with any other similar methods that need the same treatment)? I can include:
- Define CreateEvaluationParams, UpdateEvaluationParams, and related types
- Update the method signature to use the new params type
- Apply nullish coalescing for
$typeandcreatedAtfields - Document any other methods that need similar updates
Just let me know if you'd like me to open that issue!
🧠 Learnings used
Learnt from: Kzoeps
Repo: hypercerts-org/hypercerts-sdk PR: 121
File: packages/sdk-core/src/repository/HypercertOperationsImpl.ts:1108-1167
Timestamp: 2026-01-28T10:29:44.154Z
Learning: In `packages/sdk-core/src/repository/HypercertOperationsImpl.ts`, record creation helper methods (e.g., `buildAttachmentRecord`, `createLocationRecord`) should use nullish coalescing to preserve caller-supplied `$type` and `createdAt` fields when provided, defaulting only when missing. Pattern: `$type: $type ?? HYPERCERT_COLLECTIONS.X, createdAt: createdAt ?? new Date().toISOString()`.
Learnt from: CR
Repo: hypercerts-org/hypercerts-sdk PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-29T02:51:21.569Z
Learning: Applies to packages/sdk-core/src/**/*.ts : For each lexicon entity type, define five types: lexicon Main type, HypercertXxx alias, CreateXxxParams, UpdateXxxParams, and XxxParams union type
Learnt from: CR
Repo: hypercerts-org/hypercerts-sdk PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-29T02:51:21.569Z
Learning: Applies to packages/sdk-core/src/**/*.ts : For each SDK method that creates a record, define a corresponding CreateXxxParams type with optional $type and createdAt fields auto-populated
Learnt from: aspiers
Repo: hypercerts-org/hypercerts-sdk PR: 84
File: .changeset/fix-add-evidence.md:1-17
Timestamp: 2026-01-21T04:15:20.542Z
Learning: `hypercerts-org/sdk-core` is currently at version 0.10.0-beta.4. Since it's a 0.x version, breaking changes can be introduced with minor version bumps according to semantic versioning conventions for pre-1.0 releases.
There was a problem hiding this comment.
@aspiers this wasn't part of the changes here so created a new issue for this and will be done as a follow up pr @coderabbitai
There was a problem hiding this comment.
@Kzoeps, thank you for creating the follow-up issue! That's a great approach—keeping this PR focused on the profile operations changes and addressing the evaluation refactoring separately makes sense.
If you have the issue number handy, feel free to share it so it can be tracked alongside the pattern documentation in AGENTS.md. Looking forward to seeing the refactoring in the follow-up PR! 🚀
🧠 Learnings used
Learnt from: Kzoeps
Repo: hypercerts-org/hypercerts-sdk PR: 121
File: packages/sdk-core/src/repository/HypercertOperationsImpl.ts:1108-1167
Timestamp: 2026-01-28T10:29:44.154Z
Learning: In `packages/sdk-core/src/repository/HypercertOperationsImpl.ts`, record creation helper methods (e.g., `buildAttachmentRecord`, `createLocationRecord`) should use nullish coalescing to preserve caller-supplied `$type` and `createdAt` fields when provided, defaulting only when missing. Pattern: `$type: $type ?? HYPERCERT_COLLECTIONS.X, createdAt: createdAt ?? new Date().toISOString()`.
Learnt from: aspiers
Repo: hypercerts-org/hypercerts-sdk PR: 84
File: .changeset/fix-add-evidence.md:1-17
Timestamp: 2026-01-21T04:15:20.542Z
Learning: `hypercerts-org/sdk-core` is currently at version 0.10.0-beta.4. Since it's a 0.x version, breaking changes can be introduced with minor version bumps according to semantic versioning conventions for pre-1.0 releases.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/sdk-core/src/lib/blob-url.ts`:
- Around line 107-116: The code in blob-url.ts extracts a CID by calling
ref.toString(), but ref is an object like { $link: "bafy..." } so to fix: inside
the conditional handling smallImage/largeImage (check imageType, image.image ->
imageData, and ref) replace the use of ref.toString() with ref.$link (or
String(ref.$link) to be safe) so the real CID is returned; ensure you only
access $link when ref is an object and present to avoid undefined.
🧹 Nitpick comments (1)
packages/sdk-core/src/services/hypercerts/types.ts (1)
738-739: Clarify the relationship betweenCreateProfileParamsandCreateHypercertProfileParams.
CreateProfileParamsappears to be a simpler variant (onlySetOptionalwithoutOverridePropertiesfor avatar/banner). Consider whether this is intentionally different fromCreateHypercertProfileParamsor if it should be consolidated. If both are needed, add a doc comment explaining the distinction.
| const evaluationRecord: HypercertEvaluation = { | ||
| $type: HYPERCERT_COLLECTIONS.EVALUATION, | ||
| subject: { uri: subject.uri, cid: subject.cid }, | ||
| evaluators: params.evaluators, | ||
| evaluators: params.evaluators.map((evaluator) => ({ did: evaluator })), | ||
| summary: params.summary, | ||
| createdAt, | ||
| }; |
There was a problem hiding this comment.
@Kzoeps I agree with @coderabbitai here - that's why I documented that pattern in AGENTS.md for agents to follow. That said, we need a whole bunch of cleanup on that front anyway, so we could also do it in a follow-up.
|
|
||
| **Core SDK (`@hypercerts-org/sdk-core`):** | ||
|
|
||
| - Switch profile storage from `app.bsky.actor.profile` to `app.certified.profile` collection |
There was a problem hiding this comment.
Not sure if you've been given the full context of all the discussions we had around profile records, but the strategy we've settled on is to support use of both of these lexicons, not switch entirely from the bsky one to the certified one. And "both" means allowing use of either one, or both together, where we still need to figure out some of the details about which data from which profile would be read/written in any given scenario.
So as written here, this sounds like the approach needs revising.
There was a problem hiding this comment.
this has now been changed. for now i've kept it as the getCertifiedProfile or getBskyProfile so either one .if we decide to do both together i think we can just reuse these functions and merge the results.
| * Implementation of profile operations for user profile management. | ||
| * | ||
| * Profiles in AT Protocol are stored as records in the `app.bsky.actor.profile` | ||
| * Profiles in AT Protocol are stored as records in the `app.certified.profile` |
There was a problem hiding this comment.
same here as above two different types and 2 different operations
|
working on this will finish it up tmrw |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/sdk-core/tests/repository/BlobOperationsImpl.test.ts (1)
117-147:⚠️ Potential issue | 🟡 MinorType signature doesn't reflect SDS response flexibility.
The
upload()method returnsPromise<BlobRef>but the SDS path (line 176) returnsresult.blobdirectly without normalization, which can have differentrefformats (CID instance,{ $link: string }, or plain string). The test at line 131 explicitly validates string ref handling. Consider either:
- Normalizing SDS responses to match the
BlobReftype contract- Using a union return type that reflects the actual flexibility:
Promise<BlobRef | { ref: string | { $link: string }; mimeType: string; size: number }>Note:
HypercertOperationsImpl:321already handles this with runtime type checking.packages/sdk-core/src/repository/HypercertOperationsImpl.ts (1)
2006-2013:⚠️ Potential issue | 🟡 MinorFix malformed JSDoc
@exampletag.There’s an extra
*that breaks the doc block rendering.📝 Suggested fix
- * * `@example` + * `@example`packages/sdk-core/src/repository/BlobOperationsImpl.ts (2)
160-176:⚠️ Potential issue | 🟡 MinorGuard against missing
blobin SDS upload responses.The method currently returns
result.blobwithout checking if it exists, which could returnundefineddespite thePromise<BlobRef>return type contract. This happens if the SDS endpoint returns anokresponse with an unexpected payload structure.🛡️ Suggested guard
- const result = (await response.json()) - - return result.blob + const result = await response.json(); + if (!result?.blob) { + throw new NetworkError("SDS blob upload failed: missing blob in response"); + } + return result.blob;
113-133:⚠️ Potential issue | 🟡 MinorUpdate upload() documentation to match the BlobRef return type.
The examples incorrectly show destructuring
{ ref, mimeType, size }, butupload()returns aBlobReffrom@atproto/api. The documentation must be updated to reflect the actual return type. Additionally, the suggested use ofblobRef.ref.toString()in the diff may not match the BlobRef API; verify the correct property or method to access the CID (e.g.,blobRef.ipld()as used elsewhere in the codebase).packages/sdk-core/src/repository/interfaces.ts (1)
618-643:⚠️ Potential issue | 🟡 MinorStale doc block references removed methods.
The examples at lines 622–642 still reference
repo.profile.get()andrepo.profile.update(), which no longer exist. These will confuse consumers. Update or remove this block since each method now has its own doc examples.Proposed fix — remove the stale block
-/** - * Profile operations for user profile management. - * - * `@example` - * ```typescript - * // Get profile - * const profile = await repo.profile.get(); - * console.log(profile.displayName); - * - * // Update profile - * await repo.profile.update({ - * displayName: "New Name", - * description: "Updated bio", - * }); - * - * // Update avatar - * await repo.profile.update({ - * avatar: new Blob([avatarData], { type: "image/png" }), - * }); - * - * // Clear a field by passing null - * await repo.profile.update({ - * website: null, // Removes website - * }); - * ``` - */ +/** + * Profile operations for Bluesky and Certified profile management. + * + * Supports dual-profile workflows: Bluesky profiles (app.bsky.actor.profile) + * and Certified profiles (app.certified.actor.profile). + */
🤖 Fix all issues with AI agents
In `@packages/sdk-core/src/repository/interfaces.ts`:
- Around line 672-675: UpdateBskyProfileParams is incorrectly derived from
AppBskyActorDefs.ProfileViewDetailed (a read-only view) — change it to be
Partial<CreateBskyProfileParams> so it represents selective updates to the
record; replace the current type alias for UpdateBskyProfileParams with a type
based on Partial<CreateBskyProfileParams> and still allow avatar?: Blob | null
and banner?: Blob | null (i.e., derive from CreateBskyProfileParams which itself
is based on AppBskyActorProfile.Record).
In `@packages/sdk-core/src/repository/ProfileOperationsImpl.ts`:
- Around line 94-117: The imageToUrl currently treats only http(s) URIs as
passthrough and converts everything else to a blob URL; update imageToUrl (which
uses extractCidFromImage and getBlobUrl and fields pdsUrl/repoDid) to return any
valid URI scheme unchanged (e.g., detect presence of a scheme like "scheme://",
or more robustly match /^[a-z][a-z0-9+.-]*:\/\//i) instead of only http/https,
and only call getBlobUrl when the result is truly a bare CID (no scheme
present); keep the existing error throw when extractCidFromImage returns falsy.
🧹 Nitpick comments (3)
packages/sdk-core/src/lib/blob-url.ts (1)
49-62: Consider URL-encoding query parameters.While DIDs and CIDs are typically URL-safe in practice,
didandcidare inserted into the query string withoutencodeURIComponent. This could break with unusual inputs.♻️ Suggested fix
const normalizedPdsUrl = pdsUrl.replace(/\/$/, ""); - return `${normalizedPdsUrl}/xrpc/com.atproto.sync.getBlob?did=${did}&cid=${cid}`; + return `${normalizedPdsUrl}/xrpc/com.atproto.sync.getBlob?did=${encodeURIComponent(did)}&cid=${encodeURIComponent(cid)}`;packages/sdk-react/src/hooks/useProfile.ts (1)
105-111: Addpronounsfield toProfileUpdateinterface and include it in theupdateCertifiedProfilecall.The underlying
updateCertifiedProfileAPI fully supports thepronounsfield (viaAppCertifiedActorProfile.Main), but this is not exposed through the React hook'sProfileUpdatetype or the update mutation. To enable users to update pronouns through the hook, addpronouns?: string | nullto theProfileUpdateinterface inpackages/sdk-react/src/types.tsand pass it in the mutation call at lines 105-111.packages/sdk-core/src/repository/interfaces.ts (1)
687-690:UpdateCertifiedProfileParamsshould derive fromCreateCertifiedProfileParamsfor consistency.Per the coding guidelines,
UpdateXxxParamsshould bePartial<CreateXxxParams>. WhileAppCertifiedActorProfile.Mainis the record type (so the practical gap is smaller than the Bsky case), deriving fromCreateCertifiedProfileParamskeeps the two in sync and avoids drift.Proposed fix
-export type UpdateCertifiedProfileParams = OverrideProperties< - Nullable<Except<AppCertifiedActorProfile.Main, "$type" | "createdAt">>, - { avatar?: Blob | null; banner?: Blob | null } ->; +export type UpdateCertifiedProfileParams = Partial<CreateCertifiedProfileParams> & { + avatar?: Blob | null; + banner?: Blob | null; +};As per coding guidelines: "Define UpdateXxxParams as Partial for selective record updates."
Kzoeps
force-pushed
the
profile
branch
6 times, most recently
from
8b93cec to
74bbf7c
Compare
This has been done because blobToJsonRef is very flaky. And is an unnecessary step since we can directly use the blobs returned from AtProto instead. This makes the code easier to maintain and also we dont have to manually construct json of blobs which are not only inaccurate but are also wrong in some cases;
…xport Remove deprecated Hypercert*Profile* type aliases in favor of Certified*Profile* types: - Remove HypercertProfile (use CertifiedProfileRecord) - Remove CreateHypercertProfileParams (use CreateCertifiedProfileParams) - Remove UpdateHypercertProfileParams (use UpdateCertifiedProfileParams) - Remove HypercertProfileParams union type - Remove CreateProfileParams alias Remove JsonBlobRef type export: - JsonBlobRef was too "snowflaky" - required unnecessary BlobRef to JSON conversion - BlobRef instances work perfectly fine for all use cases - Removed flaky test patterns with mock JSON objects - Users can import from @atproto/lexicon if needed Additional changes: - Update exports in src/index.ts and src/types.ts - Remove blobToJsonRef() method from HypercertOperationsImpl - Fix typo in ProfileOperationsImpl comment - Update changeset with migration guide and rationale BREAKING CHANGE: Users must update imports to use new type names
2 participants
Summary
app.certified.profile) instead of Bluesky actor profilepronounsfield support (max 20 graphemes)HypercertImageRecordfor avatar/bannerChanges
Core SDK (
sdk-core)app.certified.profilecollection with proper image wrappingHypercertProfile,CreateHypercertProfileParams,UpdateHypercertProfileParamstypesProfileOperationsandProfileParamsto includepronounsand useHypercertImageRecordReact SDK (
sdk-react)Profileinterface to useHypercertImageRecordfor avatar/bannerBreaking Changes
HypercertImageRecordobjects instead of stringsapp.certified.profilecollection instead ofapp.bsky.actor.profileSummary by CodeRabbit
New Features
Improvements
Tests
Breaking Changes