Skip to content

fix: image gen log content + shared LogCard - #1974

Merged
smakosh merged 4 commits into
mainfrom
fix/image-gen-response-data
Apr 6, 2026
Merged

smakosh merged 4 commits into
mainfrom
fix/image-gen-response-data

Conversation

@smakosh

@smakosh smakosh commented Apr 5, 2026

Copy link
Copy Markdown
Member

Summary

  • Fix Google image gen logs: Google provider image generation responses that return only images (no text) left content as null in logs, causing "No response content available." in the UI. Now sets content to "Image generated"/"Image edited" label — matching xAI, alibaba, bytedance, and zai providers.
  • Extract LogCard to shared package: Moved the ~1200-line activity log card component into @llmgateway/shared/components so both apps/ui and ee/admin use a single implementation. Variant-specific behavior (Next.js links, copy buttons, wording) is controlled via props.

Test plan

  • Verify image generation logs show "Image generated" content instead of "No response content available."
  • Verify activity log cards render correctly in apps/ui (with detail links, "your balance" wording)
  • Verify activity log cards render correctly in ee/admin (with copy buttons, log ID row, neutral wording)
  • Verify admin dashboard builds successfully

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved image response handling for Google-family providers — image labels are shown when no text content is present.
  • New Features

    • Unified, consistent Log Card UI across dashboard and admin with richer, expandable details for requests, responses, metrics, costs, plugins, tools, and errors.
    • Shared Log Card component added for consistent behavior and optional copy/detail links.

smakosh and others added 2 commits April 5, 2026 22:35
Google provider image generation responses that contain
only images (no text) left content as null, causing the
UI to show "No response content available." Now sets
content to "Image generated"/"Image edited" label like
other providers (xAI, alibaba, bytedance, zai).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extract the activity log card component into
@llmgateway/shared so both apps/ui and ee/admin
use a single implementation. Variant-specific
behavior (links, copy buttons, wording) is
controlled via props.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Apr 5, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 29c22746-cd1e-4fd2-9682-908650ad8cb1

📥 Commits

Reviewing files that changed from the base of the PR and between 490c43e and cad3963.

📒 Files selected for processing (1)
  • apps/gateway/src/chat/tools/parse-provider-response.ts
✅ Files skipped from review due to trivial changes (1)
  • apps/gateway/src/chat/tools/parse-provider-response.ts

Walkthrough

Consolidates LogCard UI into a new shared component and updates gateway provider parsing: Google-family image-generation responses now set content to imageLabel when images exist without extracted text; token-estimation fallback is tightened when images are present.

Changes

Cohort / File(s) Summary
Google Provider Response Parsing
apps/gateway/src/chat/tools/parse-provider-response.ts
When a Google-family provider response includes images but no extracted text, set content = imageLabel. Also restrict fallback token estimation to cases where content exists and there are no images; otherwise set completion token count to 0.
Shared LogCard Component
packages/shared/src/components/log-card.tsx
Adds a new client-side LogCard component and exported types (LogCardData, LogCardProps) implementing compact header, badges, metrics, expandable details, and many conditional panels for request/response metadata.
LogCard Barrel Export
packages/shared/src/components/index.tsx
Re-exported the new log-card component from the components barrel.
Shared Package Dependencies
packages/shared/package.json
Added runtime dependencies: date-fns@4.1.0, pretty-bytes@7.1.0.
UI & Admin Wrappers
apps/ui/src/components/dashboard/log-card.tsx, ee/admin/src/components/log-card.tsx
Replaced previous in-file implementations with thin wrappers delegating rendering to the shared LogCard, removing local formatting, expansion state, and detailed JSX.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

auto-merge

Suggested reviewers

  • steebchen
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the two main changes: fixing image generation log content and extracting LogCard into a shared component.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/image-gen-response-data

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (3)
packages/shared/src/components/log-card.tsx (2)

261-261: Replace any with unknown in cast.

Proposed fix
-	const params = log.params as Record<string, any> | undefined;
+	const params = log.params as Record<string, unknown> | undefined;

As per coding guidelines: "Never use any or as any in TypeScript unless absolutely necessary".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/shared/src/components/log-card.tsx` at line 261, The cast "const
params = log.params as Record<string, any> | undefined" uses `any`; change it to
`unknown` by casting to `Record<string, unknown> | undefined` (i.e., update the
declaration for `params`), and then update any downstream uses of `params` in
this component (e.g., property access, mapping, or JSON stringify) to use proper
type narrowing or explicit conversions (type guards, checks, or localized casts)
so you don't reintroduce `any` while keeping runtime behavior unchanged.

198-198: Avoid any in favor of unknown.

Per coding guidelines, any should not be used. The renderParams function already handles unknown value types via typeof checks.

Proposed fix
-function renderParams(obj: Record<string, any>, depth = 0): React.ReactNode {
+function renderParams(obj: Record<string, unknown>, depth = 0): React.ReactNode {

As per coding guidelines: "Never use any or as any in TypeScript unless absolutely necessary".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/shared/src/components/log-card.tsx` at line 198, The parameter type
of renderParams uses Record<string, any>; change it to Record<string, unknown>
and then update local usages inside renderParams to properly narrow values via
typeof checks (e.g., assign const val = obj[key] and treat val as unknown before
branching), replacing any other local "any" declarations with unknown and
refining checks for object/array/string/number/boolean/null to satisfy the
compiler; ensure function signature and all internal type assumptions (in
renderParams) are adjusted so no "any" remains.
ee/admin/src/components/log-card.tsx (1)

