Skip to content

Conversation

@akshaydeo
Copy link
Contributor

Summary

Briefly explain the purpose of this PR and the problem it solves.

Changes

  • What was changed and why
  • Any notable design decisions or trade-offs

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (Next.js)
  • Docs

How to test

Describe the steps to validate this change. Include commands and expected outcomes.

# Core/Transports
go version
go test ./...

# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build

If adding new configs or environment variables, document them here.

Screenshots/Recordings

If UI changes, add before/after screenshots or short clips.

Breaking changes

  • Yes
  • No

If yes, describe impact and migration instructions.

Related issues

Link related issues and discussions. Example: Closes #123

Security considerations

Note any security implications (auth, secrets, PII, sandboxing, etc.).

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 29, 2025

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced authentication error handling for improved API security. When required authentication headers are missing, the API now correctly returns HTTP 401 (Unauthorized) status code instead of 400 (Bad Request), providing more descriptive error messaging for improved clarity.
  • Tests

    • Updated test configuration for token counting operations.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Two distinct modifications: a test utility for token counting no longer specifies a Temperature parameter, and a governance plugin's error handling has been updated to return a 401 status code with a revised message when the x-bf-vk header is missing and mandatory.

Changes

Cohort / File(s) Summary
Test Configuration
core/internal/testutil/count_tokens.go
Removed explicit Temperature pointer (0.2) from ResponsesParameters struct in CountTokens test setup, transitioning to default/empty parameter set
Error Handling Updates
plugins/governance/main.go
Updated PreHook error response: status code changed from 400 to 401 when x-bf-vk header is mandatory and missing; error message updated to "x-bf-vk header is missing and is mandatory." Minor formatting change in TransportInterceptor (blank line addition)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A token test sheds its warmth so fine,
Default temperatures now align,
While headers that are quite divine,
Return a 401 in their design!

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 12-29-test_case_fixes

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f104cf9 and 97407fc.

⛔ Files ignored due to path filters (1)
  • core/go.sum is excluded by !**/*.sum
📒 Files selected for processing (2)
  • core/internal/testutil/count_tokens.go
  • plugins/governance/main.go

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

@akshaydeo akshaydeo marked this pull request as ready for review December 29, 2025 07:23
Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions
Copy link
Contributor

🧪 Test Suite Available

This PR can be tested by a repository admin.

Run tests for PR #1183

@akshaydeo akshaydeo merged commit 5f2e630 into main Dec 29, 2025
9 of 10 checks passed
@akshaydeo akshaydeo deleted the 12-29-test_case_fixes branch December 29, 2025 07:23
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.

Files API Support

2 participants