feat(plg): fallback funnel for alt-text suggestion grant selection (SITES-44253)#2870
feat(plg): fallback funnel for alt-text suggestion grant selection (SITES-44253)#2870yashrathore11 wants to merge 2 commits into
Conversation
…ITES-44253) Grants/displays alt-text suggestions via a priority funnel so a customer's limited PLG token slots go to the most actionable images first: non-decorative images with a generated fix, then non-decorative images without a fix, and only falling back to decorative images once higher tiers can't fill the bucket.
|
This PR will trigger a minor release when merged. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Hey @yashrathore11,
⚠ 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: Approve - clean, well-scoped implementation with thorough tests; no blocking issues found.
Complexity: MEDIUM - 110 lines across 2 files, single-service internal logic.
Changes: Adds a three-tier priority funnel to alt-text suggestion grant selection so PLG token slots go to non-decorative images with fixes first, then non-decorative without fixes, then decorative (2 files).
Non-blocking (3): minor issues and suggestions
- suggestion:
getAltTextTieris recomputed on every pairwise comparison during sort; for large suggestion sets a Schwartzian transform (compute tiers once, sort on the cached value) would be more efficient and is arguably cleaner -src/support/grant-suggestions-handler.js:170 - suggestion: Add explicit test cases for
getData()returningnulland for whitespace-onlyaltText(e.g.' ') to lock in that these correctly map to tier 1 and prevent future regressions on the?.and.trim()paths -test/support/grant-suggestions-handler.test.js:327 - nit: JSDoc on
getAltTextTierdescribes tier 1 as "non-decorative image without a fix" but the function also places suggestions with missing/emptyrecommendationsinto tier 1; a parenthetical "(or no recommendations)" would help future readers -src/support/grant-suggestions-handler.js:55
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 27s | Cost: $3.11 | Commit: 0d2402f5dce4c5a8ed3dc4ad32a28bf941a7d053
If this code review was useful, please react with 👍. Otherwise, react with 👎.
Grants/displays alt-text suggestions via a priority funnel so a customer's limited PLG token slots go to the most actionable images first: non-decorative images with a generated fix, then non-decorative images without a fix, and only falling back to decorative images once higher tiers can't fill the bucket.
If the PR is changing the API specification:
grant-suggestions-handler.js; no route, controller signature, or response shape changes.If the PR is changing the API implementation or an entity exposed through the API:
If the PR is introducing a new audit type:
Related Issues
SITES-44253
Problem: Alt-text suggestions were granted/displayed to PLG customers without accounting for decorative images — a suggestion for a purely decorative image needs no alt-text fix and shouldn't consume one of the customer's limited monthly token slots ahead of suggestions that actually need action.
Fix: Added an
alt-textentry toOPPORTUNITY_STRATEGIESingetTopSuggestions(src/support/grant-suggestions-handler.js) that ranks suggestions into three tiers before the existing top-N grant selection runs:recommendations[].altTextpresent)recommendations[].isDecorative === true)Because tiers are mutually exclusive and sorted ascending, slicing the top N (the existing
fillRemainingCapacitybehavior) naturally implements the fallback funnel — no changes needed to the calling/grant code path.Testing
Unit tests (
test/support/grant-suggestions-handler.test.js): added 8 new cases covering tier assignment (non-decorative+fix / non-decorative-no-fix / decorative), bucket-filling across tiers, full fallback to decorative when higher tiers are exhausted, multi-recommendation decorative/fix detection, missing/emptyrecommendations, plain-object (non-entity) inputs, and rank/id tie-breaking within a tier. All 60 tests in the file pass; lint clean; coverage on the changed file is 100% statements/lines/functions (the one uncovered branch predates this change and sits in the untouchedbroken-backlinksstrategy).Manual end-to-end verification against a local Postgres/PostgREST-backed dev server, using an isolated seeded fixture (1 PLG-entitled org/site, 1
alt-textopportunity, 6 NEW suggestions: 2 non-decorative-with-fix, 2 non-decorative-without-fix, 2 decorative):GET .../opportunities/:opportunityId(triggersgrantSuggestionsForOpportunity) correctly persisted exactly 3 grants — both non-decorative-with-fix suggestions plus the first non-decorative-without-fix by id — with decorative suggestions excluded while the bucket wasn't yet full.OUTDATEDand re-triggering the grant refresh, the revoke+refill cycle correctly fell back to the 2 remaining decorative suggestions, confirming the full three-tier fallback.GET .../suggestions/by-status/:statusdoesn't itself triggergrantSuggestionsForOpportunity— it only reads existing grants. Calling it before any grant has been created for an opportunity returns[]even when eligible suggestions exist. This is unrelated to the funnel logic and was left out of scope for this change.