Repository navigation
feat: redesign share dialog, OG image, agent chart - #2243
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a stacked AgentModelUsageChart to the dashboard (selectable ranges/metrics, buckets logs into slots, aggregates per-model metrics) and enhances shared chat pages with dynamic metadata, a 1200x630 OpenGraph image endpoint, and a redesigned share dialog including truncated URL display, copy and social share buttons (X, LinkedIn, Reddit). ChangesAgent Model Usage Chart
Share Chat Social Features & Metadata
Radix Select Wrapper
DevPass Card & Dashboard Wiring
DevPass Seed: Weighted Models
Admin KPI & EE UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/playground/src/components/playground/share-chat-dialog.tsx (1)
39-57: ⚡ Quick winSimplify URL truncation logic.
The
truncateShareUrlfunction has complexity that may not be necessary. The condition on line 49 comparesprefix.length > URL_DISPLAY_PREFIX.length, but sinceURL_DISPLAY_PREFIXis a constant defined above, this logic could be simplified. The current implementation extracts the origin and path segments, then conditionally replaces the prefix—but for this use case, a direct string manipulation approach might be clearer.Consider simplifying to directly format the display URL:
function truncateShareUrl(url: string): string { - if (!url) { - return ""; - } - try { - const parsed = new URL(url); - const segments = parsed.pathname.split("/").filter(Boolean); - const last = segments[segments.length - 1] ?? ""; - const prefix = `${parsed.origin}/${segments.slice(0, -1).join("/")}/`; - const visiblePrefix = - prefix.length > URL_DISPLAY_PREFIX.length - ? URL_DISPLAY_PREFIX - : prefix.replace(/\/+$/, "/"); - const shortId = last.length > 8 ? `${last.slice(0, 6)}…` : last; - return `${visiblePrefix}${shortId}`; - } catch { - return url.length > 40 ? `${url.slice(0, 37)}…` : url; - } + if (!url) { + return ""; + } + const shareIdMatch = url.match(/\/share\/([^/?#]+)/); + if (!shareIdMatch) { + return url.length > 40 ? `${url.slice(0, 37)}…` : url; + } + const shareId = shareIdMatch[1]; + const shortId = shareId.length > 8 ? `${shareId.slice(0, 6)}…` : shareId; + return `${URL_DISPLAY_PREFIX}${shortId}`; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/playground/src/components/playground/share-chat-dialog.tsx` around lines 39 - 57, The truncateShareUrl function contains overcomplicated prefix logic; simplify it by parsing the URL with new URL(url), extracting the origin and last path segment (use parsed.pathname.split("/").filter(Boolean) to get last), compute shortId as last.length > 8 ? `${last.slice(0,6)}…` : last, then build a displayPrefix as `${parsed.origin}/${segments.slice(0,-1).join("/")}/` and clamp it to URL_DISPLAY_PREFIX by using a simple length check to replace with URL_DISPLAY_PREFIX when longer (remove the regex trim/replace step), and finally return `${displayPrefix}${shortId}`; keep the existing catch fallback that truncates non-URLs to 40 chars. Use the existing symbols truncateShareUrl, URL_DISPLAY_PREFIX, parsed, segments, last, and shortId to locate and update the code.apps/code/src/app/dashboard/components/AgentModelUsageChart.tsx (1)
295-296: 💤 Low value
useMemowithnew Date()creates confusion.The
endDatememo captures "now" only whenrangechanges, not continuously. While this works with the currentstaleTimesetting, it's misleading becausenew Date()is non-deterministic and the dependency onrangedoesn't reflect a true data dependency—it's a side effect to trigger refetches.Consider removing the
useMemoentirely (letendDatealways be current) or using TanStack Query's refetch intervals if you need periodic updates.♻️ Proposed refactor: remove useMemo
const api = useApi(); const startDate = useMemo(() => getRangeStart(range).toISOString(), [range]); - // eslint-disable-next-line react-hooks/exhaustive-deps - const endDate = useMemo(() => new Date().toISOString(), [range]); + const endDate = new Date().toISOString(); const sourceParam = sources.join(",");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/code/src/app/dashboard/components/AgentModelUsageChart.tsx` around lines 295 - 296, The current useMemo for endDate inside AgentModelUsageChart is misleading because new Date() is non-deterministic and only updates when range changes; remove the useMemo and make endDate computed directly (const endDate = new Date().toISOString()) so it always reflects "now", or if you need periodic updates instead of on-render, configure the data fetch with TanStack Query's refetchInterval rather than relying on useMemo; update any references to endDate accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/code/src/app/dashboard/components/AgentModelUsageChart.tsx`:
- Around line 189-192: The current aggregation in modelTotals.set(modelId,
(modelTotals.get(modelId) ?? 0) + cost + tokens + 1) mixes incompatible units
(cost, tokens, and a count); change the metric to a single meaningful value —
e.g., aggregate by total cost only — by replacing the summed expression with
just cost (or a normalized weighted combination if you prefer) and update any
related variable names/labels/comments (e.g., modelTotals, modelId, cost,
tokens) so the chart/sort uses total cost as the sorting metric.
---
Nitpick comments:
In `@apps/code/src/app/dashboard/components/AgentModelUsageChart.tsx`:
- Around line 295-296: The current useMemo for endDate inside
AgentModelUsageChart is misleading because new Date() is non-deterministic and
only updates when range changes; remove the useMemo and make endDate computed
directly (const endDate = new Date().toISOString()) so it always reflects "now",
or if you need periodic updates instead of on-render, configure the data fetch
with TanStack Query's refetchInterval rather than relying on useMemo; update any
references to endDate accordingly.
In `@apps/playground/src/components/playground/share-chat-dialog.tsx`:
- Around line 39-57: The truncateShareUrl function contains overcomplicated
prefix logic; simplify it by parsing the URL with new URL(url), extracting the
origin and last path segment (use parsed.pathname.split("/").filter(Boolean) to
get last), compute shortId as last.length > 8 ? `${last.slice(0,6)}…` : last,
then build a displayPrefix as
`${parsed.origin}/${segments.slice(0,-1).join("/")}/` and clamp it to
URL_DISPLAY_PREFIX by using a simple length check to replace with
URL_DISPLAY_PREFIX when longer (remove the regex trim/replace step), and finally
return `${displayPrefix}${shortId}`; keep the existing catch fallback that
truncates non-URLs to 40 chars. Use the existing symbols truncateShareUrl,
URL_DISPLAY_PREFIX, parsed, segments, last, and shortId to locate and update the
code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c7dfe4fa-0f1e-41f0-9b40-cdd31bb89de1
📒 Files selected for processing (7)
apps/code/src/app/dashboard/components/AgentModelUsageChart.tsxapps/code/src/app/dashboard/components/CodingAgents.tsxapps/playground/src/app/share/[shareId]/opengraph-image.tsxapps/playground/src/app/share/[shareId]/page.tsxapps/playground/src/components/playground/chat-header.tsxapps/playground/src/components/playground/chat-page-client.tsxapps/playground/src/components/playground/share-chat-dialog.tsx
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/playground/src/components/playground/share-chat-dialog.tsx (1)
39-57: ⚡ Quick winConsider clarifying the prefix selection logic.
The condition at lines 49-52 uses string length comparison to decide between showing the canonical
llmgateway.ioprefix versus the actual URL origin. While functional, the intent is not immediately clear—you're using length as a proxy for "should we display branded canonical URL."A more explicit approach would improve maintainability:
♻️ Clearer alternative using explicit domain check
- const displayPrefix = - builtPrefix.length > URL_DISPLAY_PREFIX.length - ? URL_DISPLAY_PREFIX - : builtPrefix; + // Show canonical branding for production URLs, actual origin for dev/localhost + const displayPrefix = + parsed.origin === "https://chat.llmgateway.io" + ? URL_DISPLAY_PREFIX + : builtPrefix;Or, if the length-based heuristic is intentional for flexibility:
+ // Use canonical prefix when actual origin is longer (for consistent branding) const displayPrefix = builtPrefix.length > URL_DISPLAY_PREFIX.length ? URL_DISPLAY_PREFIX : builtPrefix;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/playground/src/components/playground/share-chat-dialog.tsx` around lines 39 - 57, The prefix selection in truncateShareUrl is unclear: instead of using builtPrefix.length > URL_DISPLAY_PREFIX.length to choose between parsed.origin and URL_DISPLAY_PREFIX, make the intent explicit by checking parsed.origin (or parsed.hostname) against an allowed/canonical domain list (or by testing startsWith the branded domain) and use that to select displayPrefix, or add a concise comment explaining the length-based heuristic; update truncateShareUrl and reference URL_DISPLAY_PREFIX and parsed.origin/hostname so the logic is readable and maintainable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/playground/src/components/playground/share-chat-dialog.tsx`:
- Around line 39-57: The prefix selection in truncateShareUrl is unclear:
instead of using builtPrefix.length > URL_DISPLAY_PREFIX.length to choose
between parsed.origin and URL_DISPLAY_PREFIX, make the intent explicit by
checking parsed.origin (or parsed.hostname) against an allowed/canonical domain
list (or by testing startsWith the branded domain) and use that to select
displayPrefix, or add a concise comment explaining the length-based heuristic;
update truncateShareUrl and reference URL_DISPLAY_PREFIX and
parsed.origin/hostname so the logic is readable and maintainable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9e2b7537-c69f-4721-9964-f1a428baf05f
📒 Files selected for processing (2)
apps/code/src/app/dashboard/components/AgentModelUsageChart.tsxapps/playground/src/components/playground/share-chat-dialog.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/code/src/app/dashboard/components/AgentModelUsageChart.tsx
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
smakosh
enabled auto-merge
May 10, 2026 12:50
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/db/src/seed.ts (1)
1286-1290: ⚡ Quick winEncode non-empty
modelsat the type levelLine 1289 allows
models: DevpassAgentModel[], but Line 1474 assumes non-empty input. Tighten the type to prevent accidental empty arrays and runtime failures.Suggested fix
- const DEVPASS_AGENTS: Array<{ + const DEVPASS_AGENTS: Array<{ source: string; weight: number; - models: DevpassAgentModel[]; + models: [DevpassAgentModel, ...DevpassAgentModel[]]; }> = [Also applies to: 1474-1474
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/src/seed.ts` around lines 1286 - 1290, The DEVPASS_AGENTS declaration currently types models as DevpassAgentModel[] but some consumers (e.g., the code referencing DEVPASS_AGENTS around the logic that expects at least one model) assume a non-empty array; tighten the type to a required non-empty tuple type (e.g., [DevpassAgentModel, ...DevpassAgentModel[]]) for the models property in the DEVPASS_AGENTS element type so the compiler prevents empty arrays and update any related signatures or callers if needed (refer to DEVPASS_AGENTS and DevpassAgentModel to locate the change).apps/ui/src/components/dashboard/devpass-card.tsx (1)
28-33: ⚡ Quick winConsider adding the
secureflag to the cookie in production.The cookie is set without the
secureflag, which means it can be transmitted over HTTP. For a UI preference cookie this is low-risk, but addingsecurewhen running over HTTPS follows security best practices and prevents potential downgrade attacks.🔒 Proposed enhancement
const toggle = () => { const next = !collapsed; setCollapsed(next); const maxAge = 60 * 60 * 24 * 365; - document.cookie = `${DEVPASS_CARD_COLLAPSED_COOKIE}=${next ? "1" : "0"}; path=/; max-age=${maxAge}; samesite=lax`; + const isSecure = window.location.protocol === 'https:'; + const secure = isSecure ? '; secure' : ''; + document.cookie = `${DEVPASS_CARD_COLLAPSED_COOKIE}=${next ? "1" : "0"}; path=/; max-age=${maxAge}; samesite=lax${secure}`; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ui/src/components/dashboard/devpass-card.tsx` around lines 28 - 33, The cookie written in toggle (uses collapsed, setCollapsed, DEVPASS_CARD_COLLAPSED_COOKIE) lacks the secure flag; update the document.cookie assignment to append "; secure" when running over HTTPS (e.g., check window.location.protocol === "https:" or NODE_ENV === "production") so the cookie is only sent over TLS in production, preserving the existing path, max-age, and samesite attributes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/db/src/seed.ts`:
- Around line 1475-1476: The lookup for modelDef currently matches only on model
id which can pick the wrong provider's entry; update the find on MODELS used to
set modelDef to match both model and provider (e.g., m.model ===
agentModel.model && m.provider === agentModel.provider) so seeded metrics use
the correct usedProvider/cost data, keeping the existing fallback to MODELS[0]
if no exact match is found.
---
Nitpick comments:
In `@apps/ui/src/components/dashboard/devpass-card.tsx`:
- Around line 28-33: The cookie written in toggle (uses collapsed, setCollapsed,
DEVPASS_CARD_COLLAPSED_COOKIE) lacks the secure flag; update the document.cookie
assignment to append "; secure" when running over HTTPS (e.g., check
window.location.protocol === "https:" or NODE_ENV === "production") so the
cookie is only sent over TLS in production, preserving the existing path,
max-age, and samesite attributes.
In `@packages/db/src/seed.ts`:
- Around line 1286-1290: The DEVPASS_AGENTS declaration currently types models
as DevpassAgentModel[] but some consumers (e.g., the code referencing
DEVPASS_AGENTS around the logic that expects at least one model) assume a
non-empty array; tighten the type to a required non-empty tuple type (e.g.,
[DevpassAgentModel, ...DevpassAgentModel[]]) for the models property in the
DEVPASS_AGENTS element type so the compiler prevents empty arrays and update any
related signatures or callers if needed (refer to DEVPASS_AGENTS and
DevpassAgentModel to locate the change).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d8d39199-0bf6-4dac-a400-af0420bd92ba
📒 Files selected for processing (7)
apps/code/src/app/dashboard/components/AgentModelUsageChart.tsxapps/code/src/components/ui/select.tsxapps/ui/src/app/dashboard/[orgId]/[projectId]/page.tsxapps/ui/src/components/dashboard/dashboard-client.tsxapps/ui/src/components/dashboard/devpass-card.tsxapps/ui/src/lib/cookies.tspackages/db/src/seed.ts
✅ Files skipped from review due to trivial changes (2)
- apps/code/src/components/ui/select.tsx
- apps/ui/src/lib/cookies.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/code/src/app/dashboard/components/AgentModelUsageChart.tsx
smakosh
added this pull request to the merge queue
May 10, 2026
smakosh
removed this pull request from the merge queue due to a manual request
May 10, 2026
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
smakosh
enabled auto-merge
May 10, 2026 13:17
smakosh
added this pull request to the merge queue
May 10, 2026
This was referenced May 12, 2026
2 tasks done
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.
Summary
https://chat.llmgateway.io/share/…URL pill, and a row of social share buttons (Copy / X / LinkedIn / Reddit) similar to ChatGPT's share modal. Mobile responsive./share/[shareId](opengraph-image.tsx): uses the LLM Gateway logo, the shared chat title/prompt as the headline, and a small response preview card.generateMetadatanow also fetches the share to set per-sharetitle/descriptionandtwitter:summary_large_image.AgentModelUsageChartcomponent shows a stacked Model Usage Overview chart (matching the apps/ui chart) with a metric switcher (Requests / Cost / Tokens) and a 1h / 4h / 1d / 7d / 30d range picker. Wired in above the existing model usage table on the agent detail view.Test plan
/share/[shareId]URL and confirm the OG image loads with the prompt + response preview + LLM Gateway branding.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements