refactor: write literal property values directly + decode both old and new shapes#604
Open
HexaField wants to merge 5 commits into
Open
refactor: write literal property values directly + decode both old and new shapes#604HexaField wants to merge 5 commits into
HexaField wants to merge 5 commits into
Conversation
After the ad4m executor stops emitting signed-expression envelopes for literal-language property writes, link targets storing string/number/ boolean values come back from Literal.fromUrl().get() as the raw value rather than as an envelope object with a .data field. Existing .get().data sites would start returning undefined. Introduce unwrapLiteralValue in flux-utils which decodes literal: URLs and returns the inner value for both plain and signed-envelope shapes. Update the five sites in flux-utils + flux-api that previously assumed the envelope shape.
✅ Deploy Preview for fluxsocial-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reached
More reviews will be available in 53 minutes and 57 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR introduces a centralized ChangesLiteral value decoding refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Cross-repo branch linking means flux CI runs the same-name ad4m branch of @coasys/ad4m, which drops the .data extraction from parseLit. Caller- visible behaviour: literal:json: objects round-trip as JSON.stringify, leaving envelope-vs-plain shape decisions to unwrapLiteralValue.
createNeighbourhoodMeta, createLiteralLinks, and createLiteralObject previously routed property values through client.expression.create(value, 'literal'), which signs the value into a literal:json:<envelope> URI on the executor — embedding author/timestamp/proof in the target IRI and duplicating provenance that already lives on the link reifier. Encode the value client-side with Literal.from(value).toUrl() instead. Targets become deterministic plain literal URIs; the reifier remains the single source of truth for who wrote the link and when. createLiteralObject now uses the deterministic parent URL as a stable entity identity — two calls with the same parent.target value resolve to the same parent IRI, which is the intended semantics for value-keyed entities like web links.
HexaField
changed the title
11 tasks
HexaField
changed the title
![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
🧹 Nitpick comments (1)
packages/utils/src/createNeighbourhoodMeta.ts (1)
45-45: 💤 Low valueConsider using strict equality.
The loose equality check (
!=) works but strict equality (!==) is more idiomatic in TypeScript.Suggested change
- if (description != '') { + if (description !== '') {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/utils/src/createNeighbourhoodMeta.ts` at line 45, Replace the loose inequality check in createNeighbourhoodMeta.ts with a strict one: locate the conditional that reads if (description != '') and change it to use strict comparison (===/!==) so it becomes if (description !== '') to follow TypeScript best practices; ensure any equivalent checks in the same function (or nearby conditionals) are updated similarly to maintain consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/api/src/utils/unwrapLiteralValue.test.ts`:
- Around line 31-40: Add a test that verifies unwrapLiteralValue does NOT unwrap
a partial "envelope" missing timestamp: create an object like { data: 'value',
author: 'did:key:…', proof: { signature: 'sig' } }, convert it to a literal URL
via Literal.from(...).toUrl(), call unwrapLiteralValue(url) and assert the
result strictly equals the original object (not the inner data). Place this
alongside the existing envelope tests and name it something like "does not
unwrap objects that have data/author/proof but lack timestamp" to ensure
unwrapLiteralValue rejects incomplete envelope shapes.
In `@packages/utils/src/linkHelpers.ts`:
- Around line 45-53: The function createLiteralLinks currently marked async
contains no asynchronous code; remove the async keyword from its declaration so
it is a synchronous function, keeping the parameters (client: Ad4mClient,
source: string, map: PredicateMap) and its existing implementation that uses
Literal.from(...).toUrl() and returns new Link({...}), ensuring any callers
expect a plain array return rather than a Promise.
In `@packages/utils/src/unwrapLiteralValue.ts`:
- Around line 24-31: The envelope detection in unwrapLiteralValue (check on
parsed object) currently verifies 'data', 'author', and 'proof' but omits
'timestamp', causing false positives; update the conditional in the
parsed/object branch (the block that returns parsed.data) to also require
'timestamp' in parsed so the envelope shape matches { author, timestamp, data,
proof } before unwrapping.
---
Nitpick comments:
In `@packages/utils/src/createNeighbourhoodMeta.ts`:
- Line 45: Replace the loose inequality check in createNeighbourhoodMeta.ts with
a strict one: locate the conditional that reads if (description != '') and
change it to use strict comparison (===/!==) so it becomes if (description !==
'') to follow TypeScript best practices; ensure any equivalent checks in the
same function (or nearby conditionals) are updated similarly to maintain
consistency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b1101ad3-b6fb-4f76-aae2-c3f1302f7759
📒 Files selected for processing (9)
packages/api/src/getAgentWebLinks.tspackages/api/src/utils/parseLit.test.tspackages/api/src/utils/unwrapLiteralValue.test.tspackages/utils/src/createNeighbourhoodMeta.tspackages/utils/src/getNeighbourhoodMeta.tspackages/utils/src/index.tspackages/utils/src/linkHelpers.tspackages/utils/src/prologHelpers.tspackages/utils/src/unwrapLiteralValue.ts
Comment on lines
+31
to
+40
| it('unwraps signed-envelope literal:json: targets to their inner data', () => { | ||
| const envelope = { | ||
| author: 'did:key:z6MkExample', | ||
| timestamp: '2024-01-01T00:00:00Z', | ||
| data: 'wrapped value', | ||
| proof: { key: '#z6MkExample', signature: 'sig', valid: true }, | ||
| }; | ||
| const url = Literal.from(envelope).toUrl(); | ||
| expect(unwrapLiteralValue(url)).toBe('wrapped value'); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add test case for partial envelope shape.
The envelope unwrapping test includes all four fields (author, timestamp, data, proof). Consider adding a test case for a JSON object that has data, author, and proof but lacks timestamp, to verify it is not unwrapped (because it's not a valid envelope). This would catch the bug flagged in unwrapLiteralValue.ts.
🧪 Suggested test case
it('does not unwrap objects that have data/author/proof but lack timestamp', () => {
// Not a valid envelope — missing 'timestamp'.
const notAnEnvelope = { data: 'value', author: 'did:key:z6Mk', proof: { signature: 'sig' } };
const url = Literal.from(notAnEnvelope).toUrl();
expect(unwrapLiteralValue(url)).toEqual(notAnEnvelope);
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/api/src/utils/unwrapLiteralValue.test.ts` around lines 31 - 40, Add
a test that verifies unwrapLiteralValue does NOT unwrap a partial "envelope"
missing timestamp: create an object like { data: 'value', author: 'did:key:…',
proof: { signature: 'sig' } }, convert it to a literal URL via
Literal.from(...).toUrl(), call unwrapLiteralValue(url) and assert the result
strictly equals the original object (not the inner data). Place this alongside
the existing envelope tests and name it something like "does not unwrap objects
that have data/author/proof but lack timestamp" to ensure unwrapLiteralValue
rejects incomplete envelope shapes.
Comment on lines
+45
to
53
| export async function createLiteralLinks(_client: Ad4mClient, source: string, map: PredicateMap) { | ||
| // Values land as deterministic `literal:string:` targets — the link reifier | ||
| // carries the author/timestamp/proof for the write itself. | ||
| return Object.keys(map) | ||
| .filter((predicate) => typeof map[predicate] === 'string') | ||
| .map((predicate) => { | ||
| const target = Literal.from(map[predicate] as string).toUrl(); | ||
| return new Link({ source, predicate, target }); | ||
| }); |
There was a problem hiding this comment.
Remove async from createLiteralLinks signature.
The function no longer contains any asynchronous operations (no await, no Promise). It should be declared as a synchronous function.
♻️ Proposed fix
-export async function createLiteralLinks(_client: Ad4mClient, source: string, map: PredicateMap) {
+export function createLiteralLinks(_client: Ad4mClient, source: string, map: PredicateMap) {
// Values land as deterministic `literal:string:` targets — the link reifier
// carries the author/timestamp/proof for the write itself.
return Object.keys(map)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/utils/src/linkHelpers.ts` around lines 45 - 53, The function
createLiteralLinks currently marked async contains no asynchronous code; remove
the async keyword from its declaration so it is a synchronous function, keeping
the parameters (client: Ad4mClient, source: string, map: PredicateMap) and its
existing implementation that uses Literal.from(...).toUrl() and returns new
Link({...}), ensuring any callers expect a plain array return rather than a
Promise.
Comment on lines
+24
to
+31
| if ( | ||
| parsed && | ||
| typeof parsed === 'object' && | ||
| 'data' in parsed && | ||
| 'author' in parsed && | ||
| 'proof' in parsed | ||
| ) { | ||
| return parsed.data; |
There was a problem hiding this comment.
Add 'timestamp' to envelope detection.
The envelope detection checks for data, author, and proof, but the docstring (line 10) states the envelope shape is { author, timestamp, data, proof }. Without checking for timestamp, a user JSON object that happens to have {data, author, proof} fields would be incorrectly unwrapped.
🛡️ Proposed fix
if (
parsed &&
typeof parsed === 'object' &&
'data' in parsed &&
'author' in parsed &&
+ 'timestamp' in parsed &&
'proof' in parsed
) {
return parsed.data;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/utils/src/unwrapLiteralValue.ts` around lines 24 - 31, The envelope
detection in unwrapLiteralValue (check on parsed object) currently verifies
'data', 'author', and 'proof' but omits 'timestamp', causing false positives;
update the conditional in the parsed/object branch (the block that returns
parsed.data) to also require 'timestamp' in parsed so the envelope shape matches
{ author, timestamp, data, proof } before unwrapping.
… unwrap sites Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
Companion to coasys/ad4m#837. After that PR lands, the ad4m executor stops embedding signed-expression envelopes (
{author, timestamp, data, proof}) in link targets when properties are written through the literal language — per-link author/timestamp/proof lives only on the RDF reifier going forward. This PR brings flux into line on both sides of that contract: the write helpers that put the envelope into the target in the first place, and the read sites that pulled values out of the envelope shape via.get().data.Writes — stop embedding provenance in targets
Five flux call sites previously routed property values through
client.expression.create(value, 'literal'). The executor's literal language signs the value into aliteral:json:<envelope>URL, so the target carried{author, timestamp, data, proof}duplicating provenance the reifier already holds. After this PR every property target is encoded client-side as a deterministic plainliteral:string:/:number:/:boolean:/:json:URL viaLiteral.from(value).toUrl(). No round-trip to the executor, no embedded provenance, no envelope.packages/utils/src/createNeighbourhoodMeta.tsname,createdAt,descriptionpackages/utils/src/linkHelpers.ts(createLiteralLinks)createProfile,updateProfilepackages/utils/src/linkHelpers.ts(createLiteralObject)createAgentWebLinkcreateLiteralObjectgains a useful side effect: the deterministic parent URL doubles as a stable entity identity, so two calls with the sameparent.targetvalue resolve to the same IRI — which is what callers likecreateAgentWebLinkactually want (same URL → same WebLink entity).grep -rn "expression\.create\([^)]*['\"]literal['\"]\)"againstpackages/returns no hits after this PR. The only remainingexpression.create("…", "literal")callers across the ecosystem are dapp entanglement proofs and agent expressions in the ad4m repo, both of which intentionally produce addressable signed expressions (a separate concern from link-property storage).Reads — handle both old and new target shapes
New
unwrapLiteralValue(target)helper in@coasys/flux-utilsthat decodesliteral:URLs and returns the inner value:expression.create(value, "literal")URL that callers feed in) → returns the inner.data.undefined; call sites substitute the raw target.Updated four reader sites that previously assumed the envelope shape:
packages/utils/src/linkHelpers.ts(mapLiteralLinks).get().dataon the json: branchpackages/utils/src/getNeighbourhoodMeta.ts(getMetaFromLinks).get().dataon NAME / DESCRIPTION / CREATED_AT predicatespackages/utils/src/prologHelpers.ts(resolveExpinside model property resolution).get().dataon literal: URLspackages/api/src/getAgentWebLinks.ts.get().dataon weblink parent targetPlus
packages/api/src/utils/parseLit.test.tsflipped from asserting.dataextraction to assertingJSON.stringify(obj)to matchparseLit's new behaviour in ad4m#837 (envelope semantics live inunwrapLiteralValuenow, not inparseLit).Test plan
pnpm exec vitest run packages/api/src/utils/—unwrapLiteralValue(7/7) +parseLit(6/7 locally against the published ad4m, 7/7 expected in CI against the linked branch)pnpm exec tsc --noEmit -p packages/utils/tsconfig.jsonclean for changed filespnpm exec tsc --noEmit -p packages/api/tsconfig.jsonclean for changed filescreateNeighbourhoodMeta+getMetaFromLinksCross-repo coupling
Flux CI resolves the same-name
refactor/literal-channel-v-separationbranch of@coasys/ad4mand tests against that build, not the currently-published version. The two PRs ship as one logical change.Summary by CodeRabbit
Refactor
Tests