feat: add Semrush-backed URL Inspector stats endpoint

[aniham]

Summary

• Adds two Serenity/Semrush-backed endpoints for the url-inspector-sr dashboard's 4 KPI cards, replacing their legacy HLX fetch, matching the response shape of the existing Aurora/Postgres reference endpoint (docs/llmo-brandalf-apis/url-inspector-stats-api.md):
  • GET /v2/orgs/:spaceCatId/brands/:brandId/serenity/brand-presence/url-inspector/stats — 3 of the 4 cards (unique cited URLs, total citations, unique prompts cited) + weekly sparklines, backed by the Semrush Stats-per-URL element.
  • GET /v2/orgs/:spaceCatId/brands/:brandId/serenity/brand-presence/url-inspector/prompts/count — the 4th card (total prompts), backed by the existing getPrompts service method (shared with listPrompts), no date range.
• Split from a single combined endpoint after review/testing surfaced timeouts: the original single-endpoint design fanned out Stats-per-URL per Semrush project per trend week (up to 8 weeks x N projects) to compute 3 of the 4 cards, and repeatedly timed out once the requested date range exceeded ~2 weeks. totalPrompts is a single, unscoped, always-fast Semrush call that was just waiting on the slow ones — splitting it into its own endpoint lets the client fetch it independently/in parallel instead of blocking on the fan-out.
  • The other 3 cards stay bundled in one endpoint (not further split into 3) since they're all derived from a single Stats-per-URL response — splitting them further would triple the Semrush call volume for no latency benefit, since a client wanting all 3 needs the same underlying rows either way.
• Known approximation gap (unchanged from before): totalPromptsCited sums a per-URL count (Semrush exposes no distinct prompts-cited element, so a prompt citing multiple owned URLs is overcounted).
• Known, PERMANENT limitation: totalPrompts has no weekly trend, unlike the Aurora/Postgres reference endpoint (which returns it via rpc_url_inspector_total_prompts, a time-windowed count of distinct active prompts that ran each week). The Semrush PROMPTS element backing /url-inspector/prompts/count has no date parameter at all — it's a static, currently-configured prompt roster, not an execution log — so there is no upstream data to bucket by week for this metric. This is why totalPrompts was split out of /url-inspector/stats into its own endpoint in the first place (an earlier version repeated the all-time total in every weeklyTrends entry, which the first review round correctly flagged as misleading); a dedicated, trend-less endpoint was chosen over fabricating a per-week series. Documented in the controller docstring (getUrlInspectorPromptsCount) and the SerenityUrlInspectorPromptsCount OpenAPI schema. The consuming UI (adobe/project-elmo-ui#2479) already accounts for this: the "Total Prompts" KPI card renders as a static number with no sparkline, unlike its 3 siblings.
• Corresponding UI change: adobe/project-elmo-ui#2479.

Test plan

• npx mocha test/controllers/elements.test.js test/support/elements/elements-service.test.js test/support/elements/definitions/url-inspector-stats.test.js test/openapi-contract/serenity-api.test.js test/routes/index.test.js test/routes/facs-capabilities.test.js test/routes/required-capabilities.test.js — all pass
• npm run docs:lint / npm run docs:build — spec valid, docs regenerated
• npx eslint on all touched files — clean
• Manual verification against a real Semrush-provisioned brand

🤖 Generated with Claude Code

[github-actions]

This PR will trigger a minor release when merged.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[MysticatBot]

Hey @aniham,

Verdict: Request changes - correctness bugs in untested controller logic and missing project conventions.
Complexity: HIGH - large diff, API surface change.
Changes: Adds a Semrush-backed GET endpoint for URL Inspector stats KPI cards with per-week sparkline trends (11 files).

Note: CI checks are currently pending - resolve before merge.

Must fix before merge

1. [Critical] Date defaulting silently discards caller's explicit partial date - src/controllers/elements.js

   If a caller passes only startDate (or only endDate), both are overwritten with the default 28-day window. The caller's explicit value is silently discarded. Compare with getStats which defaults each independently. Fix: if (!hasText(startDate)) startDate = defaultRange.startDate; if (!hasText(endDate)) endDate = defaultRange.endDate;

2. [Important] projects and projectIds derived from different sources can diverge - src/controllers/elements.js

   projects comes from getOwnedUrlProjects (filters via Markets element), while projectIds comes from brandSemrushProjects.map(p => p.semrushProjectId) (DB rows). If a project exists in DB but not in Markets (or vice versa), citation KPIs and totalPrompts will be scoped to different project sets. Fix: derive projectIds from the resolved projects array: projectIds = projects.map(p => p.projectId).filter(hasText);

3. [Important] totalPrompts repeated in every weeklyTrends entry is semantically misleading - src/support/elements/elements-service.js

   The PROMPTS element has no date filter, so this value is the all-time total. Consumers reading weeklyTrends[i].totalPrompts will naturally interpret it as "prompts in that week." Once this response shape becomes a contract, fixing it is a breaking change. Options: (a) omit totalPrompts from weekly entries entirely (keep only in top-level stats), or (b) rename to totalPromptsAllTime within trend objects.

