feat(LLMO-3074): add offsite-brand-presence audit#2002
feat(LLMO-3074): add offsite-brand-presence audit#2002rarescheseli wants to merge 17 commits intomainfrom
Conversation
|
This PR will trigger no release when merged. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| for (let i = 0; i < matchedFiles.length; i += FETCH_CONCURRENCY) { | ||
| const batch = matchedFiles.slice(i, i + FETCH_CONCURRENCY); | ||
| // eslint-disable-next-line no-await-in-loop | ||
| const fetchResults = await Promise.allSettled( | ||
| batch.map((filePath) => fetchBrandPresenceData(siteId, filePath, env, log)), | ||
| ); |
There was a problem hiding this comment.
Does it make sense to do parallel fetches or is sequential fetching enough? Sequential fetching code would be a bit easier to read and maintain. Maybe we add a small delay between requests to not overload the API.
There was a problem hiding this comment.
Went ahead with sequential since the code is more readable and easier to maintain. The gain from parallel fetches isn't that relevant anyway.
There was a problem hiding this comment.
The name of the audit is subject to change.
| const rows = data.data; | ||
| for (const row of rows) { | ||
| const sources = row.Sources?.trim(); | ||
| if (sources && row.Region === 'US' && row.Mentions === 'true' && row.Citations === 'true') { |
There was a problem hiding this comment.
Would be nice if the region could be configurable (maybe configured by the client somewhere?)
| * governing permissions and limitations under the License. | ||
| */ | ||
|
|
||
| export const DRS_TOP_URLS_LIMIT = 100; |
There was a problem hiding this comment.
TBD if we should increase/decrease this.
| const rows = data.data; | ||
| for (const row of rows) { | ||
| const sources = row.Sources?.trim(); | ||
| if (sources && row.Region === 'US') { |
There was a problem hiding this comment.
Does this make sense? We're limited to english-only content at the moment anyway.
|
|
||
| return { | ||
| auditResult: { | ||
| success: true, |
There was a problem hiding this comment.
In case all the urls failed to be saved to the DB, the audit is still marked as success, but we don't know if the urls have been saved or not
| * @param {object} site - The site being audited | ||
| * @returns {Promise<object>} Audit result | ||
| */ | ||
| export async function offsiteBrandPresenceRunner(finalUrl, context, site) { |
There was a problem hiding this comment.
the runner-audit.js has 4 params, here we can add auditContext = {}
There was a problem hiding this comment.
It breaks the linter since it would be unused.
…dded to URL store
|
I suggest we rename it to offsite-brand-presence-analysis |
3 participants
Main changes:
query-index.json, parses it and fetches latest brand presence filesPlease ensure your pull request adheres to the following guidelines:
Related Issues
Thanks for contributing!