feat(prerender): refactor prerender audit#2548
Draft
anuj-adobe wants to merge 13 commits into
Draft
Conversation
71 tests across 12 files covering: early exits, URL selection (normal and CSV mode), bot-block detection (sticky + reactive), scrape error safety net, data integrity (status.json merge, S3 path contract, PageCitability writes), Mystique SQS dispatch constraints, opportunity/suggestion lifecycle, and AI-only mode routing. Tests mock only external I/O (S3, SQS, DB) — no internal stubs — so they remain valid through any refactoring of handler internals. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CLAUDE.md and .claude/ reference docs covering: handler step-by-step mechanics, suggestion lifecycle, system interactions (Mystique, scraper, API service, UI), batch mechanics, S3/SQS data flows, scraper internals, shared packages, coding guidelines, and refactoring proposal. These serve as the LLM brain map for future AI-assisted refactoring of this folder. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…866 lines) Decomposes the 1,875-line handler.js into single-concern modules: - bot-block.js: sticky check + reactive bot-block detection - mode-resolver.js: CSV/Slack/AI-only/Normal mode resolution - opportunity-syncer.js: opportunity + suggestion lifecycle management - scrape-stats.js: ScrapeJob statistics and scrape-forbidden counting - status-writer.js: status.json S3 read/write - url-fetcher.js: URL source fetching per mode - url-filter.js: PageCitability dedup + edge-deployed filter + batch cap Each module has 100% line/branch/function/statement coverage. Adds black-box behaviour tests (12 files, ~3,400 lines) that call only the three exported step functions and stub only external boundaries — immune to future module extraction. All 509 prerender tests pass. Also adds comprehensive AI-first documentation (src/prerender/CLAUDE.md + 11 sub-documents, test/audits/prerender/CLAUDE.md) covering invariants, locked contracts, S3 schemas, behavioural contracts, and refactoring plan. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
anuj-adobe
changed the title
Continues the module extraction from the previous PR: - html-comparator.js: getScrapedHtmlFromS3 + compareHtmlContent (S3 fetch + content analysis) - mystique-sender.js: sendPrerenderGuidanceRequestToMystique (AI summary dispatch) - ai-only.js: handleAiOnlyMode + fetchLatestScrapeJobId (ai-only execution path) - citability-writer.js: writeToCitabilityRecords (PageCitability DB writes) handler.js: 866 → 385 lines (-56%). Now contains only the three exported step functions and the AuditBuilder registration. Each module has 100% coverage. 575/575 prerender tests pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
This PR will trigger a minor release when merged. |
…utdatedSuggestions Extract three inline blocks from processContentAndGenerateOpportunities into their respective existing modules to move handler.js toward a spec-like north-star: - compareAllUrls → html-comparator.js (orchestrates per-URL comparison) - buildAuditResult → scrape-stats.js (pure function assembling audit output) - clearOutdatedSuggestions → opportunity-syncer.js (Branch C sync logic) handler.js no longer imports syncSuggestions directly; compareHtmlContent import replaced by compareAllUrls. Removes 5 now-duplicate handler.test.js tests covered by the new module unit tests. 144 unit tests passing (72 module + 72 behaviour). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Collapse the three-way opportunity branch (Branch A/B/C) and the deployedAtEdgePathnames + markNewSuggestionsAsCovered post-step into routeOpportunityBranch in opportunity-syncer.js, where all related functions already live. handler.js no longer imports sendPrerenderGuidanceRequestToMystique, createScrapeForbiddenOpportunity, processOpportunityAndSuggestions, markNewSuggestionsAsCovered, clearOutdatedSuggestions, or toPathname. The c8-ignore block is removed; coverage is now enforced via unit tests. Adds 5 unit tests in opportunity-syncer.test.js (33 total). Behaviour tests unchanged (72 passing). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Create log-metrics.js with named exports covering all structured log lines across the prerender audit pipeline. Consolidate the three step-3 log calls (scrape analysis, prerender findings, scraping metrics) into a single logStep3Metrics() call in handler.js. Merge the isDomainBlocked guard log into the existing Generate opportunities line (isDomainBlocked=<bool> field). Also covers logSubmitMetrics, logSuggestionsSyncMetrics, and logStatusUpload. 8 unit tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… util Delete src/prerender/utils/shared.js — no source file imports it; its agentic-sheet functions migrated to src/utils/agentic-urls.js. Remove all stale esmock stubs and unit tests referencing it from handler.test.js. Rename utils/html-comparator.js → utils/html-analyzer.js to eliminate the confusing same-name sibling pattern with the root html-comparator.js. The name now matches the wrapped @adobe/spacecat-shared-html-analyzer package. Update the one source import and all test esmock paths. 595 tests, 0 failures. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…yAndSuggestions Extract inline closures and IIFE into named private functions: - normalizeScrapedUrlsSet: pathname-normalised wrapper around rawScrapedUrlsSet - buildSuggestionKey: key selector used as syncSuggestions buildKey callback - buildSuggestionData: maps a comparison result to its Suggestion.data shape - buildAuditRunCandidates: builds the Mystique candidate list from preRenderSuggestions processOpportunityAndSuggestions shrinks from 157 → ~80 lines of clean sequential orchestration. No behaviour change. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Data Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… page-citability.js Merge getRecentlyProcessedPathnames (previously in url-filter.js) and writeToCitabilityRecords (previously in citability-writer.js) into a single page-citability.js module that owns all PageCitability entity interactions. Also update CLAUDE.md and handler-reference.md to reflect the current 15-module structure. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…er requires it D-08 is superseded — Mystique's Pydantic model no longer validates suggestion_id as required. Updated decision-log, CLAUDE.md, system-interactions, and coding-guidelines to reflect current state. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
3 tasks
4 tasks
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
1 participant
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 this PR does
Decomposes `src/prerender/handler.js` (1,875 lines, 34 functions) into 15 single-concern modules. The goal is to remove branching and nesting from the three orchestrating step functions so each reads as a flat sequence of named calls. No new business logic is introduced.
`handler.js`: 1,875 → 237 lines (−87%). Now contains only the three exported step functions and the AuditBuilder registration.
All three exported step functions (`importTopPages`, `submitForScraping`, `processContentAndGenerateOpportunities`) are unchanged in signature and observable behaviour — validated by 11 black-box behaviour test files that stub only external boundaries and never reference internal function names.
Reviewer guide — files by group
Group 1 — handler.js (1 file)
`src/prerender/handler.js`: 1,875 → 237 lines (−87%). Inline blocks replaced with named calls to the modules below. Same logic, different organisation.
Group 2 — New source modules (15 files)
Group 3 — Tests
Behaviour tests (`test/audits/prerender/behaviour/`, 11 files): Black-box tests calling only the three exported step functions and stubbing only external boundaries. Immune to module extraction — never reference internal function names.
Unit tests for new modules (13 files): One file per module, each at 100% lines/branches/functions/statements.
Modified test files: `handler.test.js` (stale stubs and duplicate tests removed), `behaviour/data-integrity.test.js` (import path updated to `page-citability.js`).
Group 4 — AI-first documentation (11 files, safe to skip in review)
`src/prerender/CLAUDE.md` + 10 sub-documents in `.claude/` + `test/audits/prerender/CLAUDE.md`. Context files for Claude Code — do not affect runtime behaviour. Cover invariants, locked contracts, S3 schemas, and behavioural contracts.
Coverage
Every new source file: 100% lines / branches / functions / statements. 595/595 prerender tests pass.
Test plan
🤖 Generated with Claude Code