feat(logs): map OTHER to UNKNOWN - #1343
Conversation
![terragon-labs[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
…NKNOWN Add handling for the finishReason value 'OTHER' in the getUnifiedFinishReason function to return UnifiedFinishReason.UNKNOWN. Also add corresponding test coverage to ensure proper behavior. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
WalkthroughThis PR extends the finish reason mapping for Google-related providers (google-ai-studio and google-vertex) by adding a case that maps the "OTHER" finish reason to UnifiedFinishReason.UNKNOWN. A corresponding test is added to verify this mapping behavior. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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 |
github-actions
Bot
changed the title
There was a problem hiding this comment.
Pull request overview
This PR adds mapping for Google AI Studio's "OTHER" finish reason to UnifiedFinishReason.UNKNOWN, ensuring non-standard finish reasons are properly categorized for safer downstream handling.
Key Changes:
- Add explicit handling for "OTHER" finish reason in Google AI Studio/Vertex cases
- Extend test coverage to verify the new mapping behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| apps/gateway/src/lib/logs.ts | Adds "OTHER" finish reason mapping to UNKNOWN for google-ai-studio and google-vertex providers |
| apps/gateway/src/lib/logs.spec.ts | Adds test case to verify "OTHER" finish reason is correctly mapped to UNKNOWN |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/gateway/src/lib/logs.ts (1)
70-72: Explicit mapping for "OTHER" improves clarity but triggers error logging.The mapping of "OTHER" to UNKNOWN is correct for Google finish reasons. Since "OTHER" is a documented, legitimate Google finish reason (not an error condition), mapping it to UNKNOWN will cause error logging at lines 139-149 whenever it occurs. Consider whether this is intended—if "OTHER" should not produce error logs, add it to the known reasons that bypass logging.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/gateway/src/lib/logs.spec.ts(1 hunks)apps/gateway/src/lib/logs.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never useanyoras anyunless absolutely necessary in TypeScript code
For database reads: Usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
apps/gateway/src/lib/logs.tsapps/gateway/src/lib/logs.spec.ts
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use tabs for indentation
Files:
apps/gateway/src/lib/logs.tsapps/gateway/src/lib/logs.spec.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use top-levelimport, never use require or dynamic imports
No unnecessary code comments
Files:
apps/gateway/src/lib/logs.tsapps/gateway/src/lib/logs.spec.ts
apps/{gateway,api}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Hono framework with Zod validation and OpenAPI documentation for backend APIs
Files:
apps/gateway/src/lib/logs.tsapps/gateway/src/lib/logs.spec.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always use top-level
import, never use require or dynamic imports
Files:
apps/gateway/src/lib/logs.tsapps/gateway/src/lib/logs.spec.ts
{apps/api,apps/gateway,packages/db}/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
{apps/api,apps/gateway,packages/db}/**/*.ts: Use Drizzle ORM with latest object syntax for database operations
For database reads: Usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
apps/gateway/src/lib/logs.tsapps/gateway/src/lib/logs.spec.ts
apps/{gateway,api}/src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/{gateway,api}/src/**/*.ts: Use Hono for backend framework in Gateway and API services
Use Zod schemas for validation in Hono services
Files:
apps/gateway/src/lib/logs.tsapps/gateway/src/lib/logs.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Unit tests should use
*.spec.tsfile naming convention
Files:
apps/gateway/src/lib/logs.spec.ts
🧬 Code graph analysis (1)
apps/gateway/src/lib/logs.ts (1)
packages/db/src/schema.ts (2)
UnifiedFinishReason(20-30)UnifiedFinishReason(32-33)
⏰ 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). (11)
- GitHub Check: Upload results
- GitHub Check: e2e-shards (4)
- GitHub Check: e2e-shards (5)
- GitHub Check: e2e-shards (2)
- GitHub Check: e2e-shards (1)
- GitHub Check: e2e-shards (3)
- GitHub Check: build / run
- GitHub Check: test / run
- GitHub Check: generate / run
- GitHub Check: lint / run
- GitHub Check: autofix
🔇 Additional comments (1)
apps/gateway/src/lib/logs.spec.ts (1)
66-68: Test correctly validates the new mapping.The test appropriately verifies that "OTHER" maps to UNKNOWN for Google AI Studio. Testing only google-ai-studio (not google-vertex) is sufficient since they share the same case block in the implementation.
Summary
UnifiedFinishReason.UNKNOWNin the unified finish reason logic.Changes
Core Logic
getUnifiedFinishReasoninapps/gateway/src/lib/logs.tsto returnUnifiedFinishReason.UNKNOWNwhenfinishReasonis "OTHER" (in addition to existing handling for content filter scenarios).Tests
logs.spec.tsto cover the new edge case:finishReasonis "OTHER" for provider "google-ai-studio", the function should returnUnifiedFinishReason.UNKNOWN.Rationale
Test plan
getUnifiedFinishReason("OTHER", "google-ai-studio")yieldsUnifiedFinishReason.UNKNOWN.Notes
🌿 Generated by Terry
ℹ️ Tag @terragon-labs to ask questions and address PR feedback
📎 Task: https://www.terragonlabs.com/task/55e97d7e-4f72-4978-be42-96f307c503fa
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.