From 94fbfd7b0f166ec93c5d609d29e502e2a3aae611 Mon Sep 17 00:00:00 2001 From: Tej Kotthakota Date: Thu, 2 Apr 2026 20:33:37 -0500 Subject: [PATCH 1/6] We only need to analyze top 3 pages for PLG customers --- src/controllers/suggestions.js | 37 +++++++++-- test/controllers/suggestions.test.js | 98 ++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 4 deletions(-) diff --git a/src/controllers/suggestions.js b/src/controllers/suggestions.js index 5a66d162a..ff91256c2 100644 --- a/src/controllers/suggestions.js +++ b/src/controllers/suggestions.js @@ -47,6 +47,11 @@ import { grantSuggestionsForOpportunity } from '../support/grant-suggestions-han const VALIDATION_ERROR_NAME = 'ValidationError'; +// Maximum number of CWV suggestions returned to PLG (sites-optimizer-ui) clients. +// The audit-worker already stores only failing-metric pages sorted by page views desc, +// so slicing to this limit gives the top N most-viewed pages with CWV issues. +const CWV_PLG_SUGGESTION_LIMIT = 3; + const GEO_EXPERIMENT_SCHEDULE = Object.freeze({ PRE_CRON_EXPRESSION: '0 * * * *', // hourly (fires immediately via triggerImmediately: true) // 5 minutes — only needs to live long enough for the immediate trigger @@ -223,6 +228,26 @@ function SuggestionsController(ctx, sqs, env) { } }; + /** + * For PLG (sites-optimizer-ui) clients, limits CWV suggestions to the top N entries. + * The suggestions are already ordered by page views descending from the audit-worker, + * so slicing gives the highest-traffic pages with failing metrics. + * Returns all suggestions unchanged for non-CWV types, non-PLG clients, or sites + * that do not have the summit-plg handler enabled. + * @param {Object} site - Site entity. + * @param {Array} entities - Suggestion entities to limit. + * @param {Object} opportunity - Opportunity entity (may be null if no suggestions). + * @param {Object} context - Request context. + * @returns {Promise} Limited suggestion entities. + */ + const applyPlgCwvLimit = async (site, entities, opportunity, context) => { + if (opportunity?.getType() !== 'cwv') return entities; + if (context.pathInfo?.headers?.['x-client-type'] !== 'sites-optimizer-ui') return entities; + const configuration = await Configuration.findLatest(); + if (!configuration?.isHandlerEnabledForSite('summit-plg', site)) return entities; + return entities.slice(0, CWV_PLG_SUGGESTION_LIMIT); + }; + /** * Gets all suggestions for a given site and opportunity * @param {Object} context of the request @@ -286,7 +311,8 @@ function SuggestionsController(ctx, sqs, env) { ); } const grantedEntities = await filterByGrantStatus(site, suggestionEntities, context); - const suggestions = grantedEntities.map( + const limitedEntities = await applyPlgCwvLimit(site, grantedEntities, opportunity, context); + const suggestions = limitedEntities.map( (sugg) => SuggestionDto.toJSON(sugg, view, opportunity), ); return ok(suggestions); @@ -345,7 +371,8 @@ function SuggestionsController(ctx, sqs, env) { } } const grantedEntities = await filterByGrantStatus(site, suggestionEntities, context); - const suggestions = grantedEntities.map( + const limitedEntities = await applyPlgCwvLimit(site, grantedEntities, opportunity, context); + const suggestions = limitedEntities.map( (sugg) => SuggestionDto.toJSON(sugg, view, opportunity), ); @@ -401,7 +428,8 @@ function SuggestionsController(ctx, sqs, env) { } } const grantedEntities = await filterByGrantStatus(site, suggestionEntities, context); - const suggestions = grantedEntities.map( + const limitedEntities = await applyPlgCwvLimit(site, grantedEntities, opportunity, context); + const suggestions = limitedEntities.map( (sugg) => SuggestionDto.toJSON(sugg, view, opportunity), ); return ok(suggestions); @@ -460,7 +488,8 @@ function SuggestionsController(ctx, sqs, env) { } } const grantedEntities = await filterByGrantStatus(site, suggestionEntities, context); - const suggestions = grantedEntities.map( + const limitedEntities = await applyPlgCwvLimit(site, grantedEntities, opportunity, context); + const suggestions = limitedEntities.map( (sugg) => SuggestionDto.toJSON(sugg, view, opportunity), ); return ok({ diff --git a/test/controllers/suggestions.test.js b/test/controllers/suggestions.test.js index 43f54b6a4..769f6ff87 100644 --- a/test/controllers/suggestions.test.js +++ b/test/controllers/suggestions.test.js @@ -800,6 +800,104 @@ describe('Suggestions Controller', () => { expect(mockLog.warn).to.have.been.calledWith('Grant suggestions handler failed', 'grant failure'); }); + it('limits CWV suggestions to top 3 for PLG (sites-optimizer-ui) clients', async () => { + const cwvSuggs = ['id-0', 'id-1', 'id-2', 'id-3'].map((id, i) => ({ + getId() { return id; }, + getOpportunityId() { return OPPORTUNITY_ID; }, + getType() { return 'CODE_CHANGE'; }, + getStatus() { return 'NEW'; }, + getRank() { return i; }, + getData() { return { type: 'url', url: `https://example.com/page${i}`, jiraLink: '' }; }, + getKpiDeltas() { return null; }, + getCreatedAt() { return '2024-01-01T00:00:00.000Z'; }, + getUpdatedAt() { return '2024-01-01T00:00:00.000Z'; }, + getSkipReason() { return null; }, + getSkipDetail() { return null; }, + getUpdatedBy() { return 'system'; }, + getOpportunity() { + return { getSiteId() { return SITE_ID; }, getType() { return 'cwv'; } }; + }, + })); + + mockSuggestionDataAccess.Suggestion.allByOpportunityId.resolves(cwvSuggs); + + // mockConfiguration already has isHandlerEnabledForSite('summit-plg', site) = true + const response = await suggestionsController.getAllForOpportunity({ + params: { siteId: SITE_ID, opportunityId: OPPORTUNITY_ID }, + pathInfo: { headers: { 'x-client-type': 'sites-optimizer-ui' } }, + ...context, + }); + expect(response.status).to.equal(200); + const result = await response.json(); + expect(result).to.be.an('array').with.lengthOf(3); + }); + + it('returns all CWV suggestions when summit-plg handler is not enabled for the site', async () => { + const cwvSuggs = ['id-0', 'id-1', 'id-2', 'id-3'].map((id, i) => ({ + getId() { return id; }, + getOpportunityId() { return OPPORTUNITY_ID; }, + getType() { return 'CODE_CHANGE'; }, + getStatus() { return 'NEW'; }, + getRank() { return i; }, + getData() { return { type: 'url', url: `https://example.com/page${i}`, jiraLink: '' }; }, + getKpiDeltas() { return null; }, + getCreatedAt() { return '2024-01-01T00:00:00.000Z'; }, + getUpdatedAt() { return '2024-01-01T00:00:00.000Z'; }, + getSkipReason() { return null; }, + getSkipDetail() { return null; }, + getUpdatedBy() { return 'system'; }, + getOpportunity() { + return { getSiteId() { return SITE_ID; }, getType() { return 'cwv'; } }; + }, + })); + + mockSuggestionDataAccess.Suggestion.allByOpportunityId.resolves(cwvSuggs); + // Override configuration to return false for summit-plg + mockSuggestionDataAccess.Configuration.findLatest.resolves({ + isHandlerEnabledForSite: () => false, + }); + + const response = await suggestionsController.getAllForOpportunity({ + params: { siteId: SITE_ID, opportunityId: OPPORTUNITY_ID }, + pathInfo: { headers: { 'x-client-type': 'sites-optimizer-ui' } }, + ...context, + }); + expect(response.status).to.equal(200); + const result = await response.json(); + expect(result).to.be.an('array').with.lengthOf(4); + }); + + it('returns all CWV suggestions when x-client-type is not sites-optimizer-ui', async () => { + const cwvSuggs = ['id-0', 'id-1', 'id-2', 'id-3'].map((id, i) => ({ + getId() { return id; }, + getOpportunityId() { return OPPORTUNITY_ID; }, + getType() { return 'CODE_CHANGE'; }, + getStatus() { return 'NEW'; }, + getRank() { return i; }, + getData() { return { type: 'url', url: `https://example.com/page${i}`, jiraLink: '' }; }, + getKpiDeltas() { return null; }, + getCreatedAt() { return '2024-01-01T00:00:00.000Z'; }, + getUpdatedAt() { return '2024-01-01T00:00:00.000Z'; }, + getSkipReason() { return null; }, + getSkipDetail() { return null; }, + getUpdatedBy() { return 'system'; }, + getOpportunity() { + return { getSiteId() { return SITE_ID; }, getType() { return 'cwv'; } }; + }, + })); + + mockSuggestionDataAccess.Suggestion.allByOpportunityId.resolves(cwvSuggs); + + // No sites-optimizer-ui header → all 4 returned + const response = await suggestionsController.getAllForOpportunity({ + params: { siteId: SITE_ID, opportunityId: OPPORTUNITY_ID }, + ...context, + }); + expect(response.status).to.equal(200); + const result = await response.json(); + expect(result).to.be.an('array').with.lengthOf(4); + }); + it('gets all suggestions for an opportunity and a site for non belonging to the organization', async () => { sandbox.stub(AccessControlUtil.prototype, 'hasAccess').returns(false); sandbox.stub(context.attributes.authInfo, 'hasOrganization').returns(false); From deb6ba8880c8964fc0204e24326b452e8f1cb4fc Mon Sep 17 00:00:00 2001 From: Tej Kotthakota Date: Fri, 3 Apr 2026 14:38:05 -0500 Subject: [PATCH 2/6] sort suggestions --- src/controllers/suggestions.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/controllers/suggestions.js b/src/controllers/suggestions.js index ff91256c2..bc79e76e5 100644 --- a/src/controllers/suggestions.js +++ b/src/controllers/suggestions.js @@ -229,9 +229,8 @@ function SuggestionsController(ctx, sqs, env) { }; /** - * For PLG (sites-optimizer-ui) clients, limits CWV suggestions to the top N entries. - * The suggestions are already ordered by page views descending from the audit-worker, - * so slicing gives the highest-traffic pages with failing metrics. + * For PLG (sites-optimizer-ui) clients, limits CWV suggestions to the top N entries + * by page views descending. * Returns all suggestions unchanged for non-CWV types, non-PLG clients, or sites * that do not have the summit-plg handler enabled. * @param {Object} site - Site entity. @@ -245,7 +244,9 @@ function SuggestionsController(ctx, sqs, env) { if (context.pathInfo?.headers?.['x-client-type'] !== 'sites-optimizer-ui') return entities; const configuration = await Configuration.findLatest(); if (!configuration?.isHandlerEnabledForSite('summit-plg', site)) return entities; - return entities.slice(0, CWV_PLG_SUGGESTION_LIMIT); + return [...entities] + .sort((a, b) => (b.getData()?.pageviews || 0) - (a.getData()?.pageviews || 0)) + .slice(0, CWV_PLG_SUGGESTION_LIMIT); }; /** From 6ab20709f6ba0f2f1bcba0da9b31e67d83245f96 Mon Sep 17 00:00:00 2001 From: Tej Kotthakota Date: Tue, 7 Apr 2026 13:23:55 -0500 Subject: [PATCH 3/6] add cwv sorting logic --- src/controllers/suggestions.js | 38 +------- src/support/grant-suggestions-handler.js | 19 ++++ test/controllers/suggestions.test.js | 96 ------------------- .../support/grant-suggestions-handler.test.js | 51 ++++++++++ 4 files changed, 74 insertions(+), 130 deletions(-) diff --git a/src/controllers/suggestions.js b/src/controllers/suggestions.js index bc79e76e5..5a66d162a 100644 --- a/src/controllers/suggestions.js +++ b/src/controllers/suggestions.js @@ -47,11 +47,6 @@ import { grantSuggestionsForOpportunity } from '../support/grant-suggestions-han const VALIDATION_ERROR_NAME = 'ValidationError'; -// Maximum number of CWV suggestions returned to PLG (sites-optimizer-ui) clients. -// The audit-worker already stores only failing-metric pages sorted by page views desc, -// so slicing to this limit gives the top N most-viewed pages with CWV issues. -const CWV_PLG_SUGGESTION_LIMIT = 3; - const GEO_EXPERIMENT_SCHEDULE = Object.freeze({ PRE_CRON_EXPRESSION: '0 * * * *', // hourly (fires immediately via triggerImmediately: true) // 5 minutes — only needs to live long enough for the immediate trigger @@ -228,27 +223,6 @@ function SuggestionsController(ctx, sqs, env) { } }; - /** - * For PLG (sites-optimizer-ui) clients, limits CWV suggestions to the top N entries - * by page views descending. - * Returns all suggestions unchanged for non-CWV types, non-PLG clients, or sites - * that do not have the summit-plg handler enabled. - * @param {Object} site - Site entity. - * @param {Array} entities - Suggestion entities to limit. - * @param {Object} opportunity - Opportunity entity (may be null if no suggestions). - * @param {Object} context - Request context. - * @returns {Promise} Limited suggestion entities. - */ - const applyPlgCwvLimit = async (site, entities, opportunity, context) => { - if (opportunity?.getType() !== 'cwv') return entities; - if (context.pathInfo?.headers?.['x-client-type'] !== 'sites-optimizer-ui') return entities; - const configuration = await Configuration.findLatest(); - if (!configuration?.isHandlerEnabledForSite('summit-plg', site)) return entities; - return [...entities] - .sort((a, b) => (b.getData()?.pageviews || 0) - (a.getData()?.pageviews || 0)) - .slice(0, CWV_PLG_SUGGESTION_LIMIT); - }; - /** * Gets all suggestions for a given site and opportunity * @param {Object} context of the request @@ -312,8 +286,7 @@ function SuggestionsController(ctx, sqs, env) { ); } const grantedEntities = await filterByGrantStatus(site, suggestionEntities, context); - const limitedEntities = await applyPlgCwvLimit(site, grantedEntities, opportunity, context); - const suggestions = limitedEntities.map( + const suggestions = grantedEntities.map( (sugg) => SuggestionDto.toJSON(sugg, view, opportunity), ); return ok(suggestions); @@ -372,8 +345,7 @@ function SuggestionsController(ctx, sqs, env) { } } const grantedEntities = await filterByGrantStatus(site, suggestionEntities, context); - const limitedEntities = await applyPlgCwvLimit(site, grantedEntities, opportunity, context); - const suggestions = limitedEntities.map( + const suggestions = grantedEntities.map( (sugg) => SuggestionDto.toJSON(sugg, view, opportunity), ); @@ -429,8 +401,7 @@ function SuggestionsController(ctx, sqs, env) { } } const grantedEntities = await filterByGrantStatus(site, suggestionEntities, context); - const limitedEntities = await applyPlgCwvLimit(site, grantedEntities, opportunity, context); - const suggestions = limitedEntities.map( + const suggestions = grantedEntities.map( (sugg) => SuggestionDto.toJSON(sugg, view, opportunity), ); return ok(suggestions); @@ -489,8 +460,7 @@ function SuggestionsController(ctx, sqs, env) { } } const grantedEntities = await filterByGrantStatus(site, suggestionEntities, context); - const limitedEntities = await applyPlgCwvLimit(site, grantedEntities, opportunity, context); - const suggestions = limitedEntities.map( + const suggestions = grantedEntities.map( (sugg) => SuggestionDto.toJSON(sugg, view, opportunity), ); return ok({ diff --git a/src/support/grant-suggestions-handler.js b/src/support/grant-suggestions-handler.js index 91ce5b388..80454d13f 100644 --- a/src/support/grant-suggestions-handler.js +++ b/src/support/grant-suggestions-handler.js @@ -98,6 +98,25 @@ const OPPORTUNITY_STRATEGIES = { ); }, }, + // Sorts CWV suggestions by pageviews descending so highest-traffic pages + // are granted first, giving users the most impactful fixes first. + // Tie-breaks by suggestion ID ascending for deterministic ordering. + cwv: { + sortFn: (groupA, groupB) => { + const getPageviews = (group) => { + const s = group.items[0]; + const data = typeof s?.getData === 'function' ? s.getData() : s?.data; + return data?.pageviews ?? 0; + }; + const pvDiff = getPageviews(groupB) - getPageviews(groupA); + if (pvDiff !== 0) return pvDiff; + const a = groupA.items[0]; + const b = groupB.items[0]; + const idA = typeof a?.getId === 'function' ? a.getId() : (a?.id ?? ''); + const idB = typeof b?.getId === 'function' ? b.getId() : (b?.id ?? ''); + return idA.localeCompare(idB); + }, + }, }; /** diff --git a/test/controllers/suggestions.test.js b/test/controllers/suggestions.test.js index 769f6ff87..65c039ce5 100644 --- a/test/controllers/suggestions.test.js +++ b/test/controllers/suggestions.test.js @@ -800,103 +800,7 @@ describe('Suggestions Controller', () => { expect(mockLog.warn).to.have.been.calledWith('Grant suggestions handler failed', 'grant failure'); }); - it('limits CWV suggestions to top 3 for PLG (sites-optimizer-ui) clients', async () => { - const cwvSuggs = ['id-0', 'id-1', 'id-2', 'id-3'].map((id, i) => ({ - getId() { return id; }, - getOpportunityId() { return OPPORTUNITY_ID; }, - getType() { return 'CODE_CHANGE'; }, - getStatus() { return 'NEW'; }, - getRank() { return i; }, - getData() { return { type: 'url', url: `https://example.com/page${i}`, jiraLink: '' }; }, - getKpiDeltas() { return null; }, - getCreatedAt() { return '2024-01-01T00:00:00.000Z'; }, - getUpdatedAt() { return '2024-01-01T00:00:00.000Z'; }, - getSkipReason() { return null; }, - getSkipDetail() { return null; }, - getUpdatedBy() { return 'system'; }, - getOpportunity() { - return { getSiteId() { return SITE_ID; }, getType() { return 'cwv'; } }; - }, - })); - - mockSuggestionDataAccess.Suggestion.allByOpportunityId.resolves(cwvSuggs); - - // mockConfiguration already has isHandlerEnabledForSite('summit-plg', site) = true - const response = await suggestionsController.getAllForOpportunity({ - params: { siteId: SITE_ID, opportunityId: OPPORTUNITY_ID }, - pathInfo: { headers: { 'x-client-type': 'sites-optimizer-ui' } }, - ...context, - }); - expect(response.status).to.equal(200); - const result = await response.json(); - expect(result).to.be.an('array').with.lengthOf(3); - }); - - it('returns all CWV suggestions when summit-plg handler is not enabled for the site', async () => { - const cwvSuggs = ['id-0', 'id-1', 'id-2', 'id-3'].map((id, i) => ({ - getId() { return id; }, - getOpportunityId() { return OPPORTUNITY_ID; }, - getType() { return 'CODE_CHANGE'; }, - getStatus() { return 'NEW'; }, - getRank() { return i; }, - getData() { return { type: 'url', url: `https://example.com/page${i}`, jiraLink: '' }; }, - getKpiDeltas() { return null; }, - getCreatedAt() { return '2024-01-01T00:00:00.000Z'; }, - getUpdatedAt() { return '2024-01-01T00:00:00.000Z'; }, - getSkipReason() { return null; }, - getSkipDetail() { return null; }, - getUpdatedBy() { return 'system'; }, - getOpportunity() { - return { getSiteId() { return SITE_ID; }, getType() { return 'cwv'; } }; - }, - })); - - mockSuggestionDataAccess.Suggestion.allByOpportunityId.resolves(cwvSuggs); - // Override configuration to return false for summit-plg - mockSuggestionDataAccess.Configuration.findLatest.resolves({ - isHandlerEnabledForSite: () => false, - }); - const response = await suggestionsController.getAllForOpportunity({ - params: { siteId: SITE_ID, opportunityId: OPPORTUNITY_ID }, - pathInfo: { headers: { 'x-client-type': 'sites-optimizer-ui' } }, - ...context, - }); - expect(response.status).to.equal(200); - const result = await response.json(); - expect(result).to.be.an('array').with.lengthOf(4); - }); - - it('returns all CWV suggestions when x-client-type is not sites-optimizer-ui', async () => { - const cwvSuggs = ['id-0', 'id-1', 'id-2', 'id-3'].map((id, i) => ({ - getId() { return id; }, - getOpportunityId() { return OPPORTUNITY_ID; }, - getType() { return 'CODE_CHANGE'; }, - getStatus() { return 'NEW'; }, - getRank() { return i; }, - getData() { return { type: 'url', url: `https://example.com/page${i}`, jiraLink: '' }; }, - getKpiDeltas() { return null; }, - getCreatedAt() { return '2024-01-01T00:00:00.000Z'; }, - getUpdatedAt() { return '2024-01-01T00:00:00.000Z'; }, - getSkipReason() { return null; }, - getSkipDetail() { return null; }, - getUpdatedBy() { return 'system'; }, - getOpportunity() { - return { getSiteId() { return SITE_ID; }, getType() { return 'cwv'; } }; - }, - })); - - mockSuggestionDataAccess.Suggestion.allByOpportunityId.resolves(cwvSuggs); - - // No sites-optimizer-ui header → all 4 returned - const response = await suggestionsController.getAllForOpportunity({ - params: { siteId: SITE_ID, opportunityId: OPPORTUNITY_ID }, - ...context, - }); - expect(response.status).to.equal(200); - const result = await response.json(); - expect(result).to.be.an('array').with.lengthOf(4); - }); it('gets all suggestions for an opportunity and a site for non belonging to the organization', async () => { sandbox.stub(AccessControlUtil.prototype, 'hasAccess').returns(false); diff --git a/test/support/grant-suggestions-handler.test.js b/test/support/grant-suggestions-handler.test.js index fa3566082..9ace31221 100644 --- a/test/support/grant-suggestions-handler.test.js +++ b/test/support/grant-suggestions-handler.test.js @@ -200,6 +200,57 @@ describe('grant-suggestions-handler', () => { const groups = getTopSuggestions([s1, s2]); expect(groups).to.have.lengthOf(2); }); + + it('sorts cwv suggestions by pageviews descending using getData()', () => { + const s1 = { getId: () => 'id-1', getRank: () => 500, getData: () => ({ pageviews: 500 }) }; + const s2 = { getId: () => 'id-2', getRank: () => 9000, getData: () => ({ pageviews: 9000 }) }; + const s3 = { getId: () => 'id-3', getRank: () => 200, getData: () => ({ pageviews: 200 }) }; + const groups = getTopSuggestions([s1, s2, s3], 'cwv'); + expect(groups).to.have.lengthOf(3); + expect(groups[0].items[0]).to.equal(s2); // 9000 first + expect(groups[1].items[0]).to.equal(s1); // 500 second + expect(groups[2].items[0]).to.equal(s3); // 200 last + }); + + it('sorts cwv suggestions by pageviews descending using plain object data', () => { + const s1 = { id: 'id-1', rank: 500, data: { pageviews: 500 } }; + const s2 = { id: 'id-2', rank: 9000, data: { pageviews: 9000 } }; + const groups = getTopSuggestions([s1, s2], 'cwv'); + expect(groups).to.have.lengthOf(2); + expect(groups[0].items[0]).to.equal(s2); // 9000 first + expect(groups[1].items[0]).to.equal(s1); // 500 second + }); + + it('sorts cwv suggestions with missing pageviews to the end', () => { + const s1 = { getId: () => 'id-1', getRank: () => 1000, getData: () => ({ pageviews: 1000 }) }; + const s2 = { getId: () => 'id-2', getRank: () => 0, getData: () => ({}) }; + const groups = getTopSuggestions([s2, s1], 'cwv'); + expect(groups[0].items[0]).to.equal(s1); // 1000 first + expect(groups[1].items[0]).to.equal(s2); // no pageviews (0) last + }); + + it('breaks cwv pageview ties by id ascending', () => { + const s1 = { getId: () => 'id-b', getRank: () => 500, getData: () => ({ pageviews: 500 }) }; + const s2 = { getId: () => 'id-a', getRank: () => 500, getData: () => ({ pageviews: 500 }) }; + const groups = getTopSuggestions([s1, s2], 'cwv'); + expect(groups[0].items[0]).to.equal(s2); // id-a before id-b + expect(groups[1].items[0]).to.equal(s1); + }); + + it('breaks cwv pageview ties by id ascending using plain object id', () => { + const s1 = { id: 'id-b', rank: 500, data: { pageviews: 500 } }; + const s2 = { id: 'id-a', rank: 500, data: { pageviews: 500 } }; + const groups = getTopSuggestions([s1, s2], 'cwv'); + expect(groups[0].items[0]).to.equal(s2); // id-a before id-b + expect(groups[1].items[0]).to.equal(s1); + }); + + it('breaks cwv pageview ties stably when items have no id', () => { + const s1 = { rank: 500, data: { pageviews: 500 } }; + const s2 = { rank: 500, data: { pageviews: 500 } }; + const groups = getTopSuggestions([s1, s2], 'cwv'); + expect(groups).to.have.lengthOf(2); // both empty-string ids → stable, no throw + }); }); describe('grantSuggestionsForOpportunity', () => { From 42e23a3be4a7abf45ba6219defdfbf66e9ef2a29 Mon Sep 17 00:00:00 2001 From: Tej Kotthakota Date: Tue, 7 Apr 2026 13:25:37 -0500 Subject: [PATCH 4/6] add cwv sorting logic --- test/controllers/suggestions.test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/controllers/suggestions.test.js b/test/controllers/suggestions.test.js index 65c039ce5..43f54b6a4 100644 --- a/test/controllers/suggestions.test.js +++ b/test/controllers/suggestions.test.js @@ -800,8 +800,6 @@ describe('Suggestions Controller', () => { expect(mockLog.warn).to.have.been.calledWith('Grant suggestions handler failed', 'grant failure'); }); - - it('gets all suggestions for an opportunity and a site for non belonging to the organization', async () => { sandbox.stub(AccessControlUtil.prototype, 'hasAccess').returns(false); sandbox.stub(context.attributes.authInfo, 'hasOrganization').returns(false); From 37944a62819746b8403659d2403dbb576a88b775 Mon Sep 17 00:00:00 2001 From: Tej Kotthakota Date: Tue, 7 Apr 2026 17:30:33 -0500 Subject: [PATCH 5/6] use rank which containes confidence score in decending order when sorting --- src/support/grant-suggestions-handler.js | 14 +++---- .../support/grant-suggestions-handler.test.js | 42 +++++++++---------- 2 files changed, 26 insertions(+), 30 deletions(-) diff --git a/src/support/grant-suggestions-handler.js b/src/support/grant-suggestions-handler.js index 80454d13f..45ca8a89c 100644 --- a/src/support/grant-suggestions-handler.js +++ b/src/support/grant-suggestions-handler.js @@ -98,18 +98,14 @@ const OPPORTUNITY_STRATEGIES = { ); }, }, - // Sorts CWV suggestions by pageviews descending so highest-traffic pages - // are granted first, giving users the most impactful fixes first. + // Sorts CWV suggestions by confidence score (rank) descending so the most + // impactful pages are granted first. Confidence score is set by the audit + // worker as projected traffic lost (organic × metric-severity multiplier). // Tie-breaks by suggestion ID ascending for deterministic ordering. cwv: { sortFn: (groupA, groupB) => { - const getPageviews = (group) => { - const s = group.items[0]; - const data = typeof s?.getData === 'function' ? s.getData() : s?.data; - return data?.pageviews ?? 0; - }; - const pvDiff = getPageviews(groupB) - getPageviews(groupA); - if (pvDiff !== 0) return pvDiff; + const rankDiff = groupB.getRank() - groupA.getRank(); + if (rankDiff !== 0) return rankDiff; const a = groupA.items[0]; const b = groupB.items[0]; const idA = typeof a?.getId === 'function' ? a.getId() : (a?.id ?? ''); diff --git a/test/support/grant-suggestions-handler.test.js b/test/support/grant-suggestions-handler.test.js index 9ace31221..1dea09d3e 100644 --- a/test/support/grant-suggestions-handler.test.js +++ b/test/support/grant-suggestions-handler.test.js @@ -201,10 +201,10 @@ describe('grant-suggestions-handler', () => { expect(groups).to.have.lengthOf(2); }); - it('sorts cwv suggestions by pageviews descending using getData()', () => { - const s1 = { getId: () => 'id-1', getRank: () => 500, getData: () => ({ pageviews: 500 }) }; - const s2 = { getId: () => 'id-2', getRank: () => 9000, getData: () => ({ pageviews: 9000 }) }; - const s3 = { getId: () => 'id-3', getRank: () => 200, getData: () => ({ pageviews: 200 }) }; + it('sorts cwv suggestions by confidence score (rank) descending', () => { + const s1 = { getId: () => 'id-1', getRank: () => 500 }; + const s2 = { getId: () => 'id-2', getRank: () => 9000 }; + const s3 = { getId: () => 'id-3', getRank: () => 200 }; const groups = getTopSuggestions([s1, s2, s3], 'cwv'); expect(groups).to.have.lengthOf(3); expect(groups[0].items[0]).to.equal(s2); // 9000 first @@ -212,42 +212,42 @@ describe('grant-suggestions-handler', () => { expect(groups[2].items[0]).to.equal(s3); // 200 last }); - it('sorts cwv suggestions by pageviews descending using plain object data', () => { - const s1 = { id: 'id-1', rank: 500, data: { pageviews: 500 } }; - const s2 = { id: 'id-2', rank: 9000, data: { pageviews: 9000 } }; + it('sorts cwv suggestions by confidence score descending using plain objects', () => { + const s1 = { id: 'id-1', rank: 500 }; + const s2 = { id: 'id-2', rank: 9000 }; const groups = getTopSuggestions([s1, s2], 'cwv'); expect(groups).to.have.lengthOf(2); expect(groups[0].items[0]).to.equal(s2); // 9000 first expect(groups[1].items[0]).to.equal(s1); // 500 second }); - it('sorts cwv suggestions with missing pageviews to the end', () => { - const s1 = { getId: () => 'id-1', getRank: () => 1000, getData: () => ({ pageviews: 1000 }) }; - const s2 = { getId: () => 'id-2', getRank: () => 0, getData: () => ({}) }; + it('sorts cwv suggestions with zero confidence score to the end', () => { + const s1 = { getId: () => 'id-1', getRank: () => 1000 }; + const s2 = { getId: () => 'id-2', getRank: () => 0 }; const groups = getTopSuggestions([s2, s1], 'cwv'); expect(groups[0].items[0]).to.equal(s1); // 1000 first - expect(groups[1].items[0]).to.equal(s2); // no pageviews (0) last + expect(groups[1].items[0]).to.equal(s2); // 0 last }); - it('breaks cwv pageview ties by id ascending', () => { - const s1 = { getId: () => 'id-b', getRank: () => 500, getData: () => ({ pageviews: 500 }) }; - const s2 = { getId: () => 'id-a', getRank: () => 500, getData: () => ({ pageviews: 500 }) }; + it('breaks cwv confidence score ties by id ascending', () => { + const s1 = { getId: () => 'id-b', getRank: () => 500 }; + const s2 = { getId: () => 'id-a', getRank: () => 500 }; const groups = getTopSuggestions([s1, s2], 'cwv'); expect(groups[0].items[0]).to.equal(s2); // id-a before id-b expect(groups[1].items[0]).to.equal(s1); }); - it('breaks cwv pageview ties by id ascending using plain object id', () => { - const s1 = { id: 'id-b', rank: 500, data: { pageviews: 500 } }; - const s2 = { id: 'id-a', rank: 500, data: { pageviews: 500 } }; + it('breaks cwv confidence score ties by id ascending using plain object id', () => { + const s1 = { id: 'id-b', rank: 500 }; + const s2 = { id: 'id-a', rank: 500 }; const groups = getTopSuggestions([s1, s2], 'cwv'); expect(groups[0].items[0]).to.equal(s2); // id-a before id-b expect(groups[1].items[0]).to.equal(s1); }); - it('breaks cwv pageview ties stably when items have no id', () => { - const s1 = { rank: 500, data: { pageviews: 500 } }; - const s2 = { rank: 500, data: { pageviews: 500 } }; + it('breaks cwv confidence score ties stably when items have no id', () => { + const s1 = { rank: 500 }; + const s2 = { rank: 500 }; const groups = getTopSuggestions([s1, s2], 'cwv'); expect(groups).to.have.lengthOf(2); // both empty-string ids → stable, no throw }); @@ -404,7 +404,7 @@ describe('grant-suggestions-handler', () => { // Only 1 remaining, so only 1 grant call expect(SuggestionGrant.grantSuggestions).to.have.been.calledOnce; expect(SuggestionGrant.grantSuggestions.firstCall.args[0]) - .to.deep.equal(['sugg-1']); + .to.deep.equal(['sugg-2']); }); describe('new token (new cycle) — revoke and re-grant', () => { From 981bdd4130460f738cad8ef20e609d2e1b532d35 Mon Sep 17 00:00:00 2001 From: Tej Kotthakota Date: Tue, 7 Apr 2026 18:33:32 -0500 Subject: [PATCH 6/6] cwv - group using page views and then sort them using confidence score for summit plg customers --- src/support/grant-suggestions-handler.js | 18 +++- .../support/grant-suggestions-handler.test.js | 87 +++++++++++++++---- 2 files changed, 83 insertions(+), 22 deletions(-) diff --git a/src/support/grant-suggestions-handler.js b/src/support/grant-suggestions-handler.js index 45ca8a89c..4d6456b2c 100644 --- a/src/support/grant-suggestions-handler.js +++ b/src/support/grant-suggestions-handler.js @@ -98,11 +98,23 @@ const OPPORTUNITY_STRATEGIES = { ); }, }, - // Sorts CWV suggestions by confidence score (rank) descending so the most - // impactful pages are granted first. Confidence score is set by the audit - // worker as projected traffic lost (organic × metric-severity multiplier). + // For PLG customers, CWV grants are limited to the top 3 pages by page views + // (pages with the most traffic that have CWV issues). Among those 3 pages, + // suggestions are sorted by confidence score (rank) descending so the most + // impactful page is granted first. Confidence score is set by the audit worker + // as projected traffic lost (organic × metric-severity multiplier). // Tie-breaks by suggestion ID ascending for deterministic ordering. cwv: { + groupFn: (suggestions) => { + const getPageviews = (s) => { + const data = typeof s?.getData === 'function' ? s.getData() : s?.data; + return data?.pageviews ?? 0; + }; + return [...suggestions] + .sort((a, b) => getPageviews(b) - getPageviews(a)) + .slice(0, 3) + .map((s) => createGroup([s])); + }, sortFn: (groupA, groupB) => { const rankDiff = groupB.getRank() - groupA.getRank(); if (rankDiff !== 0) return rankDiff; diff --git a/test/support/grant-suggestions-handler.test.js b/test/support/grant-suggestions-handler.test.js index 1dea09d3e..4fe884f01 100644 --- a/test/support/grant-suggestions-handler.test.js +++ b/test/support/grant-suggestions-handler.test.js @@ -201,10 +201,43 @@ describe('grant-suggestions-handler', () => { expect(groups).to.have.lengthOf(2); }); - it('sorts cwv suggestions by confidence score (rank) descending', () => { - const s1 = { getId: () => 'id-1', getRank: () => 500 }; - const s2 = { getId: () => 'id-2', getRank: () => 9000 }; - const s3 = { getId: () => 'id-3', getRank: () => 200 }; + it('cwv: limits to top 3 pages by pageviews before sorting by confidence', () => { + const mk = (id, pageviews, rank) => ({ + getId: () => id, getRank: () => rank, getData: () => ({ pageviews }), + }); + // 5 pages — top 3 by pageviews are s1(5000), s3(4000), s4(3000) + const s1 = mk('id-1', 5000, 200); + const s2 = mk('id-2', 1000, 9000); // high confidence but low pageviews → excluded + const s3 = mk('id-3', 4000, 500); + const s4 = mk('id-4', 3000, 100); + const s5 = mk('id-5', 2000, 300); // excluded (4th by pageviews) + const groups = getTopSuggestions([s1, s2, s3, s4, s5], 'cwv'); + // only top 3 by pageviews included + expect(groups).to.have.lengthOf(3); + const ids = groups.flatMap((g) => g.items.map((s) => s.getId())); + expect(ids).to.include.members(['id-1', 'id-3', 'id-4']); + expect(ids).to.not.include('id-2'); + expect(ids).to.not.include('id-5'); + // sorted by confidence descending: s3(500) > s1(200) > s4(100) + expect(groups[0].items[0]).to.equal(s3); + expect(groups[1].items[0]).to.equal(s1); + expect(groups[2].items[0]).to.equal(s4); + }); + + it('cwv: returns fewer than 3 when fewer pages available', () => { + const s1 = { getId: () => 'id-1', getRank: () => 500, getData: () => ({ pageviews: 3000 }) }; + const s2 = { getId: () => 'id-2', getRank: () => 9000, getData: () => ({ pageviews: 1000 }) }; + const groups = getTopSuggestions([s1, s2], 'cwv'); + expect(groups).to.have.lengthOf(2); + }); + + it('cwv: sorts top 3 pages by confidence score descending', () => { + const mk = (id, pageviews, rank) => ({ + getId: () => id, getRank: () => rank, getData: () => ({ pageviews }), + }); + const s1 = mk('id-1', 5000, 500); + const s2 = mk('id-2', 4000, 9000); + const s3 = mk('id-3', 3000, 200); const groups = getTopSuggestions([s1, s2, s3], 'cwv'); expect(groups).to.have.lengthOf(3); expect(groups[0].items[0]).to.equal(s2); // 9000 first @@ -212,45 +245,61 @@ describe('grant-suggestions-handler', () => { expect(groups[2].items[0]).to.equal(s3); // 200 last }); - it('sorts cwv suggestions by confidence score descending using plain objects', () => { - const s1 = { id: 'id-1', rank: 500 }; - const s2 = { id: 'id-2', rank: 9000 }; + it('cwv: sorts top 3 pages by confidence score descending using plain objects', () => { + const s1 = { id: 'id-1', rank: 500, data: { pageviews: 5000 } }; + const s2 = { id: 'id-2', rank: 9000, data: { pageviews: 4000 } }; const groups = getTopSuggestions([s1, s2], 'cwv'); expect(groups).to.have.lengthOf(2); expect(groups[0].items[0]).to.equal(s2); // 9000 first expect(groups[1].items[0]).to.equal(s1); // 500 second }); - it('sorts cwv suggestions with zero confidence score to the end', () => { - const s1 = { getId: () => 'id-1', getRank: () => 1000 }; - const s2 = { getId: () => 'id-2', getRank: () => 0 }; + it('cwv: sorts zero confidence score to the end among top 3 pages', () => { + const mk = (id, pageviews, rank) => ({ + getId: () => id, getRank: () => rank, getData: () => ({ pageviews }), + }); + const s1 = mk('id-1', 5000, 1000); + const s2 = mk('id-2', 4000, 0); const groups = getTopSuggestions([s2, s1], 'cwv'); expect(groups[0].items[0]).to.equal(s1); // 1000 first expect(groups[1].items[0]).to.equal(s2); // 0 last }); - it('breaks cwv confidence score ties by id ascending', () => { - const s1 = { getId: () => 'id-b', getRank: () => 500 }; - const s2 = { getId: () => 'id-a', getRank: () => 500 }; + it('cwv: breaks confidence score ties by id ascending', () => { + const mk = (id, pageviews, rank) => ({ + getId: () => id, getRank: () => rank, getData: () => ({ pageviews }), + }); + const s1 = mk('id-b', 5000, 500); + const s2 = mk('id-a', 4000, 500); const groups = getTopSuggestions([s1, s2], 'cwv'); expect(groups[0].items[0]).to.equal(s2); // id-a before id-b expect(groups[1].items[0]).to.equal(s1); }); - it('breaks cwv confidence score ties by id ascending using plain object id', () => { - const s1 = { id: 'id-b', rank: 500 }; - const s2 = { id: 'id-a', rank: 500 }; + it('cwv: breaks confidence score ties by id ascending using plain object id', () => { + const s1 = { id: 'id-b', rank: 500, data: { pageviews: 5000 } }; + const s2 = { id: 'id-a', rank: 500, data: { pageviews: 4000 } }; const groups = getTopSuggestions([s1, s2], 'cwv'); expect(groups[0].items[0]).to.equal(s2); // id-a before id-b expect(groups[1].items[0]).to.equal(s1); }); - it('breaks cwv confidence score ties stably when items have no id', () => { - const s1 = { rank: 500 }; - const s2 = { rank: 500 }; + it('cwv: breaks confidence score ties stably when items have no id', () => { + const s1 = { rank: 500, data: { pageviews: 5000 } }; + const s2 = { rank: 500, data: { pageviews: 4000 } }; const groups = getTopSuggestions([s1, s2], 'cwv'); expect(groups).to.have.lengthOf(2); // both empty-string ids → stable, no throw }); + + it('cwv: treats missing pageviews as zero for pageviews sort', () => { + const s1 = { getId: () => 'id-1', getRank: () => 500, getData: () => ({}) }; + const s2 = { getId: () => 'id-2', getRank: () => 200, getData: () => ({ pageviews: 1000 }) }; + const groups = getTopSuggestions([s1, s2], 'cwv'); + expect(groups).to.have.lengthOf(2); + // s2 has higher pageviews so it's included; s1 pageviews=0 still included (only 2 total) + const ids = groups.flatMap((g) => g.items.map((s) => s.getId())); + expect(ids).to.include.members(['id-1', 'id-2']); + }); }); describe('grantSuggestionsForOpportunity', () => {