fix(log): show reasoning effort in activity log - #604
Conversation
Co-authored-by: contact <contact@polarlights.llc>
|
Cursor Agent can help with this pull request. Just |
WalkthroughIntroduces a reasoning_effort parameter through the gateway chat API, validates capability by model/provider, forwards it to provider requests, and records it in logs. Adds database column and schema/UI plumbing to persist and display reasoningEffort. Updates model metadata to mark reasoning-capable models. No pagination or tokenization changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway
participant Provider
participant DB
Client->>Gateway: POST /chat/completions { reasoning_effort? }
Gateway->>Gateway: Validate input schema
Gateway->>Gateway: Check model/provider supports reasoning if reasoning_effort set
alt Unsupported
Gateway-->>Client: 400 Bad Request (remove reasoning_effort or use supported model)
else Supported/Not provided
Gateway->>Gateway: prepareRequestBody(..., reasoning_effort)
Gateway->>DB: create log (reasoningEffort or null)
Gateway->>Provider: Request with reasoning_effort
Provider-->>Gateway: Response/Stream
Gateway->>DB: update/complete log
Gateway-->>Client: Response/Stream
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
steebchen
changed the title
Removed prompt and reasoning token displays from the dashboard log-card component as they are no longer relevant.
Added a new reasoning token display to the dashboard log-card component. Updated model configurations to support the reasoning capability.
…-effort-in-activity-log-5dfe
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (4)
apps/api/src/routes/logs.ts (1)
267-281: Remove TypeScript any and use a typed Drizzle query object.Type annotation : any violates the TS guideline for this repo. You can type the where clause using Drizzle’s query API or inline the filter via db.query.project.findMany.
Apply this diff to remove any and use a typed findMany:
- const projectsQuery: any = { - where: { - organizationId: { - in: orgId ? [orgId] : organizationIds, - }, - status: { - ne: "deleted", - }, - }, - }; + // Prefer direct typed query with conditional where composition + const projects = await db.query.project.findMany({ + where: (project, { inArray, and, ne, eq }) => + and( + inArray(project.organizationId, orgId ? [orgId] : organizationIds), + ne(project.status, "deleted"), + projectId ? eq(project.id, projectId) : undefined, + ), + }); - // If projectId is provided, check if it belongs to user's organizations - if (projectId) { - projectsQuery.where.id = projectId; - } - - const projects = await db.query.project.findMany(projectsQuery);apps/gateway/src/chat/chat.ts (3)
1231-1238: Remove ‘as any’ from zod transform for reasoning_effort (violates TS guideline).Per coding guidelines, avoid
as any. Replace the transform to normalize empty strings without usingany.Apply this diff:
- reasoning_effort: z - .enum(["low", "medium", "high"]) - .nullable() - .optional() - .transform((val) => (val === null || (val as any) === "" ? undefined : val)) + reasoning_effort: z + .union([z.enum(["low", "medium", "high"]), z.literal("")]) + .nullable() + .optional() + .transform((val) => (val === null || val === "" ? undefined : val)) .openapi({ description: "Controls the reasoning effort for reasoning-capable models", example: "medium", }),
1539-1541: Avoidas anywhen checking provider reasoning capability; use a type guard.Replace
(provider as any).reasoningwith a safe type guard. Also reuse the guard for logging.Apply this diff:
- const supportsReasoning = modelInfo.providers.some( - (provider) => (provider as any).reasoning === true, - ); + const supportsReasoning = modelInfo.providers.some(isReasoningCapableMapping); @@ - modelProviders: modelInfo.providers.map((p) => ({ - providerId: p.providerId, - reasoning: (p as any).reasoning, - })), + modelProviders: modelInfo.providers.map((p) => ({ + providerId: p.providerId, + reasoning: getReasoningFlag(p), + })),Add these helpers once in this file (outside the changed range), e.g. near the other helpers:
// Helper to safely detect reasoning flag on provider mappings without using `any` function hasReasoningFlag(p: unknown): p is { reasoning: unknown } { return typeof p === "object" && p !== null && "reasoning" in (p as Record<string, unknown>); } function getReasoningFlag(p: unknown): boolean | undefined { if (!hasReasoningFlag(p)) return undefined; const v = (p as { reasoning: unknown }).reasoning; return typeof v === "boolean" ? v : undefined; } function isReasoningCapableMapping(p: unknown): boolean { const v = getReasoningFlag(p); return v === true; }Also applies to: 1551-1554
2226-2229: SecondsupportsReasoningcomputation also usesas any; unify with the type guard.Reuse the same type guard to remove
as any. Optionally, compute this once and reuse instead of duplicating.Apply this diff:
- const supportsReasoning = modelInfo.providers.some( - (provider) => (provider as any).reasoning === true, - ); + const supportsReasoning = modelInfo.providers.some(isReasoningCapableMapping);Optional follow-up: compute
supportsReasoningonce aftermodelInfois resolved and pass the value through, rather than recomputing later.
🧹 Nitpick comments (1)
packages/db/src/schema.ts (1)
284-284: Optional: Constrain reasoningEffort to known values (low | medium | high).If we only intend low/medium/high, consider an enum at the API layer (and optionally a DB enum/check later) to prevent invalid values.
No schema diff required if enforced at API layer, but if you want DB-level constraint later, we can add a typed enum in a follow-up migration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/ui/src/lib/api/v1.d.tsis excluded by!**/v1.d.ts
📒 Files selected for processing (8)
apps/api/src/routes/logs.ts(1 hunks)apps/gateway/src/chat/chat.ts(10 hunks)apps/ui/src/components/dashboard/log-card.tsx(2 hunks)packages/db/migrations/1754925816_long_wildside.sql(1 hunks)packages/db/migrations/meta/1754925816_snapshot.json(1 hunks)packages/db/migrations/meta/_journal.json(1 hunks)packages/db/src/schema.ts(1 hunks)packages/models/src/models/openai.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/migrations/*.{js,ts,sql}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
For DB changes, do not write manual migration files
Files:
packages/db/migrations/1754925816_long_wildside.sql
packages/db/**/migrations/**/*.sql
📄 CodeRabbit Inference Engine (CLAUDE.md)
Do not write manual SQL migrations; use
pnpm pushto generate migrations
Files:
packages/db/migrations/1754925816_long_wildside.sql
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Use localStorage instead of cookies for client-side data persistence
Files:
packages/db/src/schema.tsapps/api/src/routes/logs.tsapps/ui/src/components/dashboard/log-card.tsxpackages/models/src/models/openai.tsapps/gateway/src/chat/chat.ts
**/*.{js,ts}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{js,ts}: Use drizzle with the latest object syntax for database operations
For read queries, always usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
packages/db/src/schema.tsapps/api/src/routes/logs.tspackages/models/src/models/openai.tsapps/gateway/src/chat/chat.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use
as anyor: anyin TypeScript files.
Files:
packages/db/src/schema.tsapps/api/src/routes/logs.tsapps/ui/src/components/dashboard/log-card.tsxpackages/models/src/models/openai.tsapps/gateway/src/chat/chat.ts
{apps/{api,gateway},packages/db}/**/*.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
{apps/{api,gateway},packages/db}/**/*.ts: Use Drizzle ORM with the latest object syntax in backend code
For reads, usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
packages/db/src/schema.tsapps/api/src/routes/logs.tsapps/gateway/src/chat/chat.ts
apps/ui/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
In apps/ui (a tanstack router project), always use navigate() for navigation
Files:
apps/ui/src/components/dashboard/log-card.tsx
apps/ui/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use
navigate()for programmatic navigation in the UI (TanStack Router)
Files:
apps/ui/src/components/dashboard/log-card.tsx
apps/{ui,docs}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use localStorage instead of cookies for client-side data persistence
Files:
apps/ui/src/components/dashboard/log-card.tsx
🧬 Code Graph Analysis (1)
apps/ui/src/components/dashboard/log-card.tsx (2)
packages/db/src/schema.ts (1)
log(243-304)apps/ui/src/lib/components/tooltip.tsx (3)
Tooltip(30-30)TooltipTrigger(30-30)TooltipContent(30-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e / run
- GitHub Check: build / run
🔇 Additional comments (5)
apps/api/src/routes/logs.ts (1)
58-58: LGTM: Exposed reasoningEffort in API response schema.Matches DB column shape (nullable text). Ensure UI handles nulls.
packages/models/src/models/openai.ts (1)
296-297: Reasoning support confirmed for Groq gpt-oss-120b
Confirmed that Groq’s GPT-OSS models (20B/120B) accept the OpenAI-stylereasoning_effortparameter and always return a parsedreasoningfield with token usage. The existingreasoning: true,flag in
packages/models/src/models/openai.ts(lines 296–297) is correct—no changes needed.apps/ui/src/components/dashboard/log-card.tsx (1)
353-368: Reasoning Effort UI: LGTM.Clear label, tooltip copy is concise, and value fallback to "-" is appropriate.
apps/gateway/src/chat/chat.ts (1)
2072-2077: Reasoning effort propagation and logging: LGTM.Good coverage: cached, streaming, non-streaming, error, and canceled paths all pass
reasoning_effortintocreateLogEntry, and DB persistence usesreasoningEffort || null.Also applies to: 2153-2158, 2329-2334, 2446-2450, 3146-3151, 3271-3276, 3341-3346, 3482-3486
packages/db/migrations/meta/1754925816_snapshot.json (1)
571-576: Schema snapshot includes reasoning_effort column: LGTM.The log table reflects the new nullable text column
reasoning_effort, matching upstream code paths and UI usage.
| <div className="text-muted-foreground">Reasoning Tokens</div> | ||
| <div>{log.reasoningTokens}</div> | ||
| <div className="text-muted-foreground">Total Tokens</div> | ||
| <div className="font-medium">{log.totalTokens}</div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Deduplicate “Reasoning Tokens” row and add a fallback for missing values.
“Reasoning Tokens” is rendered twice (once unconditionally and again conditionally), causing duplication when present. Also add a fallback to avoid rendering an empty cell when the value is absent.
[recommendation below removes the duplicate conditional block and adds a "-" fallback.]
Apply this diff:
@@
- <div className="text-muted-foreground">Reasoning Tokens</div>
- <div>{log.reasoningTokens}</div>
+ <div className="text-muted-foreground">Reasoning Tokens</div>
+ <div>{log.reasoningTokens ?? "-"}</div>
<div className="text-muted-foreground">Total Tokens</div>
<div className="font-medium">{log.totalTokens}</div>
- {log.reasoningTokens && (
- <>
- <div className="text-muted-foreground">
- Reasoning Tokens
- </div>
- <div>{log.reasoningTokens}</div>
- </>
- )}Also applies to: 171-178
🤖 Prompt for AI Agents
In apps/ui/src/components/dashboard/log-card.tsx around lines 167-170 (also
applies to 171-178), remove the duplicated "Reasoning Tokens" row and replace
both occurrences with a single label/value pair; render the value using a safe
fallback (e.g., log.reasoningTokens ?? "-" ) so an empty cell isn't shown when
the value is missing, and ensure no other conditional renders the same label
elsewhere.
| @@ -0,0 +1 @@ | |||
| ALTER TABLE "log" ADD COLUMN "reasoning_effort" text; No newline at end of file | |||
There was a problem hiding this comment.
Avoid manual SQL migrations; generate with pnpm push.
This repo’s guidelines prohibit manual SQL migrations. Please remove this file and regenerate via Drizzle so metadata stays consistent and naming matches the schema.
Suggested steps:
- Revert this manual migration and journal edits.
- Update schema.ts (if needed).
- From packages/db, run: pnpm push
- Commit the generated SQL and meta snapshot/journal.
If you must keep this change, also align the column name with the schema (reasoningEffort) or update schema to text("reasoning_effort") as suggested in schema.ts.
🤖 Prompt for AI Agents
In packages/db/migrations/1754925816_long_wildside.sql around lines 1 to 1, this
is a manual SQL migration which violates repo guidelines; remove this file and
revert any manual edits to the migration journal, ensure schema.ts reflects the
intended field (either rename to reasoningEffort in schema or explicitly use
text("reasoning_effort") there), then from the packages/db directory run pnpm
push to let Drizzle generate the correct migration SQL and updated meta/journal,
and finally commit the generated SQL and the meta snapshot/journal; if you
absolutely must keep the change, align the column name between schema.ts and the
SQL so names match.
| "idx": 37, | ||
| "version": "7", | ||
| "when": 1754925816984, | ||
| "tag": "1754925816_long_wildside", | ||
| "breakpoints": true | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Journal/meta should be generated, not hand-edited. Verify consistency.
Entries in the migration journal should be produced by Drizzle. Manual edits can desync snapshots from the actual schema.
Run this script to verify snapshot consistency and the schema-to-DB column mapping:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Check snapshot mentions reasoning_effort:"
rg -n '"reasoning_effort"' packages/db/migrations/meta/1754925816_snapshot.json -A 3 -B 3 || true
echo "Check current schema mapping for reasoningEffort:"
rg -n 'reasoningEffort:\s*text\(' packages/db/src/schema.ts -n -A 0 -B 2 || trueLength of output: 732
Migration journal consistent – switch to Drizzle autogeneration
Verified that:
- packages/db/migrations/meta/1754925816_snapshot.json (lines 571–573) defines
"reasoning_effort": { "type": "text" } - packages/db/src/schema.ts (line 284) defines
reasoningEffort: text()
Please regenerate the _journal.json via npx drizzle-kit generate:meta instead of hand-editing to prevent future drift.
🤖 Prompt for AI Agents
In packages/db/migrations/meta/_journal.json around lines 265 to 270, the
migration journal was hand-edited and may be out of sync with the
schema/snapshots (reasoning_effort type mismatch noted); regenerate the meta
journal using the Drizzle CLI by running `npx drizzle-kit generate:meta` from
the repo root so the file is auto-produced and consistent with
packages/db/migrations/meta/1754925816_snapshot.json and
packages/db/src/schema.ts, then commit the regenerated _journal.json instead of
editing it manually.
| topP: real(), | ||
| frequencyPenalty: real(), | ||
| presencePenalty: real(), | ||
| reasoningEffort: text(), |
There was a problem hiding this comment.
Fix DB column name mismatch with migration (reasoningEffort vs reasoning_effort).
The SQL migration adds reasoning_effort, but the schema defines reasoningEffort without an explicit column name. This will desync schema vs DB at runtime.
Apply this diff to map the schema to the actual DB column:
- reasoningEffort: text(),
+ reasoningEffort: text("reasoning_effort"),🤖 Prompt for AI Agents
In packages/db/src/schema.ts around line 284, the schema field reasoningEffort
is defined without an explicit DB column name while the migration created
reasoning_effort; update the schema to map this field to the actual column name
(reasoning_effort) so the ORM uses the correct column at runtime, then
regenerate/verify migrations and run any schema sync checks.
| reasoning: true, | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify Groq “openai/gpt-oss-20b” reasoning capability.
Same concern as above: ensure the gateway won’t pass unsupported parameters to Groq.
🌐 Web query:
Does Groq's "openai/gpt-oss-20b" support OpenAI reasoning features (e.g., reasoning_effort input, reasoning tokens in output usage)?
💡 Result:
Yes. On GroqCloud, openai/gpt-oss-20b supports OpenAI-style reasoning controls via the reasoning_effort input (low, medium, high), and its outputs include reasoning tokens that count toward usage when enabled.[6]
Essential details:
- Reasoning effort controls: The Groq API docs state that openai/gpt-oss-20b supports reasoning_effort values 'low', 'medium', or 'high' (default medium), used to control the model’s reasoning output.[6] Community launch notes also describe “full chain-of-thought reasoning with adjustable reasoning effort (low/medium/high).”[5]
- Reasoning tokens and usage: Groq’s docs indicate that reasoning can be exposed/hidden and that reasoning tokens are part of the model’s output behavior; these tokens contribute to usage accounting when reasoning is enabled.[6] The Groq community update likewise emphasizes adjustable reasoning that implies variable reasoning token generation.[5]
- API options caveat: include_reasoning (a boolean used by some providers to return a separate reasoning field) is noted as “not supported for GPT-OSS models” in Groq’s changelog; use reasoning_effort instead.[2]
What to pass and expect:
- Input: Set reasoning_effort: 'low' | 'medium' | 'high' in chat completions or compatible Responses API calls with model "openai/gpt-oss-20b".[6][2]
- Output: Reasoning tokens are generated according to the selected effort and are billed as part of output tokens; you can configure how reasoning is surfaced via reasoning/output options documented by Groq (note: include_reasoning is not available for GPT-OSS).[6][2]
Citations:
- 1: https://groq.com/blog/day-zero-support-for-openai-open-models
- 2: https://console.groq.com/docs/changelog
- 3: https://community.groq.com/news-and-updates-1
- 4: https://ai-sdk.dev/providers/ai-sdk-providers/groq
- 5: https://community.groq.com/groq-updates-2/openai-s-gpt-oss-now-available-plus-built-in-tools-and-responses-api-253
- 6: https://console.groq.com/docs/api-reference
- 7: https://www.morningstar.com/news/pr-newswire/20250805sf44920/groq-and-humain-launch-openais-new-open-models-day-zero
- 8: https://simonw.substack.com/p/gpt-5-key-characteristics-pricing
- 9: https://console.groq.com/docs/model/openai/gpt-oss-20b
- 10: https://openrouter.ai/openai/gpt-oss-20b/providers
Ensure Groq GPT-OSS-20B uses reasoning_effort, not reasoning
Groq’s OpenAI-compatible GPT-OSS-20B model only accepts a reasoning_effort setting ("low"│"medium"│"high") and does not support a boolean reasoning or include_reasoning flag. We need to map our reasoning: true into the proper Groq parameter (or omit it) to avoid sending unsupported fields.
• Location: packages/models/src/models/openai.ts (around lines 319–320)
• Change suggestion:
- reasoning: true,
+ // For openai/gpt-oss-20b, map boolean to Groq’s reasoning_effort
+ reasoning_effort: options.model === "openai/gpt-oss-20b"
+ ? (options.reasoning ? "medium" : undefined)
+ : undefined,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| reasoning: true, | |
| }, | |
| // For openai/gpt-oss-20b, map boolean to Groq’s reasoning_effort | |
| reasoning_effort: options.model === "openai/gpt-oss-20b" | |
| ? (options.reasoning ? "medium" : undefined) | |
| : undefined, | |
| }, |
🤖 Prompt for AI Agents
In packages/models/src/models/openai.ts around lines 319-320, the code sets
reasoning: true for Groq's GPT-OSS-20B which only accepts a reasoning_effort
string ("low"|"medium"|"high"); change the mapping so that when model is Groq
GPT-OSS-20B and our internal flag indicates reasoning, send reasoning_effort
with an appropriate value (e.g., "high" or configurable) instead of a boolean
reasoning/include_reasoning field, and ensure unsupported boolean keys are
omitted for that model.
2 participants
Display reasoning effort under model parameters and reasoning tokens alongside total and prompt tokens in the activity log card.
Summary by CodeRabbit