Improve user run history and first-party asset previews - #77
Open
peacenode wants to merge 3 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
🟡 Changes recommended
The new Tooltip wrapper/dependency setup likely won’t compile or generate the intended Tailwind styles as written (import shape and origin-(...) class), so the UI changes are at risk of breaking builds/runtime.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR improves how run history details and asset previews are represented and displayed by (1) normalizing run detail fields (prompts/parameters/inference/billing), (2) rewriting provider media URLs into first-party asset IDs/URLs, and (3) adding a first-party asset proxy route with SSRF protections so previews can be served from earlyaccess.livepeer.org.
Changes:
- Reworked run execution + run detail APIs to return canonical first-party asset URLs/IDs and avoid leaking provider media/status URLs.
- Added a first-party
/api/assets/[id]proxy (range support, redirect handling, SSRF/private-IP blocking) plus dev fixtures and contract tests. - Introduced schema-driven input metadata support (Fal OpenAPI parsing/loading) and updated history UI rendering to use structured fields and robust media fallbacks.
File summaries
| File | Description |
|---|---|
| tests/integration/mcp-assets.test.ts | Updates integration expectations for serialized assets to use first-party asset URLs. |
| tests/contracts/run-security.test.ts | Extends privacy/URL extraction contract coverage for *_urls output arrays. |
| tests/contracts/run-execution.test.ts | Verifies durable execution returns asset IDs/first-party URLs and does not leak provider URLs. |
| tests/contracts/call-detail-media.test.tsx | Adds UI contract coverage for history drawer media rendering, expiry, and tooltip help. |
| tests/contracts/asset-proxy.test.ts | Adds contract tests for the asset proxy (range passthrough + private-origin refusal). |
| tests/contracts/admin-runs-preview.test.tsx | Updates admin drawer expectations to avoid “raw JSON” presentation labels. |
| package.json | Adds a Tooltip-related dependency. |
| lib/runs/types.ts | Adds displayName for assets and introduces RunInputSchema types for schema-driven help. |
| lib/runs/outputs.ts | Expands output URL extraction to include image_urls, video_urls, audio_urls. |
| lib/runs/execute.ts | Rewrites returned media payloads to public asset URLs and strips provider URLs from payload data. |
| lib/mcp/store.ts | Serializes asset URLs as first-party URLs and adds getAssetSource for proxying. |
| lib/mcp/fal-input-schema.ts | Adds Fal OpenAPI parsing + cached loading to drive metadata help for submitted inputs. |
| lib/mcp/fal-input-schema.test.ts | Adds node:test coverage for schema parsing and capability resolution. |
| lib/console/usage-capability-display.ts | Improves model name humanization by stripping provider/catalog namespaces. |
| lib/console/dev-mock.ts | Adds dev fixtures for runs/assets and a dev-mode /api/assets/* redirect shim. |
| lib/console/capability-modality.ts | Adds modality labeling + title cleanup via capabilityPresentation. |
| lib/console/capability-modality.test.ts | Adds tests for modality/title presentation and model name humanization. |
| lib/console/activity-assets.ts | Ensures attached output URLs are first-party asset URLs. |
| lib/assets/public.ts | Introduces utilities to generate public asset URLs and to rewrite/remove provider URLs at API boundaries. |
| lib/assets/public.test.ts | Adds coverage for asset URL rewriting and provider URL removal. |
| components/ui/tooltip.tsx | Adds a Radix-based Tooltip wrapper used by the updated UI. |
| components/design-system/Tooltip.tsx | Removes the prior custom Tooltip implementation. |
| components/console/ModalityChip.tsx | Adds a shared modality chip component for consistent display. |
| components/console/CallsTable.tsx | Switches tooltip usage and uses ModalityChip instead of inline styling. |
| components/console/CallDetailDrawer.tsx | Major redesign of the run detail drawer: structured fields, schema-driven help, media fallback/expiry UI, admin vs user variants. |
| components/admin/RunsPreview.tsx | Opens the drawer in admin variant for richer inspection behavior. |
| app/layout.tsx | Wraps the app with the new Tooltip provider. |
| app/api/console/runs/[id]/route.ts | Returns public run detail (rewritten assets) and loads optional Fal input schema metadata. |
| app/api/assets/[id]/route.ts | Adds first-party asset proxy route with SSRF protections and range handling. |
| app/api/admin/runs/[id]/route.ts | Returns public run detail (rewritten assets) for admin run fetches. |
Review details
- Files reviewed: 31/32 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+3
to
+6
| import * as React from "react"; | ||
| import { cn } from "@/lib/utils"; | ||
| import { Tooltip as TooltipPrimitive } from "radix-ui"; | ||
|
|
| data-slot="tooltip-content" | ||
| sideOffset={sideOffset} | ||
| className={cn( | ||
| "z-50 w-fit origin-(--radix-tooltip-content-transform-origin) animate-in rounded-md bg-foreground px-3 py-1.5 text-xs text-balance text-background fade-in-0 zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95", |
| "postgres": "^3.4.9", | ||
| "posthog-js": "^1.428.8", | ||
| "posthog-node": "^5.51.8", | ||
| "radix-ui": "^1.6.7", |
There was a problem hiding this comment.
🟡 Changes recommended
The asset proxy currently lacks owner-scoped authorization, and the media-expiry logic in the drawer doesn’t update over time without a reload/error.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
components/console/CallDetailDrawer.tsx:967
expiresAtis evaluated againstDate.now()once during render, soexpirednever flips to true as time passes; the media stage may keep trying to render content after the expiry countdown reaches zero unless a network error happens to triggermediaFailed. Consider trackingnowin state (similar toMediaExpiry) and computeexpiredfrom it so the UI updates deterministically when the asset expires.
package.json:41
- Adding the
radix-uimeta-package pulls in a very large set of Radix React packages (beyond Tooltip), increasing install surface area and potentially client bundle risk if tree-shaking is imperfect. If the only current use is Tooltip, consider depending on@radix-ui/react-tooltip(and related primitives) directly instead of the umbrella package.
"postgres": "^3.4.9",
"posthog-js": "^1.428.8",
"posthog-node": "^5.51.8",
"radix-ui": "^1.6.7",
"react": "^19.2.8",
"react-dom": "^19.2.8",
"recharts": "^3.10.1",
- Files reviewed: 31/32 changed files
- Comments generated: 1
- Review effort level: Lite
Comment on lines
+62
to
+68
| async function proxy(request: Request, id: string): Promise<Response> { | ||
| if (!/^[A-Za-z0-9_-]{1,160}$/.test(id)) { | ||
| return new Response("Not found", { status: 404 }); | ||
| } | ||
| const asset = await getAssetSource(id); | ||
| if (!asset) return new Response("Not found", { status: 404 }); | ||
|
|
There was a problem hiding this comment.
🔵 Needs a closer look
There are confirmed build/runtime correctness issues (incorrect Radix tooltip dependency/import and URL rewriting that can strip first-party input media), plus the asset proxy currently lacks an auth gate.
Review details
Suppressed comments (6)
Previously missed (1) — in code that hasn't changed since the last review.
lib/assets/public.ts:78
publicRunDetailcurrently removes any captured media URL that isn’t present indetail.assets. In the current write path,detail.assetsis populated from extracted outputs (seetransitionRun), so first-party input asset URLs (e.g. uploadedimage_url,mask_url) will be stripped fromsubmittedArguments, preventing the history UI from showing referenced inputs. Only strip non-first-party URLs that aren’t stored as assets.
This issue also appears on line 80 of the same file.
components/ui/tooltip.tsx:6
components/ui/tooltip.tsximportsTooltipPrimitivefromradix-ui, but the Radix Tooltip API is exposed by@radix-ui/react-tooltip(and the current import shape won’t provide.Provider/.Root/.Trigger/.Content). This will break typechecking/build; switch to the Radix tooltip package namespace import.
import * as React from "react";
import { cn } from "@/lib/utils";
import { Tooltip as TooltipPrimitive } from "radix-ui";
components/ui/tooltip.tsx:45
- Tailwind arbitrary value syntax looks incorrect here:
origin-(--radix-tooltip-content-transform-origin)won’t be parsed as a valid class, so the transform origin for the tooltip animation likely won’t apply. Use Tailwind’s bracket arbitrary value form.
sideOffset={sideOffset}
className={cn(
"z-50 w-fit origin-(--radix-tooltip-content-transform-origin) animate-in rounded-md bg-foreground px-3 py-1.5 text-xs text-balance text-background fade-in-0 zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95",
className
package.json:40
- A
radix-uidependency is added, but the code is using Radix Tooltip primitives (Provider/Root/Trigger/Content) which come from@radix-ui/react-tooltip. The lockfile already contains@radix-ui/react-tooltip, soradix-uilooks like the wrong package and may not even export the expected API.
"posthog-js": "^1.428.8",
"posthog-node": "^5.51.8",
"radix-ui": "^1.6.7",
"react": "^19.2.8",
"react-dom": "^19.2.8",
lib/assets/public.ts:88
- Same issue for
result.value: if a model result includes first-party asset URLs (or already-rewritten URLs) that aren’t duplicated indetail.assets, they’ll be removed. Filter removals to non-first-party URLs that aren’t represented by stored assets.
? {
...detail.result,
value: removeAssetUrls(
replaceAssetUrls(detail.result.value, detail.assets),
resultMedia.filter(
(url) => !detail.assets.some((asset) => asset.url === url)
)
),
}
app/api/assets/[id]/route.ts:68
/api/assets/[id]proxies stored media without any authentication/authorization check. Even with unguessable IDs, this makes any leaked asset ID globally retrievable (and could be indexed/cached as public). Consider requiring a console session (and optionally verifying principal ownership) before proxying.
async function proxy(request: Request, id: string): Promise<Response> {
if (!/^[A-Za-z0-9_-]{1,160}$/.test(id)) {
return new Response("Not found", { status: 404 });
}
const asset = await getAssetSource(id);
if (!asset) return new Response("Not found", { status: 404 });
- Files reviewed: 31/32 changed files
- Comments generated: 0 new
- Review effort level: Lite
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
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
Verification