Skip to content

[codex] Render inline media in citation popovers#66

Merged
steipete merged 1 commit into
mainfrom
codex/inline-popover-media
Jun 22, 2026
Merged

[codex] Render inline media in citation popovers#66
steipete merged 1 commit into
mainfrom
codex/inline-popover-media

Conversation

@steipete

Copy link
Copy Markdown
Owner

Summary

  • carry stored tweet media through Today and Discuss citation contexts
  • render up to four inline images inside tweet citation popovers
  • suppress malformed media-only t.co placeholders while retaining real links
  • accept older cached citation contexts that do not include media

Root cause

Timeline rows already contained media_json, but the compact AI-report contexts dropped it before rendering citations. Some imported media entities also have no usable text range, leaving the raw media t.co token visible in the popover.

Validation

  • pnpm run check
  • pnpm test (141 files, 1,175 tests)
  • pnpm build
  • pnpm test src/routes/today.test.tsx src/components/MarkdownViewer.test.tsx src/lib/period-digest.test.ts src/lib/search-discussion.test.ts
  • pnpm test src/lib/moderation-state.test.ts
  • pnpm test src/lib/backup.test.ts
  • Codex autoreview: clean, no accepted/actionable findings

@steipete steipete merged commit 16f3e58 into main Jun 22, 2026
2 checks passed
@steipete steipete deleted the codex/inline-popover-media branch June 22, 2026 07:41
@steipete

Copy link
Copy Markdown
Owner Author

Landed as 16f3e58.

Validation on exact PR head d3b08731748dacfb1096bac89b05ff6a3500d611:

  • pnpm run check
  • pnpm test — 141 files, 1,175 tests passed
  • pnpm build
  • pnpm test src/routes/today.test.tsx src/components/MarkdownViewer.test.tsx src/lib/period-digest.test.ts src/lib/search-discussion.test.ts — 69 tests passed
  • pnpm test src/lib/moderation-state.test.ts — 4 tests passed
  • pnpm test src/lib/backup.test.ts — 17 tests passed
  • /Users/steipete/Projects/agent-skills/skills/autoreview/scripts/autoreview --mode local --engine codex --model gpt-5.5 --thinking high --stream-engine-output — clean, no accepted/actionable findings

Exact-head CI:

  • CI / validate — success
  • GitGuardian Security Checks — success

Live deployment proof:

  • built and deployed exact head d3b0873 on Clawd macOS; restarted com.steipete.birdclaw.web; confirmed the production listener on 127.0.0.1:3000
  • existing signed-in Chrome profile at https://app.birdclaw.sh/today?period=today&includeDms=false
  • hovered tweet 2068933747961934014; popover rendered https://pbs.twimg.com/media/HLZT3J6XAAA2W4M.jpg, retained the tweet copy, and omitted the raw https://t.co/wLUzxU2jmY token
  • visually inspected the deployed popover layout; screenshot retained locally only because no image-upload destination approval was provided

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d3b0873174

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

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".

Comment on lines +172 to +174
return [...mediaUrls].some(
(url) => comparableUrl(url) === comparableUrl(entry.expandedUrl),
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Strip lost-range status media placeholders

When an imported media entity has no usable range, the normal archive shape stores the media entity's expandedUrl as the /status/.../photo/1 URL while extractTweetMedia stores the actual media_url_https as item.url (src/lib/archive/parsing.ts lines 64-66 and 189-193). In that case the raw trailing https://t.co/... remains in the rendered text because this lost-range check only compares entry.expandedUrl to the pbs media URLs, and isOwnStatusMediaUrl() is only applied to the raw trailing t.co URL, so the main malformed media-only placeholder scenario is still not suppressed.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant