feat: add content types in SR URL Inspector Filters response#2884
feat: add content types in SR URL Inspector Filters response#2884vivesing wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Hey @vivesing,
⚠ Degraded review - no spec document was found for this change (searched the PR links, the touched repos' docs, the architecture/guidelines docs, and linked Jira). This review covers code-level quality but could not validate the change against an agreed design, so confidence is reduced. Add a spec link (PR template section 4) and re-request review for a full-confidence pass.
Verdict: Request changes - OpenAPI schema must be updated to reflect the new content_types response field.
Complexity: HIGH - medium diff; API surface change.
Changes: Adds a content_types filter dimension to the URL Inspector endpoint by wiring a new Semrush element fetch in parallel with the existing ones (7 files).
Must fix before merge
- [Important] OpenAPI schema
SerenityUrlInspectorFilterDimensionsindocs/openapi/schemas.yamldoes not include the newcontent_typesproperty -src/support/elements/elements-service.js:103(details inline)
Non-blocking (4): minor issues and suggestions
- nit:
idderivation from Semrush label (label.toLowerCase().replace(/\s+/g, '_')) means upstream label changes silently change IDs that consumers may persist. Consider a static mapping for known values with fallback derivation for unknowns -src/support/elements/definitions/content-types.js:98 - suggestion: Use
sinon.useFakeTimersin the "defaults to a rolling 28-day window" test to prevent rare flakiness near day/month boundaries -test/support/elements/definitions/content-types.test.js:56 - nit:
DEFAULT_WINDOW_DAYS = 28duplicates a constant fromllmo-brand-presence.js/cited-domains.js; shared extraction would prevent drift -src/support/elements/definitions/content-types.js:18 - suggestion: Add a test for
buildContentTypesPayload({ model: 'perplexity', platform: 'openai' })to document thatmodeltakes precedence overplatform-test/support/elements/definitions/content-types.test.js
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 4m 53s | Cost: $5.51 | Commit: 42a15fbe3d46d62b87ec9ced927c0ecbd34acabb
If this code review was useful, please react with 👍. Otherwise, react with 👎.
| categories: transformCategoriesToFilterDimensions(rawTopics), | ||
| page_intents: transformIntentsToFilterDimensions(rawTopics), | ||
| origins: transformOriginsToFilterDimensions(rawTopics), | ||
| content_types: transformContentTypesToFilterDimensions(rawContentTypes), |
There was a problem hiding this comment.
issue (blocking): The CLAUDE.md rule "Specification Sync: Keep OpenAPI specs and implementation in sync" requires that this response-shape change be reflected in the OpenAPI schema.
The SerenityUrlInspectorFilterDimensions schema in docs/openapi/schemas.yaml lists required: [brands, regions, topics, categories, page_intents, origins, tags] and its properties block does not include content_types. After this PR, the implementation returns a field the spec does not declare.
Fix: Add content_types to both the required array and the properties section of SerenityUrlInspectorFilterDimensions in docs/openapi/schemas.yaml, following the same type: array, items: object pattern as the sibling dimensions. Then run npm run docs:build to regenerate.
Summary: Add
content_typesfilter dimension to URL InspectorContext
GET /v2/orgs/:orgId/brands/:brandId/serenity/brand-presence/url-inspector/filter-dimensionspreviously fetched 3 Semrush elements (
TOPICS,BRANDS,MARKETS) and returnedbrands,regions,topics,categories,page_intents,origins, plus dynamic tag groups. A newSemrush element (
d1f9e6ec-9128-4b1b-b767-d86f93f54237) exposesdomain_typevalues(
Owned/Other/Social/Earned/Benchmark Competitors) and is now wired in as acontent_typesdimension.Files changed
src/support/elements/element-ids.jsELEMENT_IDS.CONTENT_TYPESUUID constantsrc/support/elements/definitions/content-types.jsbuildContentTypesPayload()(CBF_model + CBF_date range, default rolling 28-day window) andtransformContentTypesToFilterDimensions()(maps each label →{ id: slugified label, label })src/support/elements/definitions/index.jssrc/support/elements/elements-service.jsfetchElementcall forCONTENT_TYPES; addedcontent_typeskey togetUrlInspectorFilterDimensionsresultsrc/controllers/elements.jsBehavior
idis derived from the Semrush label (lowercased, spaces → underscores) — no hardcodedlookup table needed:
Owned→owned,Other→other,Social→social,Earned→earned,Benchmark Competitors→benchmark_competitorshow every other dimension in this endpoint behaves.
startDate/endDate/model/platformquery params already accepted by theendpoint.
Tests added
test/support/elements/definitions/content-types.test.js(new)buildContentTypesPayload: default/known/unknown model, platform alias, explicit dates, default 28-day window, payload shape (union,and, emptysimple).transformContentTypesToFilterDimensions: null/emptytest/support/elements/elements-service.test.jsRAW_CONTENT_TYPESfixture +ELEMENT_IDS.CONTENT_TYPESstub; bumpedcontent_typestded dedicated assertion on the transformedcontent_typesarraytest/controllers/elements.test.jsResult: 150/150 tests passing across the touched suites, lint clean.