12-14: Use type-level assertion to validate shape compatibility instead of double cast.

The log as unknown as LogCardData pattern bypasses TypeScript's structural type checking. While ProjectLogEntry from the OpenAPI schema has all required fields (id, createdAt) and compatible types, a type assertion like satisfies LogCardData would validate the shape at compile time and make the intent clearer:

const logData = log satisfies LogCardData;
return <SharedLogCard log={logData} showCopyButtons showLogId />;

Alternatively, create a type helper to explicitly narrow the types or document why the cast is necessary.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ee/admin/src/components/log-card.tsx` around lines 12 - 14, Replace the
unsafe double-cast of log in the SharedLogCard props with a type-level shape
validation using TypeScript's satisfies operator (or a dedicated type-narrowing
helper): ensure the value passed to SharedLogCard's log prop is declared as
satisfying LogCardData (or explicitly convert ProjectLogEntry -> LogCardData via
a small mapper/validator) so the compiler checks structural compatibility;
update the usage around SharedLogCard (the log prop) to use that validated
variable and keep showCopyButtons and showLogId props as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/gateway/src/chat/tools/parse-provider-response.ts`:
- Around line 264-267: The current code sets content = imageLabel when no text
is present, which causes the downstream token estimation (using
candidatesTokenCount fallback) to count the synthetic label; instead, preserve
content for display but ensure token estimation uses the real extracted text
(textContent) or empty string, not content. Change the logic around the block
that assigns imageLabel to content (variables: content, images, imageLabel) so
you only set a displayValue or responseLabel for UI/logs while leaving content
unchanged, and update the token estimation path that references
candidatesTokenCount to fall back to textContent (or "") when
candidatesTokenCount is missing (symbols to update: content assignment,
textContent, candidatesTokenCount, and the token estimation call/site around
line ~393).

---

Nitpick comments:
In `@ee/admin/src/components/log-card.tsx`:
- Around line 12-14: Replace the unsafe double-cast of log in the SharedLogCard
props with a type-level shape validation using TypeScript's satisfies operator
(or a dedicated type-narrowing helper): ensure the value passed to
SharedLogCard's log prop is declared as satisfying LogCardData (or explicitly
convert ProjectLogEntry -> LogCardData via a small mapper/validator) so the
compiler checks structural compatibility; update the usage around SharedLogCard
(the log prop) to use that validated variable and keep showCopyButtons and
showLogId props as before.

In `@packages/shared/src/components/log-card.tsx`:
- Line 261: The cast "const params = log.params as Record<string, any> |
undefined" uses `any`; change it to `unknown` by casting to `Record<string,
unknown> | undefined` (i.e., update the declaration for `params`), and then
update any downstream uses of `params` in this component (e.g., property access,
mapping, or JSON stringify) to use proper type narrowing or explicit conversions
(type guards, checks, or localized casts) so you don't reintroduce `any` while
keeping runtime behavior unchanged.
- Line 198: The parameter type of renderParams uses Record<string, any>; change
it to Record<string, unknown> and then update local usages inside renderParams
to properly narrow values via typeof checks (e.g., assign const val = obj[key]
and treat val as unknown before branching), replacing any other local "any"
declarations with unknown and refining checks for
object/array/string/number/boolean/null to satisfy the compiler; ensure function
signature and all internal type assumptions (in renderParams) are adjusted so no
"any" remains.
🪄 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: 8d868af1-d651-4ae0-8907-1e64c2c56f00

📥 Commits

Reviewing files that changed from the base of the PR and between 1d821bf and 490c43e.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (6)
  • apps/gateway/src/chat/tools/parse-provider-response.ts
  • apps/ui/src/components/dashboard/log-card.tsx
  • ee/admin/src/components/log-card.tsx
  • packages/shared/package.json
  • packages/shared/src/components/index.tsx
  • packages/shared/src/components/log-card.tsx

Comment thread apps/gateway/src/chat/tools/parse-provider-response.ts
When Google candidatesTokenCount is missing and content
is the synthetic "Image generated" label, the fallback
was estimating ~2-3 tokens from the label string. Now
skips estimation when images are present (images.length
> 0) and sets rawCandidates to 0 instead.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@smakosh smakosh self-assigned this Apr 6, 2026
@smakosh
smakosh enabled auto-merge April 6, 2026 13:10
@smakosh
smakosh added this pull request to the merge queue Apr 6, 2026
Merged via the queue into main with commit 5c4d1ab Apr 6, 2026
31 of 32 checks passed
@smakosh
smakosh deleted the fix/image-gen-response-data branch April 6, 2026 13:25
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