feat(page-cluster)!: report cluster selection reasons via onClusterReason#927
Merged
Conversation
…ason Nitpicker needs to explain why pages ended up in the same template cluster (shared CSS, common header/footer, sidenav presence) instead of returning an opaque clusterKey. The old includeLandmarkPositions design held every member page's landmark data until final clustering completed, which is why it rejected corpora over 20,000 pages outright - fatal for a 170k-page production crawl. ClusterReason is sized by cluster count instead of page count, so it has no such ceiling and works unmodified on the streaming path. BREAKING CHANGE: includeLandmarkPositions, PageClusterKeyResult, and PageLandmarkReport are removed (never used in production). Landmark position extraction and chrome/content classification are now decomposed into independently-callable, stateless public APIs (extractLandmarks plus the newly public isChromeLandmarkInstance) that callers combine with onClusterReason's ClusterReason.landmarks[type].shellTokens themselves, instead of receiving one pre-built per-page report from the library. The CLI's --include-landmark-positions flag is replaced by --cluster-reasons-file <path>, which writes a clusterKey-keyed ClusterReason object once per run instead of per-page JSONL fields.
Verifying ClusterReason against a real 1,416-page crawl by independently reconstructing chromeRate via the public extractLandmarks + tokenize + isChromeLandmarkInstance recipe surfaced one apparent mismatch. It traced to the verification script skipping computePerPageLandmarkInstances' pre-existing per-page, cross-type signature dedup (the same rule that already prevents a CMS-duplicated footer, or a <header role="navigation"> matching both header and nav, from double-counting) - not a bug in the library. Classifying one instance you already have is unaffected either way; only reproducing the aggregate ratio by counting raw, un-deduplicated instances needs to know this.
YusukeHirao
added a commit
that referenced
this pull request
Jul 24, 2026
Includes page-cluster onClusterReason feature (PR #927) and related fixes.
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
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.
Summary
onClusterReason, a callback fired once per final cluster (not per page) with a structuredClusterReason: the blocking evidence that grouped it (shared stylesheet set or URL path prefix), the shared DOM-structural token core, per-landmark-type (header/footer/nav/aside/form/search) commonality within the cluster, and the sibling cluster keys it was split from within the same blocking group.includeLandmarkPositions,PageClusterKeyResult, andPageLandmarkReport(never used in production). That design held every member page's landmark data until final clustering completed, which is why it rejected corpora over 20,000 pages outright — a real problem for a 170k-page production crawl.ClusterReasonis sized by cluster count instead of page count, so it has no such ceiling and works unmodified on the streaming path.extractLandmarks, plus the newly publicisChromeLandmarkInstanceandjaccardSimilaritysubpaths) that callers combine withClusterReason.landmarks[type].shellTokensthemselves.--include-landmark-positionsis replaced by--cluster-reasons-file <path>, which writes aclusterKey-keyedClusterReasonobject once per run (not per-page JSONL fields) — no page-count limit.Why
Nitpicker needs to show why pages were grouped into a template cluster, not just an opaque
clusterKey. Building this surfaced that the existing landmark-report design conflated two concerns — page-level data (landmark position, needs per-page HTML) and cluster-level data (chrome/shell commonality, needs only cluster-count-bounded aggregates) — and the conflation was the actual cause of the prior 20,000-page ceiling.Test plan
yarn build— all 28 packages build cleanlyyarn lint— cleanyarn test— 1668 tests pass, including new coverage for:css/path/orphanMerge)presenceRate/chromeRate/shellTokens)onClusterReasononce per final cluster--cluster-reasons-filesuccess and failure (write error reported as clean exit 1, not an unhandled rejection)presenceRate/chromeRatefor every (cluster, landmark type) pair using only the public API and diffed againstClusterReason's own numbers — matched exactly (the one apparent mismatch traced to the verification script itself, not the library; fixed by clarifyingchromeRate's JSDoc about its per-page dedup denominator)🤖 Generated with Claude Code