-
Notifications
You must be signed in to change notification settings - Fork 7
Add JSON-LD structured data for BreadcrumbList and TechArticle #282
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
base: main
Are you sure you want to change the base?
Add JSON-LD structured data for BreadcrumbList and TechArticle #282
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds JSON-LD structured data schemas to improve SEO. Introduces breadcrumb and TechArticle schemas with conditional rendering based on page metadata, including site/page titles, URLs, and modification dates. Schemas are included in the document head. Changes
Sequence DiagramsequenceDiagram
participant Browser
participant head_custom.html
participant breadcrumb_schema.html
participant techarticle_schema.html
Browser->>head_custom.html: Render page head
alt Not home page
head_custom.html->>breadcrumb_schema.html: Include breadcrumb schema
breadcrumb_schema.html-->>head_custom.html: Render BreadcrumbList JSON-LD
end
head_custom.html->>techarticle_schema.html: Include TechArticle schema
techarticle_schema.html-->>head_custom.html: Render TechArticle JSON-LD<br/>(headline, description, dates, author)
head_custom.html-->>Browser: Inject schemas in <head>
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Comment |
Co-authored-by: gkorland <[email protected]>
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
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.
Pull Request Overview
This PR adds Schema.org JSON-LD structured data markup to improve SEO and enable rich snippets in search results for the FalkorDB documentation site.
- Implements BreadcrumbList schema to show page hierarchy in search results
- Implements TechArticle schema to mark pages as technical documentation
- Integrates both schemas into the site header via head_custom.html
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
_includes/breadcrumb_schema.html |
Generates BreadcrumbList JSON-LD from page hierarchy using Jekyll frontmatter |
_includes/techarticle_schema.html |
Creates TechArticle JSON-LD with page metadata and organization details |
_includes/head_custom.html |
Includes both schema templates in the site header |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "name": "FalkorDB", | ||
| "logo": { | ||
| "@type": "ImageObject", | ||
| "url": "{{ site.url | default: 'https://docs.falkordb.com' }}/images/falkor-logo.png" |
Copilot
AI
Nov 17, 2025
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.
The logo URL path does not match the path specified in the requirements. According to the PR description, the logo should be at '/assets/logo.png', but this implementation uses '/images/falkor-logo.png'. This will result in a broken logo URL unless the file exists at this alternate location.
| "url": "{{ site.url | default: 'https://docs.falkordb.com' }}/images/falkor-logo.png" | |
| "url": "{{ site.url | default: 'https://docs.falkordb.com' }}/assets/logo.png" |
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.
@copilot the logo is in images/falkor-logo.png
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.
Confirmed - the logo path /images/falkor-logo.png is correct and matches the site configuration in _config.yml (line 4). The file exists and the schema is using the correct path.
| {% if page.last_modified_at %} | ||
| "dateModified": "{{ page.last_modified_at | date_to_xmlschema }}", | ||
| {% elsif page.date %} | ||
| "dateModified": "{{ page.date | date_to_xmlschema }}", | ||
| {% endif %} |
Copilot
AI
Nov 17, 2025
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.
The TechArticle schema is missing the required 'datePublished' field as specified in the requirements. The schema includes 'dateModified' but should include both 'datePublished' (from page.date) and 'dateModified' (from page.last_modified_at with fallback to page.date) as separate fields.
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.
@copilot apply changes based on this feedback
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.
Added the datePublished field to the TechArticle schema. The schema now includes both datePublished (from page.date) and dateModified (from page.last_modified_at with fallback to page.date) as separate fields as required. Changes in commit e2343f6.
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
_includes/breadcrumb_schema.html(1 hunks)_includes/head_custom.html(1 hunks)_includes/techarticle_schema.html(1 hunks)
🔇 Additional comments (4)
_includes/head_custom.html (1)
14-17: Include statements added correctly.The two new schema includes are placed appropriately after the GTM block. Syntax and placement look good.
_includes/breadcrumb_schema.html (1)
18-22: Verify parent page discovery handles edge cases.Line 20 searches for pages by title:
site.pages | where: "title", page.parent. If multiple pages share the same title, only the first match is used (line 22). Consider whether this is the intended behavior, or if a more specific key (e.g., page URL or slug) would be more reliable._includes/techarticle_schema.html (2)
5-36: Schema structure and sanitization look solid.The TechArticle schema is well-formed with appropriate fallbacks (lines 9, 13) and defensive sanitization of user-provided content (line 11:
strip_html | strip_newlines | escape). The dateModified fallback chain (lines 14–18) handles pages without explicit timestamps gracefully.
28-28: Verify that the logo image path exists at build/deployment time.Line 28 references
/images/falkor-logo.pngrelative tosite.url. Ensure this asset is present in the deployed site to avoid broken schema references.
| {% if page.url != "/" %} | ||
| <script type="application/ld+json"> | ||
| { | ||
| "@context": "https://schema.org", | ||
| "@type": "BreadcrumbList", | ||
| "itemListElement": [ | ||
| { | ||
| "@type": "ListItem", | ||
| "position": 1, | ||
| "name": "Home", | ||
| "item": "{{ site.url | default: 'https://docs.falkordb.com' }}" | ||
| } | ||
| {% assign position = 2 %} | ||
| {% if page.parent %} | ||
| {% comment %}Find parent page{% endcomment %} | ||
| {% assign parent_pages = site.pages | where: "title", page.parent %} | ||
| {% if parent_pages.size > 0 %} | ||
| {% assign parent = parent_pages | first %} | ||
| ,{ | ||
| "@type": "ListItem", | ||
| "position": {{ position }}, | ||
| "name": "{{ parent.title }}", | ||
| "item": "{{ site.url | default: 'https://docs.falkordb.com' }}{{ parent.url }}" | ||
| } | ||
| {% assign position = position | plus: 1 %} | ||
| {% endif %} | ||
| {% endif %} | ||
| ,{ | ||
| "@type": "ListItem", | ||
| "position": {{ position }}, | ||
| "name": "{{ page.title }}", | ||
| "item": "{{ site.url | default: 'https://docs.falkordb.com' }}{{ page.url }}" | ||
| } | ||
| ] | ||
| } | ||
| </script> |
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.
Critical: Invalid JSON syntax due to conditional leading commas.
The JSON-LD BreadcrumbList has a fatal flaw in how commas are handled around conditionally-rendered items.
When the parent block (lines 18–31) does not render, the resulting JSON is:
"itemListElement": [
{ "position": 1, ... }
,{ "position": 2, ... } // ← Invalid: leading comma after closing brace
]This violates JSON syntax and will be rejected by search engines. Similarly, when the parent block does render, you get a double-comma sequence (},\n ,{), which is also invalid.
The root cause: The leading commas at lines 23 and 32 assume a preceding element, but JSON requires commas between elements, not before them.
Suggested fix: Move the comma after the Home item and handle the parent conditionally:
{
"@type": "ListItem",
"position": 1,
"name": "Home",
"item": "{{ site.url | default: 'https://docs.falkordb.com' }}"
- }
+ },
{% assign position = 2 %}
{% if page.parent %}
{% comment %}Find parent page{% endcomment %}
{% assign parent_pages = site.pages | where: "title", page.parent %}
{% if parent_pages.size > 0 %}
{% assign parent = parent_pages | first %}
- ,{
+ {
"@type": "ListItem",
"position": {{ position }},
"name": "{{ parent.title }}",
"item": "{{ site.url | default: 'https://docs.falkordb.com' }}{{ parent.url }}"
- }
+ },
{% assign position = position | plus: 1 %}
{% endif %}
{% endif %}
- ,{
+ {
"@type": "ListItem",
"position": {{ position }},
"name": "{{ page.title }}",
"item": "{{ site.url | default: 'https://docs.falkordb.com' }}{{ page.url }}"
}
]This ensures the JSON array maintains valid syntax regardless of whether the parent element is present.
Co-authored-by: gkorland <[email protected]>
|
https://gh.io/copilot-coding-agent-docs apply changes based on #282 (comment) |
The changes from discussion r2533384371 have already been applied in commit e2343f6. The TechArticle schema now includes both |
Implement JSON-LD Schema.org markup for BreadcrumbList and TechArticle across the FalkorDB docs site.
Plan:
Implementation Summary:
Files Created/Modified:
_includes/breadcrumb_schema.html - BreadcrumbList JSON-LD schema
_includes/techarticle_schema.html - TechArticle JSON-LD schema (Updated)
_includes/head_custom.html - Updated to include both schemas
Latest Changes:
datePublishedfield to TechArticle schema (from page.date)dateModifiedfield (from page.last_modified_at with fallback to page.date)Validation:
Original prompt
This pull request was created as a result of the following prompt from Copilot chat.
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Summary by CodeRabbit