4. [Important] Missing OpenAPI spec - CLAUDE.md "OpenAPI First" is must-follow pattern for new endpoints; no docs/openapi/paths/*.yaml added.

5. [Important] No controller-level test coverage - stubs added at test/controllers/elements.test.js but zero test scenarios exercise the ~112 lines of controller branching logic (6 early returns, 2 defaulting paths, region scoping). The sibling getStats controller has 20+ dedicated test cases covering the same patterns.

Non-blocking (2): minor issues and suggestions

• nit: Controller pre-merges model || platform into one field before passing to service, diverging from getStats which passes them separately; service platform param is always undefined - src/controllers/elements.js
• suggestion: region accepts no aliases (regionCode/region_code) unlike sibling getStats endpoint - consider adding for API consistency - src/controllers/elements.js

Out of scope, worth tracking: Fan-out cost (up to 9xN Semrush calls per page load for N projects) is author-acknowledged. Consider a project-count cap when moving past POC.

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 5m 31s | Cost: $8.57 | Commit: 32f0fd34cbd6728ca27158d597d6311796fd95fd
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[calvarezg]

Multi-Persona Review — Changes recommended

Prior MysticatBot review (commit 32f0fd34): all 5 findings verified resolved in the current diff (independent date defaulting, unified project scoping, totalPrompts split out, OpenAPI added, controller tests added). 👍

No blockers. 5 Major items to address or waive before merge:

1. [Major] Duplicated scaffolding across the two handlers (high confidence)
getUrlInspectorStats and getUrlInspectorPromptsCount copy-paste ~30–40 lines of auth / siteId cross-check / region→projectId resolution / empty-projects 404 guard. Two copies of security-relevant logic that will drift — the prompts/count copy already omits the 503-no-PostgREST and site_id-alias tests. Extract a shared resolveBrandProjectScope(ctx, auth, query) + crossCheckSiteId(...). This one fix also resolves #2 and #5 at the source.

2. [Major] platform silently mis-handled on /url-inspector/stats (medium confidence)
Stats controller sends model: query.model || query.platform and no platform key; the service destructures platform (always undefined here). Sibling /prompts/count passes model/platform separately. So ?platform=chatgpt scopes the two KPI cards on the same dashboard differently. Uncovered by tests. Normalize once and pass consistently; add a platform-only test. (This was a still-open MysticatBot nit.)

3. [Major] Nested concurrency — real in-flight ceiling is weekConcurrency × 8, not the flat 8 the comment implies (medium confidence)
mapWithConcurrency(weeks, …) wraps mapWithConcurrency(scopes, 8, …). On the endpoint that was split out for timeouts/rate limits, this bursts higher against Semrush than documented. Flatten to a single (project × week) task list under one bound, or verify the product is within budget.

4. [Major] All-or-nothing on up to 9×N upstream calls (high confidence)
One transient failure among the fan-out calls rejects the whole request → 502; failure probability scales with markets × weeks. Consider per-project settle-and-skip with a partial-result flag, or bounded retry.

5. [Major] Stats fan-out iterates unfiltered projects but the empty-scope guard checks filtered projectIds (low confidence — reachability unverified)
A mixed array [{projectId:'a'}, {region:'b'}] passes the projectIds.length === 0 guard yet issues a projectId: undefined (workspace-wide, cross-brand) fetch for the second scope. /prompts/count avoids this by using the filtered set. Reachable only if getOwnedUrlProjects can return an entry without projectId (not in this diff) — but harden anyway: fan out over projects.filter(p => hasText(p.projectId)).

Minor: untested missing-domain_type row + partial-range/empty-week trend; /prompts/count model/platform passthrough unasserted.

Question: Is folding platform into model on the stats endpoint intentional given /prompts/count keeps them separate?

Working well: prior fixes genuinely landed with tests, AuthZ correct and tested (authorizeOrg first, siteId ownership check, 503 on missing client), honest docstrings on the totalPromptsCited overcount, thorough date validation.

_{Multi-persona review (Architect · DBA · Security · Tester · Junior Dev). Review-only; no code changed.}

[calvarezg]

Well howdy @aniham — this PR's been through the rodeo and came out with its boots still shined. 🤠

All five Major findings roped, hog-tied, and covered with tests: shared resolveUrlInspectorScope helper, model/platform no longer wearing each other's hats, the fan-out concurrency flattened to one honest bound, per-scope failures settling to a partial flag instead of stampeding the whole request, and the unfiltered-projects edge fenced off. Bonus points for chasing the real 30s Semrush timeout and documenting the gateway ceiling like a proper trail boss.

Everything's bigger in Texas, and so's this changeset — nicely done. Approving. 🌵

Yeehaw and ship it. 🐎

[solaris007]

🎉 This PR is included in version 1.681.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