Conversation
- @modeloptions → @model, @optional → @Property, @Collection → @hasmany - baseExpression → id, .update() → .save(), useModel → useLiveQuery - ensureSDNASubjectClass → Model.register() - New sections: relations, relation filtering, eager loading, parent-scoped queries, transactions, dirty tracking, model inheritance, JSON Schema - Fix getter syntax (remove invalid SELECT...FROM $parent pattern) - Add clarifying note on 'through' for computed properties - Update SHACL mapping table with new ad4m:// predicates
…s guides - batch-operations: promote transaction() as primary pattern, demote manual batch control to Advanced section - model-classes: move 'How It Works' earlier, condense JSON Schema section with resolution table, restore propertyMapping and array relation examples - surreal-queries: merge overlapping graph traversal sections, fix index documentation accuracy
- Add .author, .createdAt, .updatedAt built-in instance properties
- Document RelationSubQuery for filtering/sorting eager-loaded relations
- Add full PropertyOptions reference table (transform, local, initial, getter)
- Document default through value ('ad4m://has_child') on relation decorators
- Add @optional decorator mention
- Expand getter option with SurrealQL details and Base placeholder
- Document transform option with example
- Add introspection section (getModelMetadata, generateSDNA, generateSHACL)
Contributor
|
Important Review skippedToo many files! This PR contains 248 files, which is 98 over the limit of 150. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (248)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR refactors developer docs: replaces manual batch APIs with a transaction-centric pattern (Ad4mModel.transaction), renames/expands the hooks API (useModel → useLiveQuery) with new return shapes and modes, and updates model examples from ModelOptions/Collection/Optional to Model/Property/HasMany with registration/query pattern changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer Code
participant Ad4m as Ad4mModel
participant Pers as PerspectiveProxy
participant DB as Storage/DB
Dev->>Ad4m: Ad4mModel.transaction(perspective, async (tx) => { ... })
Ad4m->>Pers: createBatch() / create transactional context
Pers->>DB: stage writes (batch)
Dev->>Ad4m: perform model create/save/link operations via tx
Ad4m->>Pers: enqueue operations into batch
alt no error
Ad4m->>Pers: commitBatch()
Pers->>DB: persist batch -> committed
Ad4m->>Dev: transaction resolves
else error
Ad4m->>Pers: rollbackBatch()
Pers->>DB: discard staged changes
Ad4m->>Dev: transaction rejects (error)
end
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. ✨ 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 |
![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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs-src/pages/social-dna.mdx (1)
242-252:⚠️ Potential issue | 🟠 MajorThe table should use the generated SHACL predicate, not the decorator option name.
The documentation currently lists
ad4m://readOnly, but the generated SHACL shape emitsad4m://writablewith a boolean value. This mismatch misdescribes what's actually stored in the perspective—developers following this table would search for the wrong predicate name.Suggested fix
- | `ad4m://readOnly` | Property is read-only | Computed/immutable property | + | `ad4m://writable` | Whether the property can be written | `literal://boolean:false` for computed/immutable properties |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs-src/pages/social-dna.mdx` around lines 242 - 252, The table incorrectly lists the decorator option ad4m://readOnly; update the table entry to use the actual generated SHACL predicate ad4m://writable (which is emitted with a boolean value) and adjust the AD4M Meaning cell to say "Property writable flag (boolean)" or similar so it reflects that the shape stores a boolean writable predicate rather than a readOnly decorator; update the Example cell accordingly (e.g., `ad4m://writable: true/false`) and ensure any surrounding text that references ad4m://readOnly is changed to ad4m://writable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs-src/pages/developer-guides/batch-operations.mdx`:
- Around line 21-22: Update the docs to stop describing Ad4mModel.transaction()
as performing an explicit rollback: change wording to state that transaction()
creates a batch, runs the callback, and only commits on success, and that if the
callback throws the batch is not committed and no changes are persisted
(PerspectiveProxy does not expose a rollback API); search for references to
Ad4mModel.transaction and any mentions of "rollback" (including the sections
around lines referencing the earlier and later mentions) and replace those
sentences to say "changes are not committed" or "no commit occurs on error"
instead of claiming a rollback occurs.
In `@docs-src/pages/developer-guides/hooks.mdx`:
- Around line 267-270: The docs incorrectly reference a nonexistent
Model.register(perspective) API; replace that line to instruct users to call the
generated model class’s static register method (e.g.,
Recipe.register(perspective) or TestPost.register(perspective)) once at app
startup, and update the wording to explicitly state “Call
<ModelClass>.register(perspective) once at app startup — the hook does not
register models automatically” so readers know to use the concrete generated
class (Recipe, TestPost, etc.) rather than the `@Model` decorator.
In `@docs-src/pages/developer-guides/model-classes.mdx`:
- Around line 189-223: The snippet uses undeclared symbols Ad4mModel, Image, and
Category (while importing decorators like Model, Property, HasMany, HasOne,
BelongsToOne, BelongsToMany); fix by either adding explicit imports/definitions
for Ad4mModel, Image, and Category at the top of the snippet or mark the example
as a partial snippet (e.g., add a comment "partial: Ad4mModel, Image, Category
defined elsewhere") so readers know those types are defined elsewhere; ensure
references in the Recipe, Comment, and Author classes remain consistent with the
chosen approach.
- Around line 112-114: The docs incorrectly state that the framework supplies a
sentinel value for required properties; update the example and table to reflect
that `@Property`({ required: true }) mandates an explicit initial value (the
decorator implementation checks if (opts.required && !opts.initial) and will
throw), remove or replace references to the sentinel string
"literal://string:uninitialized", and change the sample property declaration
(the `@Property` usage at the example lines and the table description) to include
an explicit initial value so it matches the actual API contract (e.g., show
`@Property`({ through: "recipe://category", required: true, initial: "" }) or
equivalent).
In `@docs-src/pages/developer-guides/surreal-queries.mdx`:
- Around line 329-338: The participantsQuery is selecting and grouping by the
outer `author` instead of the nested item authors; update the query string in
`participantsQuery` to SELECT the nested traversal expression (`out->link[WHERE
predicate = 'ad4m://has_child'].author`) and also change the GROUP BY to use
that same nested expression (or remove GROUP BY if using SELECT VALUE to return
unique values) so the query returns the authors from the nested items rather
than the outer subgroup links.
In `@docs-src/pages/index.mdx`:
- Around line 273-275: The `@ReadOnly` decorator example is missing the required
through predicate for computed-property metadata/SHACL generation; update the
example so the ReadOnly declaration includes a through predicate (or
alternatively a prologGetter) alongside the existing getter expression (the
getter: 'count(->link[WHERE predicate = "flux://has_reaction" AND out.uri =
"flux://thumbsup"]) > 0'), e.g. add a through: 'flux://has_reaction' (or an
appropriate prologGetter) so the computed property definition is valid for
metadata/SHACL generation.
---
Outside diff comments:
In `@docs-src/pages/social-dna.mdx`:
- Around line 242-252: The table incorrectly lists the decorator option
ad4m://readOnly; update the table entry to use the actual generated SHACL
predicate ad4m://writable (which is emitted with a boolean value) and adjust the
AD4M Meaning cell to say "Property writable flag (boolean)" or similar so it
reflects that the shape stores a boolean writable predicate rather than a
readOnly decorator; update the Example cell accordingly (e.g., `ad4m://writable:
true/false`) and ensure any surrounding text that references ad4m://readOnly is
changed to ad4m://writable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e9ab3d0a-b29b-455e-bbb6-23e396491615
📒 Files selected for processing (6)
docs-src/pages/developer-guides/batch-operations.mdxdocs-src/pages/developer-guides/hooks.mdxdocs-src/pages/developer-guides/model-classes.mdxdocs-src/pages/developer-guides/surreal-queries.mdxdocs-src/pages/index.mdxdocs-src/pages/social-dna.mdx
Comment on lines
+112
to
+114
| // Required property — a placeholder link is created on save | ||
| @Property({ through: "recipe://category", required: true }) | ||
| category: string = ""; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the decorators file
find . -type f -name "decorators.ts" | head -20Repository: coasys/ad4m
Length of output: 85
🏁 Script executed:
# Check the markdown file exists and read relevant sections
ls -la docs-src/pages/developer-guides/model-classes.mdx 2>/dev/null && wc -l docs-src/pages/developer-guides/model-classes.mdxRepository: coasys/ad4m
Length of output: 209
🏁 Script executed:
# Read the specific lines mentioned in the review (112-114, 140-143)
sed -n '105,150p' docs-src/pages/developer-guides/model-classes.mdxRepository: coasys/ad4m
Length of output: 2582
🏁 Script executed:
# Read the decorators.ts file around lines 310-336
sed -n '300,350p' core/src/model/decorators.tsRepository: coasys/ad4m
Length of output: 1895
🏁 Script executed:
# Get broader context to understand the property decorator logic
sed -n '280,370p' core/src/model/decorators.tsRepository: coasys/ad4m
Length of output: 3192
🏁 Script executed:
sed -n '185,230p' docs-src/pages/developer-guides/model-classes.mdxRepository: coasys/ad4m
Length of output: 1474
required: true requires an explicit initial value — the decorator throws at class definition if omitted.
The decorator implementation explicitly checks if (opts.required && !opts.initial) and throws an error. The documentation incorrectly claims the framework inserts a sentinel value ("literal://string:uninitialized"). The example code at lines 112–114 and the table description at lines 140–143 must be corrected to reflect the actual API contract.
Suggested fix
- `@Property`({ through: "recipe://category", required: true })
+ `@Property`({ through: "recipe://category", required: true, initial: "recipe://uncategorized" })
category: string = "";- | `initial` | `any` | — | Default value written on creation. When `required: true` with no `initial`, the framework uses a sentinel value (`"literal://string:uninitialized"`) |
+ | `initial` | `any` | — | Default value written on creation. Required properties must include an explicit `initial` value. |Also applies to: 140–143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs-src/pages/developer-guides/model-classes.mdx` around lines 112 - 114,
The docs incorrectly state that the framework supplies a sentinel value for
required properties; update the example and table to reflect that `@Property`({
required: true }) mandates an explicit initial value (the decorator
implementation checks if (opts.required && !opts.initial) and will throw),
remove or replace references to the sentinel string
"literal://string:uninitialized", and change the sample property declaration
(the `@Property` usage at the example lines and the table description) to include
an explicit initial value so it matches the actual API contract (e.g., show
`@Property`({ through: "recipe://category", required: true, initial: "" }) or
equivalent).
Contributor
Author
There was a problem hiding this comment.
The docs are correct on this one:
The public Property decorator (core/src/model/decorators.ts lines 1033–1038) wraps the internal applyPropertyMetadata and fills in the sentinel before the check runs:
export function Property(opts: PropertyOptions) {
const required = opts.required ?? false;
return applyPropertyMetadata({
...opts,
required,
initial: opts.initial ?? (required ? "literal://string:uninitialized" : undefined),
});
}So calling @Property({ through: "recipe://category", required: true }) without an explicit initial:
Propertysetsinitialto"literal://string:uninitialized"automaticallyapplyPropertyMetadatathen seesopts.initialis truthy → theif (opts.required && !opts.initial)check passes → no throw
The applyPropertyMetadata guard at decorators.ts line 315 only catches direct internal callers that bypass the Property wrapper. The public API never throws here.
The docs at model-classes.mdx line 113 (@Property({ through: "recipe://category", required: true })) and the table at line 143 (describing the sentinel behavior) both correctly describe this behavior.
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the Property decorator implementation at lines 1033-1038
sed -n '1025,1050p' core/src/model/decorators.ts
Length of output: 947
@jhweir, you're right — I apologize for the incorrect review. The script output confirms exactly what you described:
export function Property(opts: PropertyOptions) {
const required = opts.required ?? false;
return applyPropertyMetadata({
...opts,
required,
readOnly: opts.readOnly ?? false,
resolveLanguage: opts.resolveLanguage ?? "literal",
initial: opts.initial ?? (required ? "literal://string:uninitialized" : undefined),
});
}The public Property wrapper injects the "literal://string:uninitialized" sentinel before applyPropertyMetadata is ever called, so the internal guard at line 315 is never triggered for public API consumers. The docs at line 113 and the table description at line 143 are both correct.
✏️ Learnings added
Learnt from: jhweir
Repo: coasys/ad4m PR: 740
File: docs-src/pages/developer-guides/model-classes.mdx:112-114
Timestamp: 2026-03-10T22:34:39.792Z
Learning: In `core/src/model/decorators.ts`, the public `Property` decorator (around line 1029) automatically injects a sentinel value `"literal://string:uninitialized"` as the `initial` when `required: true` and no `initial` is explicitly provided, before delegating to `applyPropertyMetadata`. The internal `applyPropertyMetadata` guard (`if (opts.required && !opts.initial)`) only fires for direct internal callers that bypass the `Property` wrapper. Therefore, calling `Property({ through: "...", required: true })` without an explicit `initial` is valid and does NOT throw — the sentinel is auto-supplied by the public wrapper. Documentation describing this sentinel behavior for `required: true` properties is correct.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: lucksus
Repo: coasys/ad4m PR: 644
File: core/src/model/Ad4mModel.ts:814-822
Timestamp: 2026-01-15T17:30:01.739Z
Learning: In core/src/model/Ad4mModel.ts, when parsing literal URLs with Literal.fromUrl(value).get() in the getData() method's SurrealDB fallback path (around lines 814-822), the code correctly checks for a `data` property (`parsed.data !== undefined ? parsed.data : parsed`) to handle two cases: Expression objects (which have a data property that should be extracted when we want just the value) and other literal types (which should be used as-is). This conditional is intentional and correct *in this specific context* where we want to reduce Expressions to their data values. Note that this is not a universal pattern—Expression shapes are preserved with full metadata in other contexts where that's needed.
…predicate - batch-operations: remove inaccurate rollback references (transaction has no rollback API) - hooks: fix Model.register() → <ModelClass>.register(perspective) with concrete class example - model-classes: add Ad4mModel import and inline Image/Category definitions to relations snippet - surreal-queries: fix participantsQuery to SELECT nested item authors instead of outer link author - index: add missing through predicate to @readonly getter example
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs-src/pages/developer-guides/hooks.mdx (1)
275-279:⚠️ Potential issue | 🟡 MinorAvoid
!dataas the generic loading guard.
useLiveQuerycollection mode always returns an array ([]initially), so this pattern never renders a loading state there. Useloadingexplicitly, or show separate examples for collection vs. instance-mode hooks.✏️ Suggested doc tweak
- if (!data) return <LoadingComponent />; + if (loading) return <LoadingComponent />; + // For useLiveQuery collections: + // if (loading && data.length === 0) return <LoadingComponent />;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs-src/pages/developer-guides/hooks.mdx` around lines 275 - 279, The example using "if (error) ..." / "if (!data) ..." is misleading for useLiveQuery because collection-mode returns [] initially; update the docs to check the hook's loading state explicitly (e.g., use the returned loading boolean) or show two examples: one for collection-mode (check data.length === 0 or loading) and one for instance-mode (check !data). Mention the hook name useLiveQuery and example components ErrorComponent and LoadingComponent so readers know where to apply the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs-src/pages/developer-guides/batch-operations.mdx`:
- Around line 185-187: Update the doc text to remove the claim that
Ad4mModel.transaction handles failed-batch cleanup and explicitly state its
current behavior: transaction() calls createBatch(), invokes the callback, then
calls commitBatch(), but if the callback throws the batch remains uncommitted
(no automatic rollback/cleanup). Reference Ad4mModel.transaction and the
createBatch/commitBatch flow so readers know they must handle try/catch/cleanup
for errors when using manual batches or transaction() until the implementation
adds error-path cleanup.
In `@docs-src/pages/developer-guides/model-classes.mdx`:
- Line 393: The phrasing "must be set on creation" for `sh:minCount 1` is too
strict; update the line to say "must exist on creation" and add a short
parenthetical note that when `@Property({ required: true })` is used without an
explicit `initial`, the public wrapper auto-supplies the sentinel value
`literal://string:uninitialized` (i.e., a placeholder is inserted before
creation), so callers don't always need to populate the field themselves.
In `@docs-src/pages/developer-guides/surreal-queries.mdx`:
- Around line 181-186: The current example collects friendsOfFriends via
perspective.querySurrealDB where the selected friend_of_friend is an array per
row, so using [...new Set(friendsOfFriends.map(f => f.friend_of_friend))]
deduplicates arrays not inner URIs; update the docs to flatten the results first
(e.g., use flatMap or map(...).flat()) on the friendsOfFriends result before
constructing a Set so you deduplicate individual URIs produced by querySurrealDB
for friend_of_friend.
---
Outside diff comments:
In `@docs-src/pages/developer-guides/hooks.mdx`:
- Around line 275-279: The example using "if (error) ..." / "if (!data) ..." is
misleading for useLiveQuery because collection-mode returns [] initially; update
the docs to check the hook's loading state explicitly (e.g., use the returned
loading boolean) or show two examples: one for collection-mode (check
data.length === 0 or loading) and one for instance-mode (check !data). Mention
the hook name useLiveQuery and example components ErrorComponent and
LoadingComponent so readers know where to apply the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a432662c-aa93-4045-aa70-c9e3457a9858
📒 Files selected for processing (5)
docs-src/pages/developer-guides/batch-operations.mdxdocs-src/pages/developer-guides/hooks.mdxdocs-src/pages/developer-guides/model-classes.mdxdocs-src/pages/developer-guides/surreal-queries.mdxdocs-src/pages/index.mdx
Comment on lines
+185
to
+187
| 3. **Error Handling**: With `transaction()`, errors thrown inside the callback automatically prevent commit. With manual batches, wrap operations in try/catch. | ||
|
|
||
| 4. **Resource Management**: Don't keep batches open longer than necessary. With `transaction()` this is handled for you. |
Contributor
There was a problem hiding this comment.
Don’t claim transaction() handles failed-batch cleanup.
The current Ad4mModel.transaction() implementation only does createBatch() → callback → commitBatch(). If the callback throws, the batch is left uncommitted; there is no error-path cleanup in core/src/model/Ad4mModel.ts:3590-3598. Line 187 should document that limitation instead of implying lifecycle management is automatic.
✏️ Suggested doc tweak
-4. **Resource Management**: Don't keep batches open longer than necessary. With `transaction()` this is handled for you.
+4. **Resource Management**: Don't keep batches open longer than necessary. `transaction()` keeps the happy path short, but its current implementation does not clean up a batch when the callback throws.📝 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
| 3. **Error Handling**: With `transaction()`, errors thrown inside the callback automatically prevent commit. With manual batches, wrap operations in try/catch. | |
| 4. **Resource Management**: Don't keep batches open longer than necessary. With `transaction()` this is handled for you. | |
| 3. **Error Handling**: With `transaction()`, errors thrown inside the callback automatically prevent commit. With manual batches, wrap operations in try/catch. | |
| 4. **Resource Management**: Don't keep batches open longer than necessary. `transaction()` keeps the happy path short, but its current implementation does not clean up a batch when the callback throws. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs-src/pages/developer-guides/batch-operations.mdx` around lines 185 - 187,
Update the doc text to remove the claim that Ad4mModel.transaction handles
failed-batch cleanup and explicitly state its current behavior: transaction()
calls createBatch(), invokes the callback, then calls commitBatch(), but if the
callback throws the batch remains uncommitted (no automatic rollback/cleanup).
Reference Ad4mModel.transaction and the createBatch/commitBatch flow so readers
know they must handle try/catch/cleanup for errors when using manual batches or
transaction() until the implementation adds error-path cleanup.
Contributor
Author
There was a problem hiding this comment.
No fix needed — the docs already accurately reflect the current behavior after the rollback fixes from commit 1da33767.
Current doc state
- Line 181: "if the callback throws, the batch is not committed and no changes are persisted" — accurate.
transaction()callsfn()beforecommitBatch(), so iffnthrows,commitBatchis never reached. - Line 185: "errors thrown inside the callback automatically prevent commit" — accurate. Same reasoning: the exception propagates before
commitBatchruns. - Line 219: The migration example shows
transaction()without try/catch — accurate. The batch simply won't commit iffnthrows; the error propagates to the caller.
Analysis
The finding asks us to "explicitly state... no automatic rollback/cleanup" and "they must handle try/catch/cleanup for errors." But looking at the source code (Ad4mModel.ts lines 3594–3598):
static async transaction<R = void>(
perspective: PerspectiveProxy,
fn: (tx: { batchId: string }) => Promise<R>,
): Promise<R> {
const batchId = await perspective.createBatch();
const result = await fn({ batchId });
await perspective.commitBatch(batchId);
return result;
}There is nothing to clean up — an uncommitted batch is just an in-memory buffer that gets garbage-collected. There is no cancelBatch() or deleteBatch() API. The batch simply evaporates when it's never committed.
The docs correctly say "the batch is not committed and no changes are persisted" — that is the cleanup behavior. Adding warnings about manual cleanup would be misleading since there's nothing to manually clean up.
Contributor
There was a problem hiding this comment.
@jhweir, you're absolutely right — I apologize for the incorrect finding. The key insight I missed is that uncommitted batches in AD4M are purely in-memory buffers with no persistent state to clean up. Since there's no cancelBatch()/deleteBatch() API, a thrown exception leaving the batch uncommitted is the cleanup — it simply gets garbage-collected. The existing doc language ("the batch is not committed and no changes are persisted") accurately describes the full lifecycle. No changes needed here.
- model-classes: update sh:minCount 1 description to 'must exist on creation' with sentinel note - surreal-queries: fix friendsOfFriends dedup to use flatMap (traversal returns arrays per row)
Contributor
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 (1)
docs-src/pages/developer-guides/surreal-queries.mdx (1)
557-575:⚠️ Potential issue | 🟠 MajorCorrect the traversal direction in both graph examples at lines 557-575.
When filtering with
WHERE out.uri = 'user://alice' AND predicate = 'follows',in.urirepresents the followers. The first query incorrectly labels followers asPost, and the second query traverses fromout(Alice) instead ofin(followers). Both should traverse fromin->linkto query follower activities.Suggested fix
-// Use graph traversal to find posts liked by Alice's followers (2-hop) -const results = await perspective.querySurrealDB(` - SELECT - in.uri AS Post, - out.uri AS User - FROM link - WHERE out.uri = 'user://alice' - AND predicate = 'follows' - AND in->link[WHERE predicate = 'likes'].out.uri IS NOT NONE -`); +// Use graph traversal to find posts liked by Alice's followers (2-hop) +const likedPosts = await perspective.querySurrealDB(` + SELECT + in.uri AS follower, + in->link[WHERE predicate = 'likes'].out.uri AS liked_posts + FROM link + WHERE out.uri = 'user://alice' + AND predicate = 'follows' +`); -// Alternative: Get all liked posts by traversing from followers -const likedPosts = await perspective.querySurrealDB(` - SELECT - out.uri AS follower, - out->link[WHERE predicate = 'likes'].out.uri AS liked_posts - FROM link - WHERE out.uri = 'user://alice' AND predicate = 'follows' -`); +// Alternative: flatten the liked post URIs in JavaScript if needed +// [...new Set(likedPosts.flatMap(r => r.liked_posts))]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs-src/pages/developer-guides/surreal-queries.mdx` around lines 557 - 575, Both query examples passed to perspective.querySurrealDB use the wrong traversal direction: when filtering WHERE out.uri = 'user://alice' AND predicate = 'follows', the followers are represented by in, so change the selections and traversals to start from in. Update the first query to select in.uri as the follower/user and traverse in->link[WHERE predicate = 'likes'].out.uri for the posts (instead of in.uri AS Post), and update the second query to SELECT in.uri AS follower and in->link[WHERE predicate = 'likes'].out.uri AS liked_posts (traversing from in, not out). Ensure both query strings passed to perspective.querySurrealDB reflect these in->link traversals.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs-src/pages/developer-guides/model-classes.mdx`:
- Around line 603-607: The nested include references an author relation on
Comment that doesn't exist; update the Comment model to declare the relation
(e.g., add a `@BelongsToOne`(() => Author) relation property named author and the
corresponding foreign key property) so the nested include in Recipe.query (the
comments include block) resolves, or alternatively change the nested include to
reference an existing Comment relation; modify the Comment class (the class
currently only defining body) to add the author relation and type Author so the
include { comments: { include: { author: true } } } is valid.
- Around line 231-233: The relation named "author" conflicts with the built-in
AD4M author field; rename the `@BelongsToOne` relation and its property from
author to a non-conflicting identifier (e.g., creator or recipeAuthor) in the
model declaration (`@BelongsToOne`(() => Author, { through: "recipe://author" })
creator: string = ""), and update all corresponding eager-load examples and
references that access the relation (the eager include/relationships shown in
the examples that currently reference author) to use the new name so the model
property and example usage stay consistent.
---
Outside diff comments:
In `@docs-src/pages/developer-guides/surreal-queries.mdx`:
- Around line 557-575: Both query examples passed to perspective.querySurrealDB
use the wrong traversal direction: when filtering WHERE out.uri = 'user://alice'
AND predicate = 'follows', the followers are represented by in, so change the
selections and traversals to start from in. Update the first query to select
in.uri as the follower/user and traverse in->link[WHERE predicate =
'likes'].out.uri for the posts (instead of in.uri AS Post), and update the
second query to SELECT in.uri AS follower and in->link[WHERE predicate =
'likes'].out.uri AS liked_posts (traversing from in, not out). Ensure both query
strings passed to perspective.querySurrealDB reflect these in->link traversals.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5a16e0f3-5383-49ec-be27-cf8c6330edfa
📒 Files selected for processing (2)
docs-src/pages/developer-guides/model-classes.mdxdocs-src/pages/developer-guides/surreal-queries.mdx
- Author class → Chef (chef://name predicate) - Recipe relation: creator → chef (recipe://chef) - Comment relation: writer → chef (comment://chef) - Updated nested include examples to reference chef
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs-src/pages/developer-guides/model-classes.mdx`:
- Around line 118-122: The ReadOnly decorator example for averageRating is
inconsistent: update the getter to use the Base placeholder as explained later
(use Base as the current instance node). Replace the getter expression in the
`@ReadOnly` on averageRating so it uses Base->link (e.g., `math::mean(->link[WHERE
predicate = 'recipe://user_rating'].out.uri)` should become one that starts with
`Base->link`, matching the documented convention for Base; ensure the getter in
the `@ReadOnly` on the `averageRating: number = 0;` property uses the Base
placeholder syntax shown elsewhere.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4498ea32-13e5-4477-ad24-f3f25d425d10
📒 Files selected for processing (1)
docs-src/pages/developer-guides/model-classes.mdx
Comment on lines
+118
to
+122
| @ReadOnly({ | ||
| through: "recipe://rating", | ||
| getter: `math::mean(->link[WHERE predicate = 'recipe://user_rating'].out.uri)` | ||
| }) | ||
| averageRating: number = 0; |
Contributor
There was a problem hiding this comment.
Inconsistent getter example — missing Base placeholder.
This example omits the Base placeholder, but the detailed explanation later (lines 162–167) explicitly states "Use Base as a placeholder for the current instance's node" and shows Base->link in the example. These should be consistent.
📝 Suggested fix
`@ReadOnly`({
through: "recipe://rating",
- getter: `math::mean(->link[WHERE predicate = 'recipe://user_rating'].out.uri)`
+ getter: `math::mean(Base->link[WHERE predicate = 'recipe://user_rating'].out.uri)`
})
averageRating: number = 0;📝 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
| @ReadOnly({ | |
| through: "recipe://rating", | |
| getter: `math::mean(->link[WHERE predicate = 'recipe://user_rating'].out.uri)` | |
| }) | |
| averageRating: number = 0; | |
| `@ReadOnly`({ | |
| through: "recipe://rating", | |
| getter: `math::mean(Base->link[WHERE predicate = 'recipe://user_rating'].out.uri)` | |
| }) | |
| averageRating: number = 0; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs-src/pages/developer-guides/model-classes.mdx` around lines 118 - 122,
The ReadOnly decorator example for averageRating is inconsistent: update the
getter to use the Base placeholder as explained later (use Base as the current
instance node). Replace the getter expression in the `@ReadOnly` on averageRating
so it uses Base->link (e.g., `math::mean(->link[WHERE predicate =
'recipe://user_rating'].out.uri)` should become one that starts with
`Base->link`, matching the documented convention for Base; ensure the getter in
the `@ReadOnly` on the `averageRating: number = 0;` property uses the Base
placeholder syntax shown elsewhere.
lucksus
approved these changes
Mar 11, 2026
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
2 participants
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.
PR: Update documentation for Ad4mModel API refactor
Branch:
docs/ad4m-model-api-refactorFiles changed: 6 | +1,060 / −944 lines
Summary
Updates all developer-facing documentation to reflect the Ad4mModel decorator API refactoring (PR #717). Covers API renames, structural improvements, accuracy fixes, and new feature documentation.
Commits
1.
docs: update documentation for Ad4mModel API refactorSystematic rename pass across all 6 doc files:
@Subject→@Model,@SubjectProperty→@Property,@SubjectCollection→@HasMany,@SubjectFlag→@FlagSubjectEntity→Ad4mModel,.baseExpression→.idSDNAClass.generateSDNA()→Ad4mModel.generateSDNA()FROM $parent→ raw SurrealQL expression)Files: model-classes.mdx, surreal-queries.mdx, batch-operations.mdx, hooks.mdx, index.mdx, social-dna.mdx
2.
docs: restructure batch-operations, model-classes, and surreal-queries guidesStructural improvements for clarity and scannability:
transaction()as the primary pattern; demoted manualcreateBatch()/commitBatch()to an "Advanced" sectionDEFINE INDEXstatements in source3.
docs: add missing API documentation to model-classes guideFilled documentation gaps found by auditing the source code:
.author,.createdAt,.updatedAt(with usage inorderqueries)includetransform,local,initial(sentinel behavior),getter(Base placeholder, SurrealQL wrapping)@Optionaldecorator — Mentioned as explicit alias for@Propertywithrequired: falsethrough— Documented that omittingthroughon relations defaults to'ad4m://has_child'getModelMetadata(),generateSDNA(),generateSHACL()with usage exampleFiles changed
model-classes.mdxsurreal-queries.mdxbatch-operations.mdxhooks.mdxindex.mdxsocial-dna.mdxSummary by CodeRabbit