Skip to content

feat(logging): add usedModelMapping - #769

Merged
steebchen merged 4 commits into
mainfrom
terragon/add-used-model-mapping
Sep 10, 2025
Merged

steebchen merged 4 commits into
mainfrom
terragon/add-used-model-mapping

Conversation

@steebchen

@steebchen steebchen commented Sep 9, 2025 •

Copy link
Copy Markdown
Member

Summary

  • Introduces a new usedModelMapping field in the log schema to capture the original provider model name
  • Updates chat logging to include both the formatted model name (usedModelFormatted) and the original model mapping (usedModelMapping)
  • Enhances traceability of model usage by storing both LLMGateway format and original provider model names

Changes

Database Schema

  • Added usedModelMapping as an optional text field in the log table schema

Chat Application

  • Modified createLogEntry function to accept and store usedModelMapping
  • Updated multiple chat logging calls to pass both usedModelFormatted (LLMGateway format) and usedModelMapping (original provider model name)
  • Defined usedModelMapping as the original provider model name and usedModelFormatted as a combination of usedProvider and base model name

Test plan

  • Verify logs contain both usedModelFormatted and usedModelMapping fields with correct values
  • Test chat completions to ensure logging works without errors
  • Confirm database schema migration applies successfully and preserves existing data

🌿 Generated by Terry


ℹ️ Tag @terragon-labs to ask questions and address PR feedback

📎 Task: https://www.terragonlabs.com/task/1bd45e34-dd4b-4c83-9057-aa96f8191a87

Summary by CodeRabbit

  • Chores

    • Enhanced logging to record both the gateway-formatted model name and the underlying provider model across all chat flows (streaming, non-streaming, cached, errors, cancellations).
    • Database schema updated to store the provider model mapping (migration and metadata updated).
    • No changes to user-facing behavior or performance.
  • Tests

    • Updated end-to-end assertion to expect provider-prefixed model identifiers and verify the provider model mapping.

- Introduced usedModelMapping field in log schema to store original provider model name.
- Updated chat logging to include usedModelMapping alongside usedModelFormatted.
- Enables better tracking and differentiation of models used in requests.

Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Sep 9, 2025 •

Copy link
Copy Markdown
Contributor

Walkthrough

Adds recording of both gateway-formatted and provider-original model identifiers across chat logging: chat handler computes and passes usedModelFormatted and usedModelMapping; createLogEntry signature and log payload gain usedModelMapping; DB schema and migration add nullable used_model_mapping; e2e test expectations updated.

Changes

Cohort / File(s) Summary
Gateway chat logging updates
apps/gateway/src/chat/chat.ts
Compute usedModelMapping and usedModelFormatted; extend createLogEntry signature to accept usedModelMapping; update all logging call sites (streaming, non-streaming, cache, error, cancel) to pass both formatted and mapped model values; include usedModelMapping in log payload.
DB migration: log table
packages/db/migrations/1757376520_brief_roughhouse.sql
Add used_model_mapping TEXT column to log table.
Migration metadata
packages/db/migrations/meta/1757376520_snapshot.json, packages/db/migrations/meta/_journal.json
Add new migration snapshot reflecting schema change and append new journal entry.
DB schema definition
packages/db/src/schema.ts
Add usedModelMapping: text() field to log table declaration (nullable, positioned after usedModel and before usedProvider).
E2E test expectation
apps/gateway/src/api.e2e.ts
Update assertions to expect provider-prefixed model identifier (openai/gpt-5-nano) and verify log.usedModelMapping === "gpt-5-nano".

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant G as Gateway Chat Handler
  participant L as Logger
  participant DB as Database

  C->>G: Send chat request (provider, baseModelName)
  G->>G: derive usedModelMapping (original) and usedModelFormatted (provider/baseModelName)
  G->>L: createLogEntry(requestId, project, apiKey, providerKey?.id, usedModelFormatted, usedModelMapping, usedProvider, ...)
  L->>DB: insert log (includes used_model_mapping)
  G-->>C: return response / stream
  alt error or cancel
    G->>L: createLogEntry(..., status=error/cancel)
    L->>DB: insert/update log
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title “feat(logging): add usedModelMapping” concisely summarizes the primary change by indicating a new logging feature and the addition of the usedModelMapping field, which aligns directly with the main alterations in the migration, schema, and logging code changes. It is clear, specific, and focused on the central change without unnecessary detail.
Description Check ✅ Passed The pull request description clearly outlines the introduction of the new usedModelMapping field, describes the related database schema and chat application changes, and provides a coherent test plan, all of which directly relate to the changeset. It offers enough detail to understand the scope and impact of the changes without diverging into unrelated topics.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4ec7744 and d9d7851.

