Return metadata-only Article when no content is found - #45
Merged
fivefilters merged 3 commits intoJul 15, 2026
Merged
Conversation
When grabArticle finds no content, the title and document metadata have already been extracted; Readability.js throws that information away with its bare null return, but there is no reason for the PHP port to do the same. ParseException::noContent() now carries what was extracted (title, byline, dir, lang, excerpt, siteName, publishedTime, lead image) as readonly nullable properties, so callers can still label a failed extraction with the document's metadata. No option/toggle needed: the success path is unchanged and the data on the exception is free to ignore. The lead-image absolutization moves ahead of grabArticle so the failure path reports the same URL the success path would. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UdfoRvMdRY6ingGKPEBigK
Reworks the previous commit's design: instead of carrying the extracted title/metadata on ParseException, parse() now always returns an Article. When content detection fails (where Readability.js returns a bare null), the Article carries the title and metadata extracted before the failure, with the content-derived properties (content, textContent, length, contentElement) set to null; Article::hasContent() tells the two results apart. ParseException reverts to its simple form and is reserved for the cases where parsing cannot be attempted: empty input, and the maxElemsToParse guard (where Readability.js throws too). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UdfoRvMdRY6ingGKPEBigK
…ll-text-rss-php-85-deu0wn
fivefilters
marked this pull request as ready for review
July 15, 2026 12:09
fivefilters
marked this pull request as draft
July 15, 2026 12:23
fivefilters
marked this pull request as ready for review
July 15, 2026 12:23
fivefilters
added a commit
that referenced
this pull request
Jul 16, 2026
… 0.6.0 (#41) * Scaffolding for v4.0: PHP 8.4 Dom API, Mozilla 0.6.0 test corpus - Require PHP >= 8.4; drop masterminds/html5, psr/log, ext-xml - Remove src/Nodes (DOM subclasses, NodeTrait, NodeUtility): the new Dom\ classes have no registerNodeClass, and the hacks these existed for (node shifting, attribute-based state) are obsolete with Lexbor - New Configuration (readonly options, Readability.js 0.6.0 defaults), Article result object, RegExps (0.6.0 patterns), ParseException - Replace test corpus with Mozilla's 130 pages verbatim; keep 7 PHP-only pages with metadata converted to Mozilla's format; drop image fixtures and per-page config files - Test harness: DomCompare ports Mozilla's structural DOM comparison, ReadabilityTest mirrors Mozilla's jsdom test path; cross-check tools run Readability.js over the corpus for divergence attribution - CI matrix: PHP 8.4/8.5, no Docker/libxml pinning * Port Readability.js 0.6.0 core to PHP 8.4 Dom API Fresh method-by-method transcription of Readability.js v0.6.0 onto Dom\HTMLDocument (Lexbor). Method names and order mirror the JS prototype to keep upstream syncs mechanical. Notable PHP adaptations: - Scoring state and data-table flags live in per-parse SplObjectStorage maps (the new DOM API has no registerNodeClass or expando properties) - getAllNodesWithTag uses querySelectorAll snapshots, which is the only query path used while mutating - JS-compatible whitespace handling (NBSP etc.) for trim/normalize - toAbsoluteURI reproduces WHATWG URL parser behaviors that RFC 3986 resolvers reject or leave alone: control/tab/newline stripping, space encoding, empty-path normalization, colon-in-first-segment references - fdiv for JS division semantics where a zero score is possible - Readerable ports Readability-readerable.js (new to the PHP library) All 130 Mozilla test pages pass content, metadata and readerable comparisons; the 7 PHP-only pages await regenerated goldens. * Converge test corpus: regenerated PHP-only goldens, cross-check findings - Regenerate expected.html for the 7 PHP-only pages via the golden-file workflow, after validating the algorithm against Mozilla's 130 reference fixtures (extracted text length matches the old goldens) - Add readerable keys for those pages, computed with Mozilla's own isProbablyReaderable via jsdom - WHATWG URL behaviors in toAbsoluteURI (empty-path normalization, whitespace stripping, colon-in-first-segment refs, absolute passthrough for opaque schemes) - Treat test sources as UTF-8 text like jsdom does, so fixture meta charsets don't trigger re-decoding - Document accepted divergences from the npm Readability.js release in test/tools/known-divergences.md: the port tracks git master, which is what Mozilla's fixtures are generated from Full suite green: 415 tests, 946 assertions, 0 skipped. * Add unit tests for pure helpers and Configuration - ReadabilityUnitTest pins textSimilarity, unescapeHtmlEntities, toAbsoluteURI (including the WHATWG behaviors), isValidByline and getRowAndColumnCount against values verified with Readability.js - ConfigurationTest checks the 0.6.0 defaults and fromArray Also verified: E_ALL warning sweep over all 137 test pages is silent, parse state is released after each run, peak memory 16 MB for the whole corpus. * Docs for v4.0: README rewrite, CHANGELOG, upstream-sync guide - README: new Article/Configuration API, PHP 8.4 requirement, option reference with Readability.js mapping, v3 -> v4 migration table, cross-check tooling docs - CHANGELOG: v4.0.0 entry - CONTRIBUTING: how to sync with a new Readability.js release, and the intentional PHP differences that should not be 'fixed' * Fix PHP 8.5 SplObjectStorage deprecations PHP 8.5 deprecates SplObjectStorage::contains() and ::attach() in favor of offsetExists()/offsetSet(). Also surface deprecation details in test output by default so these show up in CI logs. Suite is now a clean OK on both 8.4 and 8.5 (was: 10 deprecations on 8.5). * CI: bump actions/checkout to v5 (Node 24) actions/checkout@v4 runs on Node.js 20, which GitHub Actions runners are deprecating. v5 runs on Node 24. * Update checkout action version to v7 * Use a real WHATWG URL parser: native Uri\WhatWg\Url on 8.5, rowbot/url on 8.4 Readability.js resolves URLs with the browser's WHATWG new URL(); the port emulated its behaviors (control/whitespace stripping, space encoding, empty-path normalization, opaque-scheme passthrough, colon-in-first-segment references) on top of league/uri's RFC 3986 resolver. Replace all of that with the real thing: PHP 8.5's native Uri\WhatWg\Url when available, falling back to rowbot/url (a WHATWG-compliant, WPT-tested parser) on PHP 8.4, via a small internal Url wrapper. toAbsoluteURI is now a 1:1 mirror of the JS closure, and isUrl matches JS new URL(str) strictness exactly. Also untrack .phpunit.result.cache (committed by accident earlier). * Add Psalm static analysis, bump PHPUnit to 12, tighten types - vimeo/psalm ^6 at errorLevel 3 (strictBinaryOperands off — mixed int/float arithmetic mirrors JS's single number type), wired into CI and exposed as 'composer analyse'. A small stub under stubs/ covers PHP 8.5's native URI classes so analysis on 8.4 resolves them. - Fix everything Psalm found: per-parse state ($doc/$scores/$dataTables) is now non-nullable and reset to fresh empty instances after parse (same memory release, no null-juggling); preg_* false/null returns get explicit fallbacks; Dom\Node::remove() calls (not part of that class) become removeChild(); unwrapNoscriptImages guards its querySelector results; allowedVideoRegex is typed non-empty-string with a guarded assignment; array properties and params get shape docblocks. - phpunit/phpunit ^11 -> ^12 (dev-only; ^13 conflicts with Psalm's sebastian/diff constraint). rowbot/url already latest. * Add UPGRADE.md (3.x to 4.0 guide), expand README usage examples UPGRADE.md covers the full migration: before/after code, a mapping table for every 3.x result getter and configuration option (verified against the actual 3.x API), replacement snippets for removed features (image extraction via contentElement->querySelectorAll, og:image via the document; PSR-3 -> debug flag), and the behavior changes (Article value object, page wrapper div, always-on byline, WHATWG URL fixing, encoding handling). All code snippets in the guide are executed and verified. README gains finer-control output and contentElement post-processing examples; its migration section now summarizes and links to UPGRADE.md, as does the CHANGELOG. * Reinstate three 3.x features: image extraction, keepInlineByline, PSR-3 logger Requested by the maintainer to ease 3.x upgrades — these were the removed features most likely to be missed. - Image extraction returns as readonly Article fields ($article->image, $article->images) rather than the old getter methods, fitting the value-object API. Lead image comes from og:image/twitter:image or <link rel=img_src|image_src>; the list prepends it to the content <img> srcs, de-duplicated. Both absolutized when fixRelativeURLs is on. - keepInlineByline (default false) replaces v3's articleByline. The byline is always extracted into Article::$byline now (as in JS); this option only controls whether an inline byline stays in the content. - PSR-3 logging returns via a Configuration $logger option (psr/log back as a dependency); messages go to the logger independently of the debug flag. log() dispatches to both. New PhpFeaturesTest covers all three. Docs (README options + Article fields, UPGRADE.md, CHANGELOG) updated. 472 tests green on 8.4/8.5, Psalm clean, corpus content output unchanged. * Document the keepInlineByline default behavior change more prominently Add a warning callout and a comparison table making explicit that a 3.x install using the default kept the inline byline in the content, whereas 4.0 removes it by default (keepInlineByline: true restores the old behavior). * Add isProbablyReaderable unit tests and expand its docs Mirror Mozilla's test/test-isProbablyReaderable.js: option tests for minContentLength, minScore, and a custom visibilityChecker (the corpus-wide readerable check already runs in ReadabilityTest). Widen minScore to float, as in Readability.js, whose own tests use fractional scores. Document the tuning parameters and the check-before-parse example in the README, note in UPGRADE.md how to reproduce 3.x's unwrapped content output, and credit the tooling used for the 4.0 rewrite. * Clarify that the unwrap one-liner keeps all top-level article elements * Always neutralize javascript: links, independent of fixRelativeURLs Readability.js always strips javascript: anchors in _postProcessContent via _fixRelativeUris. This port gated the entire fixRelativeUris() step behind the fixRelativeURLs config flag (default false), so with the default configuration javascript: links passed straight through to the output — a regression from upstream. The test corpus masked this because its harness always enables fixRelativeURLs (jsdom always has a base URI). Decouple the two concerns: javascript: neutralization now always runs (it needs no base URL and is a defense-in-depth measure), while absolutizing relative URLs stays opt-in via fixRelativeURLs. Add a regression test exercising the default configuration, and expand the README/UPGRADE security notes to spell out what does and does not survive extraction (event handlers, data: URIs on media, whitelisted video embeds) so callers still run a real sanitizer. * Revise CHANGELOG for v4.0.0 release * Accept options directly in the Readability constructor; drop build.Dockerfile - Readability's constructor now takes options as named arguments, the PHP equivalent of Readability.js's options object: new Readability(charThreshold: 20). A pre-built Configuration is still accepted for options built up separately or shared between instances, and new Readability() uses the defaults. Passing both at once throws. - Update README, UPGRADE.md, tests and the cross-check tool to the direct form, and stop constructing an empty Configuration just to get defaults. - Remove docker/php/build.Dockerfile: it existed to compile PHP against a pinned libxml2 for the old libxml parsing path. PHP >= 8.4 bundles the Lexbor HTML parser in ext-dom, so the plain official CLI images used by docker-compose (via docker/php/Dockerfile) are all that's needed, and nothing references the build file anymore. * Prepare 4.0.0-beta.1: retitle CHANGELOG entry, document @beta install flag * Merge parseDocument() into parse() parse() now accepts \Dom\HTMLDocument|string, matching Readerable::isProbablyReaderable() and leaving a single entry point, as in Readability.js. A passed document is still consumed (modified in place), as parseDocument() was documented to do. * Return metadata-only Article when no content is found (#45) * Preserve extracted metadata on ParseException When grabArticle finds no content, the title and document metadata have already been extracted; Readability.js throws that information away with its bare null return, but there is no reason for the PHP port to do the same. ParseException::noContent() now carries what was extracted (title, byline, dir, lang, excerpt, siteName, publishedTime, lead image) as readonly nullable properties, so callers can still label a failed extraction with the document's metadata. No option/toggle needed: the success path is unchanged and the data on the exception is free to ignore. The lead-image absolutization moves ahead of grabArticle so the failure path reports the same URL the success path would. * Return metadata-only Article when no content is found Reworks the previous commit's design: instead of carrying the extracted title/metadata on ParseException, parse() now always returns an Article. When content detection fails (where Readability.js returns a bare null), the Article carries the title and metadata extracted before the failure, with the content-derived properties (content, textContent, length, contentElement) set to null; Article::hasContent() tells the two results apart. ParseException reverts to its simple form and is reserved for the cases where parsing cannot be attempted: empty input, and the maxElemsToParse guard (where Readability.js throws too). --------- * Defer innerHTML serialization in the "Grabbed" debug log The log() call in grabArticle() concatenated $articleContent->innerHTML into the message unconditionally. Because PHP evaluates arguments eagerly, the full article subtree was serialized on every parse even when no logger was configured and debug was off, then discarded. Pass a closure instead and resolve it inside log()'s formatter, which runs only after the enabled check. When logging is off the innerHTML is never built; when it is on the output is unchanged. The formatter now resolves any Closure argument first, so other call sites can defer expensive values the same way. * Remove the Docker-based local test setup docker-compose.yml, the Makefile that wrapped it, and docker/ existed only to run the suite on multiple PHP versions locally. CI already covers PHP 8.4 and 8.5 directly via setup-php, and locally the suite runs with plain ./vendor/bin/phpunit, so the Docker layer is redundant maintenance. --------- Co-Authored-By: Claude Fable 5
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.
What
When
grabArticlefinds no content, the title and document metadata have already been extracted —getArticleMetadata()runs before content detection. Readability.js throws that information away with its barenullreturn; consumers (Full-Text RSS being the motivating case) want to label a "could not extract content" result with the page's title and metadata.Design (second iteration, after feedback): the extracted data lives on the
Articleobject, not on an exception.parse()now always returns anArticle; when no content was found, the content-derived properties arenullandArticle::hasContent()is the discriminator:ParseExceptionstays in its original simple form, reserved for the cases where parsing cannot be attempted — empty input, and themaxElemsToParseguard (where Readability.js throws anErrortoo). Finding no content is a normal outcome, not an exception.Articlechanges:content,textContent,length,contentElementbecome nullable;hasContent()added;__toString()returns''when there's no content;imagescontains just the lead image (if any) in the no-content case. The success path is byte-identical to before.Also: the lead-image absolutization moved ahead of
grabArticleso the no-content result reports the same URL a successful parse would.(Readability.js has no equivalent capability — its
parse()isif (!articleContent) return null;, metadata discarded, no option to keep it — so this is a documented PHP-specific extension.)Testing
testNoContentReturnsMetadataOnlyArticle(rich<head>metadata + empty body → all metadata properties populated, all content properties null,hasContent()false,(string)$article === '') andtestArticleWithContentReportsHasContent; the empty-input and oversized-document throw tests are unchanged../vendor/bin/phpunit— 483 tests, 1078 assertions, all green../vendor/bin/psalm— no errors; info-level issues identical to the base branch.README (usage section), UPGRADE.md and CHANGELOG updated.
🤖 Generated with Claude Code
https://claude.ai/code/session_01UdfoRvMdRY6ingGKPEBigK