Skip to content

feat(page-cluster)!: report cluster selection reasons via onClusterReason#927

Merged
YusukeHirao merged 2 commits into
devfrom
worktree-page-cluster-reasons
Jul 24, 2026
Merged

feat(page-cluster)!: report cluster selection reasons via onClusterReason#927
YusukeHirao merged 2 commits into
devfrom
worktree-page-cluster-reasons

Conversation

@YusukeHirao

Copy link
Copy Markdown
Member

Summary

  • Add onClusterReason, a callback fired once per final cluster (not per page) with a structured ClusterReason: 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.
  • This directly answers "why are these pages in the same cluster" (e.g. "header/footer are common chrome, but they differ in whether a sidebar nav is present") without page-cluster ever producing human-readable text — callers assemble their own wording from the structured data.
  • Breaking change: removes includeLandmarkPositions, PageClusterKeyResult, and PageLandmarkReport (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. ClusterReason is sized by cluster count instead of page count, so it has no such ceiling and works unmodified on the streaming path.
  • Landmark position extraction and chrome/content classification are decomposed into independently-callable, stateless public APIs (extractLandmarks, plus the newly public isChromeLandmarkInstance and jaccardSimilarity subpaths) that callers combine with ClusterReason.landmarks[type].shellTokens themselves.
  • CLI: --include-landmark-positions is replaced by --cluster-reasons-file <path>, which writes a clusterKey-keyed ClusterReason object 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 cleanly
  • yarn lint — clean
  • yarn test — 1668 tests pass, including new coverage for:
    • blocking reason variants (css / path / orphanMerge)
    • per-landmark-type commonality (presenceRate / chromeRate / shellTokens)
    • sibling-cluster computation for a real Stage A split within one block
    • streaming path (20,001 synthetic pages) completing without error and firing onClusterReason once per final cluster
    • CLI --cluster-reasons-file success and failure (write error reported as clean exit 1, not an unhandled rejection)
  • Verified against a real ~1,400-page production crawl archive: independently recomputed presenceRate/chromeRate for every (cluster, landmark type) pair using only the public API and diffed against ClusterReason's own numbers — matched exactly (the one apparent mismatch traced to the verification script itself, not the library; fixed by clarifying chromeRate's JSDoc about its per-page dedup denominator)

🤖 Generated with Claude Code

…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
YusukeHirao requested a review from yusasa16 as a code owner July 24, 2026 16:41
@YusukeHirao
YusukeHirao merged commit b0d1e7e into dev Jul 24, 2026
6 checks passed
@YusukeHirao
YusukeHirao deleted the worktree-page-cluster-reasons branch July 24, 2026 17:23
YusukeHirao added a commit that referenced this pull request Jul 24, 2026
Includes page-cluster onClusterReason feature (PR #927) and related fixes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant