Skip to content

PLAT-39403 connector: bridge Reddit avatars from the chat web API - #17

Merged
Fizzadar merged 2 commits into
mainfrom
nick/plat-39403-ghost-avatars
Oct 29, 2026
Merged

Fizzadar merged 2 commits into
mainfrom
nick/plat-39403-ghost-avatars

Conversation

@Fizzadar

Copy link
Copy Markdown
Contributor

Reddit's chat Matrix server has no avatars (/profile, member events and the user directory all return none), so ghosts were marked as having none. Chat info and GetUserInfo now take avatars from the ChatRedditors GraphQL operation that reddit.com/chat itself uses, batched per room and cached per client. Existing ghosts get their avatar the next time a room they're in syncs.

🤖 Generated with Claude Code

Reddit's chat Matrix server returns no avatar in profiles, member events or the
user directory, so every ghost was marked as having none. Reddit's own chat UI
reads them from the ChatRedditors GraphQL operation.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@indent

indent Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Issues

2 potential issues found:

  • A ChatRedditors response with a missing or null redditorsInfoByIds returns an empty map with no error. redditorAvatars then caches "no avatar" for every requested ID until the bridge restarts.
    Found by Indent Review Agent
  • Only the first 50 uncached members get avatars per chat sync. In groups with more than 50 members, the rest stay without avatars until later resyncs fetch 50 more at a time.
    Found by Indent Review Agent

↑ Select any checkbox above to have Indent auto-fix the issue

1 issue already resolved
  • avatarMu is held across the ChatRedditors network calls (up to 30s each), so all chat info resyncs and ghost info fetches for a login serialize behind it. A slow or hanging GraphQL endpoint stalls every resync, and failures aren't cached, so each one retries under the lock. (fixed by commit a52376d)
    Found by Indent Review Agent

CI Checks

All CI checks passed on a52376d.

Review agents

Select any unchecked box below to run or rerun that agent.

Found issues (1)
  • Indent Review Agent · Two nits: 50-ID cap per call, null-list caching
Full results

Indent Review Agent

  • Summary: Two nits: 50-ID cap per call, null-list caching
  • Last ran on commit: a52376d1
  • Latest result
    {
      "summary": "Two nits: 50-ID cap per call, null-list caching",
      "findings": [
        {
          "fix": "In redditorAvatars, claim every uncached ID and fetch them with slices.Chunk(missing, redditorBatchSize) outside the lock, as the previous version did. On error, stop and release the claims for the batches that weren't fetched. Alternatively, keep claiming and fetching batches in a loop until claimUncachedRedditors returns nothing.",
          "line": 52,
          "path": "pkg/connector/avatars.go",
          "issue": "claimUncachedRedditors stops after redditorBatchSize (50) IDs, and redditorAvatars makes only one fetch per call. For a group with more than 50 members, one GetChatInfo or ChatResync fetches avatars for 50 members, picked by Go's random map order. The rest get no avatar on that sync and only fill in 50 at a time as later room updates trigger more resyncs, so quiet large groups can stay partly avatar-less for a long time. The previous commit chunked through every missing ID in one call, so this is a regression from the backoff rework.",
          "title": "Only the first 50 uncached members get avatars per chat sync",
          "severity": "nit"
        },
        {
          "fix": "Make RedditorsInfoByIDs a pointer (or otherwise detect that it's missing) and return an error when it is nil. fetchRedditorIcons then backs off instead of caching. Only cache \"\" for IDs that are missing from a list that actually came back.",
          "line": 16,
          "path": "pkg/redditchat/redditors.go",
          "issue": "This still applies at a52376d. If ChatRedditors returns data without a redditorsInfoByIds list (null field, schema change, different operation shape), GetRedditorIcons returns an empty map and no error. fetchRedditorIcons then stores \"\" for every requested ID, and that negative entry lasts for the client's lifetime, so those ghosts never get avatars until restart. GetBlockedUsers treats an absent list as an error for this reason.",
          "title": "Missing or null redditorsInfoByIds caches every requested user as having no avatar",
          "severity": "nit"
        }
      ]
    }

Comment thread pkg/connector/avatars.go
} `json:"icon"`
} `json:"redditorsInfoByIds"`
}
err := s.chatGraphQL(ctx, "ChatRedditors", map[string]any{"ids": redditorIDs, "includeRedditorAttributes": true}, &data)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚪️ Nit · Missing redditorsInfoByIds caches every requested user as having no avatar

If the response has data but no redditorsInfoByIds list (schema change, null field, different shape), this returns an empty map with no error. redditorAvatars then caches "" for every ID in the batch for the client's lifetime, so those ghosts get no avatar until restart. GetBlockedUsers in blocking.go treats an absent list as an error for this reason; do the same here.

Found by Indent Review Agent

…fter failures

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread pkg/connector/avatars.go
defer r.avatarMu.Unlock()
if time.Now().Before(r.avatarRetryAt) {
return nil
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚪️ Nit · Only the first 50 uncached members get avatars per chat sync

claimUncachedRedditors stops after redditorBatchSize (50) IDs and redditorAvatars makes one fetch per call. In a group with more than 50 members, one sync fetches avatars for 50 members, chosen by Go's random map order. The rest only fill in 50 at a time as later room updates trigger resyncs, so quiet large groups can stay partly avatar-less for a long time. The previous commit chunked through every missing ID.

Claim all uncached IDs and fetch them in chunks outside the lock, releasing the claims for unfetched batches on error. Or loop until nothing is left to claim.

Found by Indent Review Agent

@Fizzadar

Copy link
Copy Markdown
Contributor Author

Live-tested at this head (a52376d, build 7990356): all Reddit ghosts that had no avatar on main got one after room activity (custom snoovatar and default avatars both), member events carry avatar_url, a ghost reset to no avatar was repopulated after a restart, u/reddit got its icon, and no avatar fetch failures were logged. Downtime catch-up and no echo unchanged.

@Fizzadar
Fizzadar merged commit bd87019 into main Oct 29, 2026
11 checks passed
@Fizzadar
Fizzadar deleted the nick/plat-39403-ghost-avatars branch October 29, 2026 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant