Skip to content

feat: generate path-level prerender suggestions in audit worker#2500

Merged
ssilare-adobe merged 89 commits into
mainfrom
feat/path-level-prerender-suggestions
Jul 6, 2026
Merged

feat: generate path-level prerender suggestions in audit worker#2500
ssilare-adobe merged 89 commits into
mainfrom
feat/path-level-prerender-suggestions

Conversation

@ssilare-adobe

@ssilare-adobe ssilare-adobe commented May 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements path-level prerender suggestion generation in the audit worker — the third tier between per-URL and domain-wide suggestions. Groups URLs by first path segment (e.g. /products/*, /blog/*), scores each group using the same formula as the rcv-scoring-dashboard, and emits deployable path suggestions.

What it does

  • Generates path suggestions — groups per-URL audit results by first path segment, qualifies via pluggable strategy (default: RcvPathQualificationStrategy), persists qualifying paths with allowedRegexPatterns
  • Opt-in per site — controlled by prerender.pathSuggestionsEnabled config flag (defaults false)
  • Skips when domain-wide is deployed — no path suggestions generated if /* is already deployed (redundant)
  • Preserves deployed/active paths — re-audits keep deployed or active path suggestions; only metrics are refreshed
  • Marks coverage — after sync, marks matching per-URL NEW suggestions as coveredByPattern for deployed paths (including exact root segment match, e.g. /products)
  • Marks path suggestions as coveredByDomainWide — when domain-wide is deployed, existing path suggestions get coveredByDomainWide set
  • Self-heals — clears stale coveredByPattern references pointing to undeployed/deleted paths on every audit run
  • Error resilient — all saveMany calls wrapped in try-catch with error logging

Path suggestion data shape (stored in DB)

{
  "type": "CONFIG_UPDATE",
  "rank": 0,
  "status": "NEW",
  "data": {
    "url": "https://example.com/products/*",
    "allowedRegexPatterns": ["/products/*"],
    "score": 2.84,
    "contentGainRatio": 1.8,
    "wordCountBefore": 9600,
    "wordCountAfter": 19200,
    "aiReadablePercent": 78.5
  }
}

Detection: Array.isArray(data.allowedRegexPatterns) && !data.isDomainWide
Key: {allowedRegexPatterns[0]}|prerender (e.g. /products/*|prerender)

Changes summary

Category Description What changed
New file Core path suggestion logic src/prerender/features/path-suggestions/path-suggestions.jsbuildPathTypeSuggestions, findPreservablePathSuggestions, markSuggestionsAsCoveredByPaths
New file Strategy interface (abstract base class) src/prerender/features/path-suggestions/strategies/path-qualification-strategy.jsPathQualificationStrategy with abstract qualify()
New file RCV strategy implementation src/prerender/features/path-suggestions/strategies/rcv-path-qualification-strategy.jsRcvPathQualificationStrategy extends PathQualificationStrategy
New file Barrel index src/prerender/features/path-suggestions/index.js — re-exports all public APIs
New file SQL for agentic hits src/cdn-logs-report/sql/top-agentic-urls-with-hits.sql — returns url + total_hits over a 4-week window
New function Agentic hits map from Athena getAgenticHitsMapFromAthena() in src/utils/agentic-urls.js — returns Map<pathname, totalHits>
New function SQL query builder for hits createTopUrlsWithHitsQuery() in src/cdn-logs-report/utils/query-builder.js
Refactored Shared Athena setup helper runAthenaQuery() in src/utils/agentic-urls.js — DRYs up config guards, client creation, error handling between existing and new Athena queries
Updated Handler wiring src/prerender/handler.js — path generation, preservation, metrics refresh, sync, coverage marking, domain-wide skip, buildMergeDataFunction extracted
Updated Prerender utils src/prerender/utils/utils.js — added isPathSuggestionData, isDomainWideSuggestionData, extractPathType, shouldPreservePathSuggestion, isEligibleStatus
Updated Constants src/prerender/utils/constants.js — added PATH_TYPE_MIN_URLS=10, PATH_TYPE_MIN_VALUABLE_PCT=33, PATH_TYPE_SCORE_THRESHOLD=2, PATH_TYPE_METRICS_FIELDS
Updated Domain-wide coverage markDeployedUrlSuggestionsAsCovered now also marks path suggestions as coveredByDomainWide
Removed isDomainWideSuggestionData from handler.js Moved to src/prerender/utils/utils.js

What was removed from the original PR (after plan change)

Removed Why
pathPattern field in suggestion data Not needed — allowedRegexPatterns[0] serves as the pattern
pathScore field Renamed to score to match plan schema
urlCount, valuableCount, valuablePercent, totalAgenticTraffic fields Removed from data shape per plan — not stored on the suggestion
avgContentGainRatio, totalWordCountBefore, totalWordCountAfter fields Renamed to contentGainRatio, wordCountBefore, wordCountAfter
PATH_TYPE_SUGGESTION_RANK constant Rank reverted to 0 for all suggestion types — no tiered ranking
Tiered rank logic (domain-wide=999999, path=100000, per-URL=0) Reverted to rank: 0 for all
Chunked saveMany loop with PATH_TYPE_METRICS_REFRESH_CHUNK_SIZE Replaced with single saveMany call + try-catch

Sequence

PR 3 of 4 — depends on spacecat-shared (PR 1) and spacecat-api-service (PR 2). project-elmo-ui (PR 4) depends on this.

Test plan

  • 45 unit tests for path-suggestions.js (100% coverage)
  • 44 unit tests for agentic-urls.js (100% coverage)
  • 56 unit tests for query-builder.js additions
  • Handler tests cover pathSuggestionsEnabled=true/false, domain-wide deployed skip, mapNewSuggestion, metrics refresh, buildMergeDataFunction, coveredByDomainWide for path suggestions
  • All tests pass, lint clean on all changed files

@codecov

codecov Bot commented May 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@github-actions

Copy link
Copy Markdown
Contributor

This PR will trigger a minor release when merged.

@MysticatBot-Dev MysticatBot-Dev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ssilare-adobe,

Strengths

  • Clean strategy pattern (src/prerender/path-suggestions.js:54-100): RcvPathQualificationStrategy with constructor-injectable thresholds is well-designed for testability and extensibility. The early-return pattern with clear rejection reasons makes debugging qualification failures straightforward.
  • Self-healing stale references (src/prerender/path-suggestions.js:264-284): Proactively clearing coveredByDomainWide refs pointing to undeployed path suggestions prevents orphaned coverage links from accumulating across re-audit cycles.
  • Graceful Athena degradation (src/prerender/path-suggestions.js:148-151, src/utils/agentic-urls.js:225-228): Catching Athena failures and falling back to an empty Map means path qualification still works (degraded to zero traffic weighting) rather than failing the audit.
  • Config-gated rollout (src/prerender/handler.js:1120): Defaulting pathSuggestionsEnabled to false is the right approach for a new scoring tier. Isolates blast radius while the feature stabilizes.
  • Thorough test coverage (test/audits/prerender/path-suggestions.test.js): 752 lines covering edge cases for invalid URLs, undefined fields, boundary conditions on qualification thresholds, and the self-heal flow.
  • No new attack surface: No new dependencies, no new endpoints, no auth changes. URL parsing is defensive throughout.

Issues

Important (Should Fix)

1. Path prefix matching lacks segment boundary check - potential incorrect coverage marking

src/prerender/path-suggestions.js:302:

const prefix = pathPat.replace('/*', '');
// ...
return new URL(s.getData().url).pathname.startsWith(prefix);

A path pattern /prod/* produces prefix /prod, which matches /products/item-1, /production/line-3, etc. The startsWith check does not enforce a path segment boundary.

Why it matters: A deployed /prod/* path suggestion would incorrectly mark /products/... and /production/... URLs as covered, suppressing them from future processing.

How to fix: Append a trailing slash to the prefix check:

return new URL(s.getData().url).pathname.startsWith(prefix + '/');

Since extractPathType always produces /<segment>/*, appending / ensures only /products/... URLs match, not /productsale/....

2. Redundant opportunity.getSuggestions() calls in a single audit run

When pathSuggestionsEnabled=true, getSuggestions() is called:

  • Once in findPreservablePathSuggestions (path-suggestions.js:123)
  • Once in buildPathTypeSuggestions (path-suggestions.js:159)
  • Once in markSuggestionsAsCoveredByPaths (path-suggestions.js:280)
  • Plus the existing call in findPreservableDomainWideSuggestion

Each call is a database round-trip fetching the same collection. For sites with hundreds of suggestions, this is 3-4x the latency and load.

How to fix: Fetch once in the handler and pass the array down. The first three functions already operate on the in-memory list; they can accept an optional existingSuggestions parameter. The fourth call (after syncSuggestions) legitimately needs fresh data and should remain separate.

3. O(n^2) lookup in stale detection logic

src/prerender/path-suggestions.js:274:

const ref = suggestions.find((r) => r.getId() === covId);

This find() is called inside a filter() over all suggestions. For n suggestions, this is O(n^2).

How to fix: Build a Map once before the filter:

const suggestionsById = new Map(suggestions.map((s) => [s.getId(), s]));
// Then: const ref = suggestionsById.get(covId);

4. /* c8 ignore */ directives suppress coverage on 28 lines of branching logic

The repo's CLAUDE.md states: "Coverage enforcement is strict: 100% lines, branches, and statements across all src/**/*.js. Every code path must be tested."

New c8 ignore directives in this PR:

  • handler.js:1222: /* c8 ignore next 6 */ on the rank ternary (3 branches)
  • handler.js:1229: /* c8 ignore next 22 */ on mergeDataFunction (3 code paths for path-type/domain-wide/individual merge)

The mergeDataFunction in particular handles data integrity during syncs (preserving edgeDeployed and coveredByDomainWide on path-type merges). This is exactly where data-loss bugs hide.

How to fix: Extract mergeDataFunction into a named, exported utility and test it directly with the three code paths. The rank ternary is also testable via the existing handler test patterns.

5. Rank assignment uses hardcoded magic number

src/prerender/handler.js:1225:

rank: suggestion.key && suggestion.data?.pathType
  ? PATH_TYPE_SUGGESTION_RANK
  : (suggestion.key ? 999999 : 0),

PATH_TYPE_SUGGESTION_RANK is properly named, but 999999 (domain-wide) and 0 (per-URL) are magic numbers. If a future suggestion type adds .key without .data.pathType, it silently gets rank 999999.

How to fix: Define DOMAIN_WIDE_SUGGESTION_RANK = 999999 as a named constant alongside PATH_TYPE_SUGGESTION_RANK, and add a brief comment explaining the ranking hierarchy (per-URL < path < domain-wide).

Minor (Nice to Have)

6. coveredByDomainWide field overloading creates semantic ambiguity

The field now holds either a domain-wide suggestion ID or a path suggestion ID. Disambiguation relies on looking up the referenced suggestion and checking pathType === true. If a referenced suggestion is deleted, the self-heal logic cannot distinguish a stale path ref from a stale domain-wide ref.

Not blocking for this PR, but worth tracking as tech debt. A dedicated field or structured { type, id } value would be more robust long-term.

7. extractPathType locale-prefix limitation

For sites structured as /en/products/..., /en/blog/..., all content groups under /en/*. The qualification thresholds will likely pass (large URL count), but the resulting path suggestion is overly coarse. Worth documenting as a known limitation in a code comment.

8. Unused log parameter in RcvPathQualificationStrategy.qualify()

src/prerender/path-suggestions.js:69: The log parameter is accepted but never used (has an eslint-disable for unused vars). Either use it or remove it from the signature.

Recommendations

  • Document the suggestion-type taxonomy: There are now three suggestion shapes flowing through syncSuggestions (per-URL, path-type, domain-wide). A brief comment block at the top of the sync section enumerating the three types and their identifying traits would help future maintainers.
  • Consider a pathPattern uniqueness assertion: The code uses pathPattern as the preservation key. If two suggestions somehow have the same pathPattern, the Map constructor silently drops the earlier one. A debug log would surface data integrity issues early.

Assessment

Ready to merge? With fixes

The overall architecture is sound - the three-tier suggestion model (per-URL, path, domain-wide) is a natural hierarchy, the qualification strategy is well-abstracted, and the operational safety (opt-in flag, graceful degradation, self-healing) reflects mature thinking. However, the path prefix matching bug (item 1) is a correctness issue that could cause incorrect coverage suppression, and the c8-ignore on merge logic (item 4) leaves important data-integrity code paths untested.

Next Steps

  1. Fix the path prefix matching to enforce segment boundaries (item 1) - this is a correctness issue.
  2. Remove the c8 ignore directives and add tests for the merge and rank logic (item 4).
  3. Address the O(n^2) lookup (item 3) and redundant DB calls (item 2) before enabling on sites with large suggestion sets.
  4. Consider naming the domain-wide rank constant (item 5).

Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 30s | Cost: $6.60 | Commit: d91b6bc79cfe42f2ff57197c31f25288410e958f
If this code review was useful, please react with 👍. Otherwise, react with 👎.

@anuj-adobe anuj-adobe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #2500 Review — Path-Level Prerender Suggestions (audit-worker)

Reviewer context: This review cross-references the CLAUDE.md invariants for the prerender audit, the UI PR #1719 brainmap (cross-PR data contract), and the Testing Plan wiki.


Summary

This PR implements the audit-worker half of the path-level prerender feature: buildPathTypeSuggestions groups per-URL findings by first path segment, qualifies groups by traffic/score thresholds, and feeds them into syncSuggestions alongside the existing domain-wide and per-URL flows.

The architecture is sound and the main logic is clear. However, there are 3 critical N+1 bugs that will exhaust the 200-connection DB pool under production load, 1 critical stale-ref gap that permanently hides URLs from the UI, and a type mismatch in allSuggestions that silently corrupts path suggestion data.


🔴 Critical Issues

1. Three N+1 patterns (CLAUDE.md §"Never do this")

The CLAUDE.md invariants explicitly prohibit Promise.all(items.map(s => s.save())). This PR introduces three violations:

  • path-suggestions.js:315 — stale coveredByDomainWide clear
  • path-suggestions.js:354 — per-path toCover.map(s => s.save()) inside a for...of loop (compounding N×M)
  • handler.js:1268 — metrics refresh on preserved paths

All three must use Suggestion.saveMany() per the established pattern.

2. Self-heal stale ref gap (path-suggestions.js:311)

When a path suggestion is deleted (OUTDATED/purged), its ID remains in coveredByDomainWide on per-URL suggestions. The stale ref is never cleared because ref && ref.getData()... returns false when ref is undefined. Those URLs are permanently invisible to users. Fix: !ref || (ref.getData()?.pathType === true && ...).

3. preservablePaths DB entities break syncSuggestions key mapping (handler.js:1338)

DB entities don't have .key or .url as direct properties — buildKey(entity) returns undefined. All preserved path entities collide on key undefined, causing mergeDataFunction to call mapSuggestionData(entity) on a DB entity rather than a raw audit object. This writes url: undefined, contentGainRatio: undefined back to path suggestion data. Fix: wrap preserved paths as { key: s.getData().pathPattern, data: s.getData() } before adding to allSuggestions.


🟡 Should Fix

4. markSuggestionsAsCoveredByPaths runs unconditionally (handler.js:1361)

When pathSuggestionsEnabled = false, this still fetches all suggestions from DB and iterates them. Move inside the if (pathSuggestionsEnabled) block.

5. buildMergeDataFunction branch order fragility (handler.js:1146)

A path suggestion with data.pathType falsy silently falls through to the domain-wide branch (replaces all data, drops edgeDeployed/coveredByDomainWide). Tighten the guard to data.pathType === true.

6. Path matching misses exact root segment (path-suggestions.js:345)

pathname.startsWith('/products/') never matches https://example.com/products (exact segment, no trailing slash). Add || pathname === prefix.


🟢 Design Notes (no change needed)

  • ELIGIBLE_STATUSES includes FIXED (path-suggestions.js:29): Already-deployed paths contributing to scoring can inflate counts. The comment at lines 137-138 says "only NEW or FIXED per-URL suggestions are eligible" — if FIXED inclusion is intentional, confirm this is for regression detection after rollback.
  • Locale prefix limitation (path-suggestions.js:36-37): Documented in the code. The comment correctly flags that /en/*, /fr/* groupings are known false-positives. Acceptable as a known limitation.
  • CLAUDE.md invariants checked: scrapeJobId write-once ✅, isDomainBlocked hard stop ✅, domain-wide preservation (findPreservableDomainWideSuggestion) ✅, scrapedUrlsSet excludes edge-deployed ✅, OUTDATED protection via scrapedUrlsSet guard ✅ (path suggestions have no .url so can't be OUTDATEd by the guard — both intentional and necessary for the preservation logic to work).

Cross-PR Contract Notes (from UI PR #1719 analysis)

The wiki testing plan (P0 test #14) documents:

"There is no pathType field in the API response — the hook derives it client-side as Array.isArray(data.allowedRegexPatterns) && !data.isDomainWide"

This confirms the UI's pathType inference from allowedRegexPatterns is intentional at the API contract level (even though the audit worker does set pathType: true in the stored data, it may not be forwarded by the API service). The risk flagged in the UI review (B2 — pathType inferred, not read directly) stands, but it's an API-service concern, not this PR.

The coveredByDomainWide field is used by this PR to store path suggestion IDs (audit-time coverage hint). The coveredByPattern field is written by tokowaka-client (PR #1598) for deploy-time coverage. These are distinct lifecycle moments — the naming collision is a known tech debt (tracked in the inline comment at path-suggestions.js:282).

Comment thread src/prerender/features/path-suggestions/path-suggestions.js Outdated
// Only clear refs that point to a path suggestion that is no longer deployed
const ref = suggestionsById.get(covId);
return ref && ref.getData()?.pathType === true && !deployedPathIds.has(covId);
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Critical — Self-heal stale ref gap: deleted suggestions leave permanent coveredByDomainWide pointers

const ref = suggestionsById.get(covId);
return ref && ref.getData()?.pathType === true && !deployedPathIds.has(covId);

When ref is undefined (the referenced path suggestion was deleted — e.g. marked OUTDATED and purged), the condition returns false. The per-URL suggestion's coveredByDomainWide field is never cleared, so that URL is permanently hidden from the Current tab — even though the path suggestion it referenced no longer exists.

The comment below (line 282-283) already documents this limitation but the fix is straightforward:

// If ref is undefined, it's a dangling reference — always clear it
return !ref || (ref.getData()?.pathType === true && !deployedPathIds.has(covId));

This makes the self-heal also clean up dangling refs where the pointed-to suggestion was deleted.

Comment thread src/prerender/path-suggestions/path-suggestions.js Outdated
Comment thread src/prerender/features/path-suggestions/path-suggestions.js Outdated
Comment thread src/prerender/features/path-suggestions/path-suggestions.js Outdated
Comment thread src/prerender/handler.js
Comment thread src/prerender/handler.js Outdated
Comment thread src/prerender/handler.js
Comment thread src/prerender/handler.js

@anuj-adobe anuj-adobe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KISS / DRY / YAGNI Review — PR #2500

This pass focuses on code design principles. The bugs flagged in the first review stand independently — these are separate concerns about complexity, duplication, and over-engineering.


Summary of findings

Principle Finding Location
YAGNI RcvPathQualificationStrategy exported class with pluggable interface — no second strategy exists path-suggestions.js:57
DRY new URL(...).pathname.replace(...) repeated 4× in 18 lines — toPathname() already exists path-suggestions.js:177–209
DRY totalAgenticTraffic + avgContentGainRatio recomputed after qualify() already computed them path-suggestions.js:240
DRY+KISS getAgenticHitsMapFromAthena duplicates the entire Athena guard/setup from runTopAgenticUrlsQuery agentic-urls.js:180
KISS 4-week window built via 4× generateReportingPeriods calls to extract 2 dates agentic-urls.js:203
DRY builtSuggestions filtered twice with the same predicate handler.js:1268+1289
KISS Nested ternary for rank requires eslint-disable; magic 999999 not named handler.js:1350

Not flagged as a violation (design is fine as-is)

  • buildMergeDataFunction factory: justified — the merged function has three distinct cases that must share mapSuggestionDataFn via closure. Acceptable complexity.
  • getTopAgenticUrlsFromAthena wrapper around runTopAgenticUrlsQuery: this is a thin delegation with clear naming. The asymmetry is that getAgenticHitsMapFromAthena does NOT delegate to the private helper — it duplicates it instead. Fix the latter, not the former.

Comment thread src/prerender/path-suggestions.js Outdated
Comment thread src/prerender/features/path-suggestions/path-suggestions.js Outdated
Comment thread src/prerender/path-suggestions.js Outdated
Comment thread src/utils/agentic-urls.js
Comment thread src/utils/agentic-urls.js Outdated
Comment thread src/prerender/handler.js Outdated
Comment thread src/prerender/handler.js Outdated

@anuj-adobe anuj-adobe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Monitorability & Test Correctness Review — PR #2500


Logging Gaps

The new code follows the established LOG_PREFIX + log.{level} pattern correctly. The gaps are about observability coverage — specific code paths that fire no log, making debugging in production harder.

Location Gap
path-suggestions.js:265-270 Skipped paths are logged but qualified paths are not — only the final count is logged
path-suggestions.js:324 deployedPaths.length === 0 fast-path exits silently — no debug log confirming normal completion
handler.js:1268-1287 Metrics refresh loop: no log for how many preserved paths were actually updated vs. unchanged

Test Issues

Two tests have wrong or misleading assertions:

  1. handler.test.js:4433 — Asserts markSuggestionsAsCoveredByPaths is called when pathSuggestionsEnabled=false. This confirms (and silently approves) the unconditional call bug from the main review. Either document why this is intentional, or fix the call site and flip the assertion to notCalled.

  2. handler.test.js:4556 — No test for the if (changed) guard — the changed=false branch (save skipped when metrics are unchanged) is untested.

One test has a dead variable (staleRef = { ...{} } on line 699).


Correction: Three inline comments in this review originally recommended asserting log.info/log.debug message strings in tests. Those are retracted — log strings are implementation details, not behaviour. The existing state assertions (save.calledOnce, field value checks, stub notCalled) already cover the behaviour those logs describe. Asserting exact log wording makes tests brittle without adding safety. The logging gaps above are source-level concerns only.

Comment thread src/prerender/path-suggestions/main.js
Comment thread src/prerender/path-suggestions/path-suggestions.js Outdated
Comment thread src/prerender/handler.js Outdated
Comment thread test/audits/prerender/handler.test.js
Comment thread test/audits/prerender/handler.test.js
Comment thread test/audits/prerender/handler.test.js
Comment thread test/audits/prerender/path-suggestions.test.js
Comment thread test/audits/prerender/path-suggestions.test.js
Comment thread test/audits/prerender/path-suggestions.test.js

@MysticatBot MysticatBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ssilare-adobe,

Verdict: Approve - prior blocking findings addressed; code is well-structured with proper error handling and comprehensive test coverage.
Complexity: HIGH - large diff (3633 lines, 13 files); SQL/Database signal + API surface.
Changes: Adds path-level prerender suggestion generation as a third tier between per-URL and domain-wide, with qualification strategy, coverage marking, metrics refresh, and self-healing of stale references (13 files).

Non-blocking (3): minor issues and suggestions
  • nit: createTopUrlsWithHitsQuery is a zero-logic passthrough that delegates to createTopUrlsQueryWithLimit unchanged - consider exporting createTopUrlsQueryWithLimit directly under both names or documenting why the indirection exists for future readers - src/cdn-logs-report/utils/query-builder.js:261
  • suggestion: refreshPreservedPathMetrics logs "Metrics refreshed on N/M" unconditionally after the try/catch on saveMany, so when saveMany throws, the error log is followed by a success-looking info message - move the final log.info inside the if (toSave.length > 0) block after the successful save, or add an else for the zero-save case only - src/prerender/path-suggestions/main.js:439
  • suggestion: Consider adding an integration-level test that exercises resolvePathSuggestions with enough URLs to actually trigger path suggestion generation (current test uses 5 URLs but PATH_TYPE_MIN_URLS=10) to prove the end-to-end flow produces suggestions

Previously flagged, now resolved

  • Case-sensitivity mismatch in agenticHitsMap lookup fixed (both toPathname and getAgenticHitsMapFromAthena now lowercase consistently)
  • Duplicate Athena query eliminated (hits map fetched once in handler, passed as parameter)
  • CI coverage failure resolved (all checks passing)

Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 3m 49s | Cost: $2.76 | Commit: 5ac7b6dd5b6ea9a262833c0d0e57c8ebd5ccf5c4
If this code review was useful, please react with 👍. Otherwise, react with 👎.

@anuj-adobe

Copy link
Copy Markdown
Contributor

are you planning to add any behaviour tests or move ahead without it?

@ssilare-adobe

Copy link
Copy Markdown
Contributor Author

are you planning to add any behaviour tests or move ahead without it?

Added, please check.

@anuj-adobe anuj-adobe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image

It seems we are adding 3 new files for path suggestions and two seems to be added with redundant names.
The regression one should be removed and instead added as normal behaviour tests file by proper grouping.

@MysticatBot MysticatBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ssilare-adobe,

Verdict: Approve - prior blocking findings remain resolved; latest commits are well-structured test additions and directory consolidation.
Complexity: HIGH - large diff (4561 lines, 23 files); API surface + Database signal.
Changes: Adds path-level prerender suggestion generation as a third tier between per-URL and domain-wide, with qualification strategy, coverage marking, metrics refresh, and self-healing (23 files).

Non-blocking (5): minor issues and suggestions
  • suggestion: Thread the handler's pre-fetched suggestions array (handler.js line 903) into resolvePathSuggestions to avoid 2 redundant opportunity.getSuggestions() DB round-trips per audit run. findPreservablePathSuggestions and buildPathTypeSuggestions both call it internally; the existing suggestions option on buildPathTypeSuggestions already supports this - src/prerender/path-suggestions/main.js:391,432
  • suggestion: resolvePathSuggestions integration test uses 5 URLs but default PATH_TYPE_MIN_URLS=10, so qualification never fires and the test passes vacuously. Either increase URLs to >= 10 or inject a low-threshold qualifier to prove the positive path actually produces suggestions - test/audits/prerender/path-suggestions.test.js
  • suggestion: Test aiReadablePercent with non-zero values (supply aiReadable: true on some URL fixtures) to prove the percentage formula produces correct output beyond the trivial 0% case - src/prerender/path-suggestions/main.js:495
  • suggestion: Add try/catch around new URL(site.getBaseURL()) in buildPathTypeSuggestions for defensive resilience against malformed DB entries. Every other URL parse in the new code is wrapped; this is the sole unprotected call - src/prerender/path-suggestions/main.js:457
  • nit: createTopUrlsWithHitsQuery is a zero-logic passthrough that delegates to createTopUrlsQueryWithLimit unchanged. Either call the original directly or document why the named entry point is reserved for future divergence - src/cdn-logs-report/utils/query-builder.js:261

Previously flagged, now resolved

  • Case-sensitivity mismatch in agenticHitsMap lookup fixed (both toPathname and getAgenticHitsMapFromAthena now lowercase consistently)
  • Duplicate Athena query eliminated (hits map fetched once in handler, passed as parameter)
  • CI coverage failure resolved (all checks passing)

Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 2m 17s | Cost: $7.25 | Commit: e7393f18c2398c8c5fe972e9d1bffdca8c1da07d
If this code review was useful, please react with 👍. Otherwise, react with 👎.

@ssilare-adobe ssilare-adobe merged commit 0f7600b into main Jul 6, 2026
18 checks passed
@ssilare-adobe ssilare-adobe deleted the feat/path-level-prerender-suggestions branch July 6, 2026 04:37
solaris007 pushed a commit that referenced this pull request Jul 6, 2026
# [1.496.0](v1.495.1...v1.496.0) (2026-07-06)

### Features

* generate path-level prerender suggestions in audit worker ([#2500](#2500)) ([0f7600b](0f7600b))
@solaris007

Copy link
Copy Markdown
Member

🎉 This PR is included in version 1.496.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

ssilare-adobe added a commit to adobe/spacecat-api-service that referenced this pull request Jul 6, 2026
## Summary

- Adds `path-suggestions {enable/disable} {site}` Slack command to
toggle path-level prerender suggestion generation for a site. This sets
`pathSuggestionsEnabled` in the site's `handlers.prerender` config,
which is read by the audit worker
([spacecat-audit-worker#2500](adobe/spacecat-audit-worker#2500)).
- Adds `get path-suggestions {site}` Slack command to check whether
path-level suggestions are currently enabled for a site.
- Both commands support site lookup by base URL or site ID.

## Test plan

- [x] Unit tests for `toggle-path-suggestions` command (11 tests)
- [x] Unit tests for `get-path-suggestions-status` command (9 tests)
- [ ] Manual Slack test: `path-suggestions enable <site>` then verify
config via `get path-suggestions <site>`
- [ ] Manual Slack test: `path-suggestions disable <site>` then verify
disabled status
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants