Drop the insight metrics Meta retired - #5
Merged
Merged
Conversation
The API validates the whole metric list before it reads any of it, so a single retired name answers 400 for the request and costs every metric for that media kind. `plays` had been kept on video and reel on the belief that v21 still served it. It does not: production rejects it, and the error names every value the API will accept. `exits` is absent from that list too, so the story set was failing the same way. The practical effect was that every feed video and every reel published through the app recorded no insights at all, while carousels and images came back whole. The ingestion job rescues the error and logs it, so nothing surfaced. Video counts plays as `views` now, verified against a real feed video on the same call that rejected `plays`. Stories drop `exits` and keep what the list allows; `navigation` is what replaced it, but it reports through a breakdown this client does not ask for yet, so that waits on a live story to check the shape. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
|
@codex review this pr |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
playsandexitsare no longer valid metric names. The API validates the wholemetriclist up front, so one retired name answers 400 for the entire request: every feed video and every reel published through LMP has been recording no insights at all, andFetchMediaInsightsJobrescued the 400 into a log line. Carousels and images were unaffected, which is why it stayed invisible.Found by probing production media directly. A real feed video rejected the current list with:
metric[6]isplays. The same call succeeded 7/7 withviewsin its place, verified against the same post.exitsdoes not appear anywhere in that list either, which is why the story set drops it.video:plays→views(verified)reel: dropplays, keepviewsand the watch-time metrics (unreachable today, sinceinsight_metric_kindmaps reels to:video, but it would fail identically)story: dropexits, nowreach replies views. Unverified:navigationis what replacedexits, but it reports through a breakdown this client does not ask for yet, so that waits on a live story.Specs assert no retired name is ever asked for, with the 400-for-the-whole-request reason recorded so the list is only widened against a verified response.
Downstream
Rails pins this gem by ref (
Gemfile:64), so this needs a ref bump to ship.Both clients compute "Total exits" and "Completion rate" from
exits(AnalyzeSwiftUIViewModel.swift:381,StoriesAnalytics.tsx:30). With the metric gone they will read "0 exits, 100% completion" rather than erroring, so those two cards need thenavigationbreakdown or removal.🤖 Generated with Claude Code