📒 Files selected for processing (1)
  • apps/gateway/src/api.e2e.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/gateway/src/api.e2e.ts
⏰ 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: build / run
  • GitHub Check: e2e / run
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch terragon/add-used-model-mapping

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.

@github-actions github-actions Bot changed the title Add usedModelMapping field to log and update chat logging feat(logging): add usedModelMapping Sep 9, 2025
@steebchen
steebchen marked this pull request as ready for review September 9, 2025 00:29

@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/db/src/schema.ts (1)

265-265: Add a brief field comment for clarity.

Consider documenting that usedModelMapping stores the original provider model name; usedModel holds the LLMGateway-formatted value. Helps avoid future confusion.

-	usedModelMapping: text(),
+	// Original provider model name (e.g., "gpt-4o-mini-2025-07-18")
+	usedModelMapping: text(),
apps/gateway/src/chat/chat.ts (2)

228-236: Name the parameter to reflect its semantics (formatted vs mapping).

createLogEntry now stores the formatted model string in usedModel and the provider original in usedModelMapping. Rename the parameter for precision and coerce mapping to null to avoid undefined flowing into inserts.

-function createLogEntry(
+function createLogEntry(
   requestId: string,
   project: Project,
   apiKey: ApiKey,
   providerKeyId: string | undefined,
-  usedModel: string,
-  usedModelMapping: string | undefined,
+  usedModelFormatted: string,
+  usedModelMapping: string | undefined,
   usedProvider: string,
@@
-    usedModel,
-    usedModelMapping,
+    usedModel: usedModelFormatted,
+    usedModelMapping: usedModelMapping ?? null,

2921-2946: All createLogEntry calls correctly match the updated signature. Consider refactoring createLogEntry to accept a single object parameter (e.g. { requestId, project, apiKey, providerKeyId, usedModel, … }) to eliminate positional-ordering risks as parameters grow.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 54f9033 and 6287e11.

📒 Files selected for processing (5)
  • apps/gateway/src/chat/chat.ts (11 hunks)
  • packages/db/migrations/1757376520_brief_roughhouse.sql (1 hunks)
  • packages/db/migrations/meta/1757376520_snapshot.json (1 hunks)
  • packages/db/migrations/meta/_journal.json (1 hunks)
  • packages/db/src/schema.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/migrations/*.{js,ts,sql}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

For DB changes, do not write manual migration files

Files:

  • packages/db/migrations/1757376520_brief_roughhouse.sql
**/{drizzle,migrations}/**/*.sql

📄 CodeRabbit inference engine (AGENTS.md)

Do not write manual migration files for DB changes

Files:

  • packages/db/migrations/1757376520_brief_roughhouse.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.ts
  • apps/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 use db().query.<table>.findMany() or db().query.<table>.findFirst()

Files:

  • packages/db/src/schema.ts
  • apps/gateway/src/chat/chat.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

Never use as any or : any in TypeScript files.

Files:

  • packages/db/src/schema.ts
  • apps/gateway/src/chat/chat.ts
apps/{api,gateway}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

apps/{api,gateway}/**/*.{ts,tsx}: Use Drizzle with the latest object syntax for database operations
For read queries, use db().query.

.findMany() or db().query.
.findFirst()

apps/{api,gateway}/**/*.{ts,tsx}: Use the Hono framework for backend HTTP services in apps/api and apps/gateway
Use Zod for request/response validation in backend routes and handlers
Maintain OpenAPI/Swagger documentation for backend APIs

Files:

  • apps/gateway/src/chat/chat.ts
apps/gateway/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

In apps/gateway (Hono), always use Hono + Zod + OpenAPI for validation and typesafety

Files:

  • apps/gateway/src/chat/chat.ts
⏰ 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). (1)
  • GitHub Check: e2e / run
🔇 Additional comments (4)
packages/db/migrations/1757376520_brief_roughhouse.sql (1)

1-1: LGTM: nullable column added without blocking legacy rows. Confirmed this migration was auto-generated by Luca Steeb (bot) in commit 6287e11, satisfying the “no manual migrations” rule.

packages/db/migrations/meta/1757376520_snapshot.json (1)

451-456: Snapshot includes used_model_mapping with correct nullability.

The snapshot reflects the new column on table log and keeps it nullable—good for back-compat.

apps/gateway/src/chat/chat.ts (2)

2629-2632: LGTM: clear separation between formatted and provider-native names.

usedModelFormatted and usedModelMapping values align with the new schema and response metadata.


4517-4537: API metadata keeps both representations—nice.

used_model (formatted) and underlying_used_model (provider native) in metadata mirror the DB fields; this improves traceability across logs and client responses.

Comment thread packages/db/migrations/meta/_journal.json

@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 (1)
apps/gateway/src/api.e2e.ts (1)

326-346: Extend validateLogs() to cover usedModelMapping presence.

Lightweight sanity checks keep all tests aligned with the new schema while avoiding provider-specific coupling.

   const log = logs[0];
   expect(log.usedProvider).toBeTruthy();

   expect(log.errorDetails).toBeNull();
   expect(log.finishReason).not.toBeNull();
   expect(log.unifiedFinishReason).not.toBeNull();
   expect(log.unifiedFinishReason).toBeTruthy();

   expect(log.usedModel).toBeTruthy();
   expect(log.requestedModel).toBeTruthy();
+  // New field added by this PR
+  expect(log).toHaveProperty("usedModelMapping");
+  expect(log.usedModelMapping === null || typeof log.usedModelMapping === "string").toBe(true);

   return log;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6287e11 and 4ec7744.

📒 Files selected for processing (1)
  • apps/gateway/src/api.e2e.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use localStorage instead of cookies for client-side data persistence

Files:

  • apps/gateway/src/api.e2e.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 use db().query.<table>.findMany() or db().query.<table>.findFirst()

Files:

  • apps/gateway/src/api.e2e.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

Never use as any or : any in TypeScript files.

Files:

  • apps/gateway/src/api.e2e.ts
apps/{api,gateway}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

apps/{api,gateway}/**/*.{ts,tsx}: Use Drizzle with the latest object syntax for database operations
For read queries, use db().query.

.findMany() or db().query.
.findFirst()

apps/{api,gateway}/**/*.{ts,tsx}: Use the Hono framework for backend HTTP services in apps/api and apps/gateway
Use Zod for request/response validation in backend routes and handlers
Maintain OpenAPI/Swagger documentation for backend APIs

Files:

  • apps/gateway/src/api.e2e.ts
apps/gateway/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

In apps/gateway (Hono), always use Hono + Zod + OpenAPI for validation and typesafety

Files:

  • apps/gateway/src/api.e2e.ts
**/*.e2e.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Name end-to-end tests with the .e2e.ts suffix (run by pnpm test:e2e)

Files:

  • apps/gateway/src/api.e2e.ts
**/*.{spec,e2e}.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Use Vitest as the test framework for unit and E2E tests

Files:

  • apps/gateway/src/api.e2e.ts
🧬 Code graph analysis (1)
apps/gateway/src/api.e2e.ts (1)
packages/db/src/schema.ts (1)
  • log (244-311)
⏰ 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: build / run
  • GitHub Check: e2e / run

Comment thread apps/gateway/src/api.e2e.ts
@steebchen
steebchen added this pull request to the merge queue Sep 9, 2025
@steebchen
steebchen removed this pull request from the merge queue due to a manual request Sep 9, 2025
Added check for `usedModelMapping` in API e2e test to ensure proper validation of the field.
@steebchen
steebchen added this pull request to the merge queue Sep 10, 2025
Merged via the queue into main with commit ad5cd6a Sep 10, 2025
10 of 11 checks passed
@steebchen
steebchen deleted the terragon/add-used-model-mapping branch September 10, 2025 00:18
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