Repository navigation
fix(core): reduce JSON-LD rule false positives (SEO018/019/021) + rendered entity handling - #57
Conversation
…dered entity handling SEO018: drop @id from URL_KEYS (it's a node identifier, often a relative fragment like "#organization"); broaden isAbsoluteUrl to accept any URI scheme (data:/mailto:/urn:) and protocol-relative URLs, so only scheme-less paths flag. SEO019: accept schema.org reduced-precision dates (year, year-month) in isIso8601, keeping calendar/range validation. SEO021: treat empty/blank/null/[] required values as missing via new hasNonEmpty helper instead of a presence-only `in` check. Rendered mode: read JSON-LD via script.rawText, not script.text — `<script>` is a raw-text element whose entities crawlers don't decode; .text decoded `"` and corrupted the JSON before SEO016 parsed it. Docs (en/ja seo018, seo019) and tests updated to match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR updates JSON-LD validation to accept broader URL and ISO date forms, treats blank required values as missing, and preserves raw ChangesJSON-LD validation updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/rules/seo/jsonld-engine.ts (1)
92-106: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject out-of-range timezone offsets in
isIso8601.Line 92 accepts any
±dd:ddoffset, and Lines 103-106 only range-check the localhh:mm:ss. That means values like2026-06-26T10:00:00+24:00or2026-06-26T10:00:00+00:60currently returntrue, so SEO019 will silently accept invalidDateTimevalues.Suggested fix
export function isIso8601(s: string): boolean { const str = s.trim(); if (!/^\d{4}(-\d{2}(-\d{2}(T\d{2}:\d{2}(:\d{2}(\.\d+)?)?(Z|[+-]\d{2}:\d{2})?)?)?)?$/.test(str)) return false; const y = Number(str.slice(0, 4)); const hasMonth = str.length >= 7; const hasDay = str.length >= 10; const m = hasMonth ? Number(str.slice(5, 7)) : 1; if (m < 1 || m > 12) return false; if (hasDay) { const d = Number(str.slice(8, 10)); const dt = new Date(Date.UTC(y, m - 1, d)); if (dt.getUTCFullYear() !== y || dt.getUTCMonth() !== m - 1 || dt.getUTCDate() !== d) return false; } - const tm = /^\d{4}-\d{2}-\d{2}T(\d{2}):(\d{2})(?::(\d{2}))?/.exec(str); + const tm = /^\d{4}-\d{2}-\d{2}T(\d{2}):(\d{2})(?::(\d{2}))?(?:\.\d+)?(?:Z|([+-])(\d{2}):(\d{2}))?$/.exec(str); if (tm) { const [hh, mm, ss] = [Number(tm[1]), Number(tm[2]), tm[3] !== undefined ? Number(tm[3]) : 0]; if (hh > 23 || mm > 59 || ss > 59) return false; + if (tm[5] !== undefined) { + const offH = Number(tm[5]); + const offM = Number(tm[6]); + if (offH > 23 || offM > 59) return false; + } } return true; }🤖 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/core/src/rules/seo/jsonld-engine.ts` around lines 92 - 106, `isIso8601` currently validates the local time parts but does not reject invalid timezone offsets, so out-of-range values like +24:00 or +00:60 can still pass. Update the offset validation in `isIso8601` to parse and range-check the timezone hours and minutes explicitly, alongside the existing `tm` local-time checks, and return false when the offset is outside valid ISO 8601 bounds.
🤖 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.
Outside diff comments:
In `@packages/core/src/rules/seo/jsonld-engine.ts`:
- Around line 92-106: `isIso8601` currently validates the local time parts but
does not reject invalid timezone offsets, so out-of-range values like +24:00 or
+00:60 can still pass. Update the offset validation in `isIso8601` to parse and
range-check the timezone hours and minutes explicitly, alongside the existing
`tm` local-time checks, and return false when the offset is outside valid ISO
8601 bounds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5aadd58d-e85e-4cbd-acef-2e901523dd9b
📒 Files selected for processing (11)
.changeset/seo-jsonld-false-positives.mddocs/src/content/docs/ja/rules/seo018.mddocs/src/content/docs/ja/rules/seo019.mddocs/src/content/docs/rules/seo018.mddocs/src/content/docs/rules/seo019.mdpackages/core/src/rules/seo/jsonld-engine.tspackages/core/src/rules/seo/seo016-021.tspackages/core/test/jsonld-engine.test.tspackages/core/test/seo-jsonld-rules.test.tspackages/vite/src/providers/rendered/parse-html.tspackages/vite/test/parse-html.test.ts
Follow-up to #55. These refinements were reviewed and committed locally but never pushed before #55 merged, so they're re-submitted here on their own branch.
Why
The JSON-LD rules from #55 produced false positives on valid, idiomatic structured data, and rendered-mode capture could corrupt JSON before validation.
Changes
@id(a node identifier, often a relative fragment like#organization) flagged as a relative URL;data:/mailto:/urn:/protocol-relative values also flagged@idfromURL_KEYS; broadenisAbsoluteUrlto accept any URI scheme or//protocol-relative — only scheme-less paths flag2026,2026-06) wrongly flaggedisIso8601accepts year / year-month precision (calendar + range validation kept)"headline": ""/null/[]counted as present (false negative)hasNonEmptyhelper treats empty/blank/null/empty-array required values as missingscript.textHTML-entity-decodes ("→"), corrupting JSON before SEO016 parses itscript.rawText—<script>is a raw-text element crawlers don't decodeDocs (en/ja for SEO018, SEO019) updated to match. Tests added for each case (core +10, vite +1).
Validation
pnpm -r test(368: core 165 / vite 58 / cli 136 / mcp 9),pnpm -r typecheck,pnpm lint— all green.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation