-
Notifications
You must be signed in to change notification settings - Fork 4
eng-1212 Base json-ld export #648
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
4d19e5b to
483e769
Compare
|
Breaking up of eng-1032 is shown in https://www.loom.com/share/afa48744aa0441628ec182a2147e8321 |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThe changes add JSON-LD export capability to the Roam application with supporting schema definitions. A new canonicalRoamUrl utility generates space URLs, while getPageMetadata adds modification timestamp tracking. New RDF ontology files define a DiscourseGraph schema vocabulary for nodes and relationships. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Client
participant Export as Export Handler
participant JsonLd as jsonld.ts
participant PageMeta as getPageMetadata
participant PageMd as pageToMarkdown
participant Schema as Schema Data
User->>Export: Initiate JSON-LD export
Export->>Export: Gather nodes, relations, results
Export->>JsonLd: getJsonLdData(results, nodes, relations, ...)
rect rgb(200, 220, 255)
Note over JsonLd: Schema Generation Phase
JsonLd->>JsonLd: getJsonLdSchema(allNodes, allRelations)
loop For each node
JsonLd->>PageMeta: getPageMetadata(nodeName)
PageMeta-->>JsonLd: Return metadata + modified time
JsonLd->>PageMd: pageToMarkdown(pageData)
PageMd-->>JsonLd: Return markdown content
JsonLd->>Export: updateExportProgress(progress)
end
end
rect rgb(200, 255, 220)
Note over JsonLd: Document Build Phase
JsonLd->>JsonLd: jsonLdContext(baseUrl)
JsonLd->>JsonLd: Construct `@context` with CURIE mappings
JsonLd->>JsonLd: Build `@graph` with schema + node entries
JsonLd->>JsonLd: Process relations with blank node refs
JsonLd->>JsonLd: Deduplicate relation instances
end
JsonLd-->>Export: Return JSON-LD document
Export->>Schema: Save JSON-LD output
Schema-->>User: Export complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Pre-merge checks✅ Passed checks (3 passed)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🧹 Nitpick comments (5)
apps/roam/src/utils/canonicalRoamUrl.ts (1)
1-4: Consider adding explicit return type.As per coding guidelines, functions should have explicit return types.
🔎 Proposed fix
const ROAM_URL_PREFIX = "https://roamresearch.com/#/app/"; -const canonicalRoamUrl = (graphName = window.roamAlphaAPI.graph.name) => +const canonicalRoamUrl = (graphName = window.roamAlphaAPI.graph.name): string => ROAM_URL_PREFIX + graphName; export default canonicalRoamUrl;apps/roam/src/utils/jsonld.ts (2)
11-35: Consider adding trailing delimiter todgbnamespace URI.RDF namespace URIs typically end with
#or/to properly form complete URIs when concatenated with local names. Thedgbprefix at line 18 lacks this delimiter, which may cause issues with RDF processors.🔎 Proposed fix
- dgb: "https://discoursegraphs.com/schema/dg_base", + dgb: "https://discoursegraphs.com/schema/dg_base#",
134-145: Mutating inputpageobjects may cause unintended side effects.Line 141 mutates
page.contentdirectly on theResultobjects frompageData. If these objects are shared or used elsewhere, this could lead to unexpected behavior.🔎 Proposed fix - create new objects instead of mutating
+ const pageContents: Map<string, string> = new Map(); await Promise.all( pageData.map(async (page: Result) => { const r = await pageToMarkdown(page, { ...settings, allNodes, linkType: "roam url", }); - page.content = r.content; + pageContents.set(page.uid, r.content); numTreatedPages += 1; await updateExportProgress(0.1 + (numTreatedPages / numPages) * 0.75); }), ); const nodes = pageData.map(({ text, uid, content, type }) => { const { date, displayName, modified } = getPageMetadata(text); + const resolvedContent = pageContents.get(uid) ?? (content as string); const r = { "@id": `pages:${uid}`, "@type": nodeSchemaUriByName[type], title: text, - content: content as string, + content: resolvedContent, modified: modified?.toJSON(), created: date.toJSON(), creator: displayName, }; return r; });apps/website/public/schema/dg_base.ttl (1)
3-3: Remove redundant prefix declaration.The
:prefix is defined as the same namespace asrdfs:, but it's not used elsewhere in the file. Consider removing this redundant declaration.🔎 Proposed fix
-@prefix : <http://www.w3.org/2000/01/rdf-schema#> .apps/website/public/schema/dg_core.ttl (1)
6-8: Consider removing unused namespace prefixes.The prefixes
vs:,sioc:, andprov:are declared but not used in this file. Consider removing them to reduce clutter, or keep them if they're planned for future use.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/roam/src/utils/canonicalRoamUrl.tsapps/roam/src/utils/getExportTypes.tsapps/roam/src/utils/getPageMetadata.tsapps/roam/src/utils/jsonld.tsapps/roam/src/utils/pageToMarkdown.tsapps/roam/src/utils/supabaseContext.tsapps/website/public/schema/dg_base.ttlapps/website/public/schema/dg_core.ttl
💤 Files with no reviewable changes (1)
- apps/roam/src/utils/pageToMarkdown.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/main.mdc)
**/*.{ts,tsx}: Use Tailwind CSS for styling where possible
When refactoring inline styles, use tailwind classes
Prefertypeoverinterfacein TypeScript
Use explicit return types for functions
Avoidanytypes when possible
Prefer arrow functions over regular function declarations
Use named parameters (object destructuring) when a function has more than 2 parameters
Use PascalCase for components and types
Use camelCase for variables and functions
Use UPPERCASE for constants
Function names should describe their purpose clearly
Prefer early returns over nested conditionals for better readability
Files:
apps/roam/src/utils/supabaseContext.tsapps/roam/src/utils/getPageMetadata.tsapps/roam/src/utils/canonicalRoamUrl.tsapps/roam/src/utils/getExportTypes.tsapps/roam/src/utils/jsonld.ts
apps/roam/**/*.{js,ts,tsx,jsx,json}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Prefer existing dependencies from package.json when working on the Roam Research extension
Files:
apps/roam/src/utils/supabaseContext.tsapps/roam/src/utils/getPageMetadata.tsapps/roam/src/utils/canonicalRoamUrl.tsapps/roam/src/utils/getExportTypes.tsapps/roam/src/utils/jsonld.ts
apps/roam/**/*.{ts,tsx,jsx,js,css,scss}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension
Files:
apps/roam/src/utils/supabaseContext.tsapps/roam/src/utils/getPageMetadata.tsapps/roam/src/utils/canonicalRoamUrl.tsapps/roam/src/utils/getExportTypes.tsapps/roam/src/utils/jsonld.ts
apps/roam/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
apps/roam/**/*.{ts,tsx,js,jsx}: Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when implementing Roam functionality
Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when implementing extension functionality
Files:
apps/roam/src/utils/supabaseContext.tsapps/roam/src/utils/getPageMetadata.tsapps/roam/src/utils/canonicalRoamUrl.tsapps/roam/src/utils/getExportTypes.tsapps/roam/src/utils/jsonld.ts
apps/roam/**
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Implement the Discourse Graph protocol in the Roam Research extension
Files:
apps/roam/src/utils/supabaseContext.tsapps/roam/src/utils/getPageMetadata.tsapps/roam/src/utils/canonicalRoamUrl.tsapps/roam/src/utils/getExportTypes.tsapps/roam/src/utils/jsonld.ts
apps/website/**
📄 CodeRabbit inference engine (.cursor/rules/website.mdc)
Prefer existing dependencies from apps/website/package.json when adding dependencies to the website application
Files:
apps/website/public/schema/dg_base.ttlapps/website/public/schema/dg_core.ttl
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/** : Implement the Discourse Graph protocol in the Roam Research extension
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{ts,tsx,js,jsx} : Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when implementing Roam functionality
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{ts,tsx,js,jsx} : Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when implementing extension functionality
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 189
File: packages/database/supabase/migrations/20250603144146_account_centric.sql:50-63
Timestamp: 2025-06-04T11:41:34.951Z
Learning: In the discourse-graph database, all accounts currently stored are Roam platform accounts, making platform-specific migration logic safe for global operations.
Learnt from: sid597
Repo: DiscourseGraphs/discourse-graph PR: 232
File: apps/roam/src/utils/getAllDiscourseNodesSince.ts:18-31
Timestamp: 2025-06-22T10:40:52.752Z
Learning: In apps/roam/src/utils/getAllDiscourseNodesSince.ts, the user confirmed that querying for `?title` with `:node/title` and mapping it to the `text` field in the DiscourseGraphContent type is the correct implementation for retrieving discourse node content from Roam Research, despite it appearing to query page titles rather than block text content.
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 220
File: apps/roam/src/utils/conceptConversion.ts:11-40
Timestamp: 2025-06-23T11:49:45.457Z
Learning: In the DiscourseGraphs/discourse-graph codebase, direct access to `window.roamAlphaAPI` is the established pattern throughout the codebase. The team prefers to maintain this pattern consistently rather than making piecemeal changes, and plans to address dependency injection as a global refactor when scheduled.
📚 Learning: 2025-11-23T23:53:43.094Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-11-23T23:53:43.094Z
Learning: In `apps/roam/src/utils/supabaseContext.ts`, the Roam URL normalization uses a targeted string replacement `url.replace("/?server-port=3333#/", "/#/")` rather than URL parsing to avoid impacting unforeseen URL patterns. This conservative approach is intentional to handle only the specific known case.
Applied to files:
apps/roam/src/utils/supabaseContext.tsapps/roam/src/utils/canonicalRoamUrl.ts
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{ts,tsx,js,jsx} : Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when implementing Roam functionality
Applied to files:
apps/roam/src/utils/supabaseContext.tsapps/roam/src/utils/canonicalRoamUrl.tsapps/roam/src/utils/jsonld.ts
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{ts,tsx,js,jsx} : Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when implementing extension functionality
Applied to files:
apps/roam/src/utils/supabaseContext.tsapps/roam/src/utils/canonicalRoamUrl.tsapps/roam/src/utils/jsonld.ts
📚 Learning: 2025-05-30T14:37:30.215Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 182
File: apps/website/app/api/supabase/rpc/search-content/route.ts:53-56
Timestamp: 2025-05-30T14:37:30.215Z
Learning: The search-content route (apps/website/app/api/supabase/rpc/search-content/route.ts) is intentionally designed to be platform-agnostic rather than Roam-specific. The generic "Platform" terminology (like subsetPlatformIds) is used because the route will support multiple platforms in the near future, not just Roam.
Applied to files:
apps/roam/src/utils/supabaseContext.ts
📚 Learning: 2025-11-05T21:57:14.909Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 534
File: apps/roam/src/utils/createReifiedBlock.ts:40-48
Timestamp: 2025-11-05T21:57:14.909Z
Learning: In the discourse-graph repository, the function `getPageUidByPageTitle` from `roamjs-components/queries/getPageUidByPageTitle` is a synchronous function that returns a string directly (the page UID or an empty string if not found), not a Promise. It should be called without `await`.
Applied to files:
apps/roam/src/utils/supabaseContext.tsapps/roam/src/utils/getPageMetadata.ts
📚 Learning: 2025-06-22T10:40:52.752Z
Learnt from: sid597
Repo: DiscourseGraphs/discourse-graph PR: 232
File: apps/roam/src/utils/getAllDiscourseNodesSince.ts:18-31
Timestamp: 2025-06-22T10:40:52.752Z
Learning: In apps/roam/src/utils/getAllDiscourseNodesSince.ts, the user confirmed that querying for `?title` with `:node/title` and mapping it to the `text` field in the DiscourseGraphContent type is the correct implementation for retrieving discourse node content from Roam Research, despite it appearing to query page titles rather than block text content.
Applied to files:
apps/roam/src/utils/supabaseContext.tsapps/roam/src/utils/getPageMetadata.ts
📚 Learning: 2025-12-07T20:54:20.007Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 559
File: apps/roam/src/utils/findDiscourseNode.ts:37-39
Timestamp: 2025-12-07T20:54:20.007Z
Learning: In apps/roam/src/utils/findDiscourseNode.ts, the function findDiscourseNodeByTitleAndUid accepts both uid and title parameters where uid is primarily used for cache access (as the cache key) while title is used for the actual matching via matchDiscourseNode. This design reflects the pattern where downstream, the uid is mostly used to fetch the title, so the function caches by uid but matches by title.
Applied to files:
apps/roam/src/utils/supabaseContext.tsapps/roam/src/utils/getPageMetadata.ts
📚 Learning: 2025-06-25T22:56:17.522Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-06-25T22:56:17.522Z
Learning: In the Roam discourse-graph system, the existence of the configuration page (identified by DISCOURSE_CONFIG_PAGE_TITLE) and its corresponding UID is a system invariant. The code can safely assume this page will always exist, so defensive null checks are not needed when using `getPageUidByPageTitle(DISCOURSE_CONFIG_PAGE_TITLE)`.
Applied to files:
apps/roam/src/utils/supabaseContext.ts
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{ts,tsx,jsx,js,css,scss} : Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension
Applied to files:
apps/roam/src/utils/supabaseContext.ts
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/** : Implement the Discourse Graph protocol in the Roam Research extension
Applied to files:
apps/roam/src/utils/supabaseContext.tsapps/roam/src/utils/canonicalRoamUrl.tsapps/roam/src/utils/jsonld.ts
📚 Learning: 2025-06-23T11:49:45.457Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 220
File: apps/roam/src/utils/conceptConversion.ts:11-40
Timestamp: 2025-06-23T11:49:45.457Z
Learning: In the DiscourseGraphs/discourse-graph codebase, direct access to `window.roamAlphaAPI` is the established pattern throughout the codebase. The team prefers to maintain this pattern consistently rather than making piecemeal changes, and plans to address dependency injection as a global refactor when scheduled.
Applied to files:
apps/roam/src/utils/canonicalRoamUrl.ts
📚 Learning: 2025-05-20T03:06:16.600Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 165
File: packages/database/schema.yaml:116-121
Timestamp: 2025-05-20T03:06:16.600Z
Learning: In the discourse-graph project's LinkML schema (packages/database/schema.yaml), attributes and slots are equivalent constructs. Items can be defined either as slots or attributes without needing to duplicate them in both sections.
Applied to files:
apps/website/public/schema/dg_base.ttl
📚 Learning: 2025-10-08T18:55:18.029Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-10-08T18:55:18.029Z
Learning: In the discourse-graph database schema, security views like `my_concepts`, `my_contents`, and `my_documents` were introduced to significantly improve query performance. Direct table access had dismal performance, while the view-based approach provides substantial performance gains.
Applied to files:
apps/website/public/schema/dg_base.ttl
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{js,ts,tsx,jsx,json} : Prefer existing dependencies from package.json when working on the Roam Research extension
Applied to files:
apps/roam/src/utils/jsonld.ts
🧬 Code graph analysis (1)
apps/roam/src/utils/getExportTypes.ts (1)
apps/roam/src/utils/jsonld.ts (1)
getJsonLdData(94-181)
🔇 Additional comments (5)
apps/roam/src/utils/supabaseContext.ts (1)
5-5: LGTM!Clean refactor to use the centralized
canonicalRoamUrlutility, eliminating the localROAM_URL_PREFIXconstant and simplifying URL construction.Also applies to: 58-58
apps/roam/src/utils/getExportTypes.ts (1)
148-170: LGTM!The new JSON-LD export option follows the established patterns:
- Consistent progress tracking with other exports
- Proper filename handling
- Pretty-printed JSON output for readability
apps/roam/src/utils/jsonld.ts (1)
37-92: LGTM on schema generation!The schema generation correctly:
- Processes node schemas with metadata
- Creates relation definitions with domain/range
- Generates inverse relations with proper
owl:inverseOfreferencesapps/website/public/schema/dg_core.ttl (2)
12-26: LGTM! Well-defined node schemas.The node schema definitions (Question, Claim, Evidence, Source) are clear, well-documented, and follow proper RDF/OWL patterns.
72-92: LGTM! Correct inverse relation references.The relation pairs
addresses/addressedByandcuratedTo/curatedFromcorrectly use lowercase URI references in theirowl:inverseOfdeclarations, unlike the earlier relations.
mdroidian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets address the coderabbit suggestions (or resolve if unnecessary ), then re-tag for review.
4aaf3e1 to
715c55c
Compare
c5d3cc0 to
f3e9ded
Compare
This is the main JSON-LD export functionality, the base schema, etc.
Incidentally: A utility function for the canonical roam URL.
Add modified-date to page metadata.
Summary by CodeRabbit
Release Notes
New Features
Refactoring
✏️ Tip: You can customize this high-level summary in your review settings.