Skip to content

fix: vertex count tokens unsupported fields - #5619

Merged
akshaydeo merged 1 commit into
devfrom
07-28-fix_vertex_count_tokens_unsupported_fields
Jul 29, 2026
Merged

fix: vertex count tokens unsupported fields#5619
akshaydeo merged 1 commit into
devfrom
07-28-fix_vertex_count_tokens_unsupported_fields

Conversation

@TejasGhatte

@TejasGhatte TejasGhatte commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

The Vertex countTokens endpoint rejects several fields that are valid in generateContent requests, causing 400 errors. Previously, the field-stripping logic incorrectly removed generationConfig and systemInstruction, which the countTokens endpoint actually supports and uses to produce accurate token counts. This PR fixes the set of fields being stripped and consolidates the logic into a dedicated helper.

Changes

  • Introduced stripVertexCountTokensUnsupportedFields to replace the inline ad-hoc field deletions in CountTokens. The new helper drops only the fields the countTokens endpoint actually rejects: toolConfig, safetySettings, cachedContent, serviceTier, and labels.
  • Removed the incorrect stripping of generationConfig and systemInstruction, both of which are valid and meaningful to the countTokens endpoint.
  • Added unit tests covering: fields that should be preserved, fields that should be dropped, and a nil/empty body edge case.

Type of change

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

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

go test ./core/providers/vertex/...

Verify that TestStripVertexCountTokensUnsupportedFields passes, confirming that systemInstruction, tools, and generationConfig survive stripping while toolConfig, safetySettings, cachedContent, serviceTier, and labels are removed.

Screenshots/Recordings

N/A

Breaking changes

  • Yes
  • No

Related issues

Security considerations

No security implications. This change only affects which JSON fields are forwarded to the Vertex countTokens endpoint.

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

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Vertex CountTokens request sanitization now uses a centralized helper that removes unsupported generateContent fields. A new test covers passthrough, field removal, and empty input behavior.

Changes

Vertex CountTokens sanitization

Layer / File(s) Summary
Centralize CountTokens field stripping
core/providers/vertex/vertex.go
A helper removes fields rejected by Vertex countTokens, and CountTokens uses it instead of inline deletions.
Test sanitization behavior
core/providers/vertex/payload_ordering_test.go
Tests verify supported-field passthrough, unsupported-field removal, and empty or nil input handling.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: akshaydeo, pratham-mishra04

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR fixes Vertex countTokens stripping, but linked issue #123 requires Files API support, so the main requirement is unmet. Implement the Files API support requested in #123 or relink the PR to the correct issue.
Out of Scope Changes check ⚠️ Warning All code changes are unrelated to Files API support and therefore fall outside the linked issue's requested scope. Remove or retarget this change set so it addresses the Files API support requested by #123.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title is concise and accurately summarizes the main change to Vertex countTokens field stripping.
Description check ✅ Passed The description follows the template well, covering summary, changes, type, affected areas, testing, breaking changes, security, and checklist.
✨ 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 07-28-fix_vertex_count_tokens_unsupported_fields

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

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


tejas ghatte seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@TejasGhatte
TejasGhatte marked this pull request as ready for review July 28, 2026 12:59

TejasGhatte commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

akshaydeo commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Merge activity

  • Jul 29, 11:07 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jul 29, 11:07 AM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo
akshaydeo merged commit c0bb8dc into dev Jul 29, 2026
15 of 16 checks passed
@akshaydeo
akshaydeo deleted the 07-28-fix_vertex_count_tokens_unsupported_fields branch July 29, 2026 11:07
akhsaul pushed a commit to akhsaul/bifrost that referenced this pull request Aug 27, 2026
## Summary

The Vertex `countTokens` endpoint rejects several fields that are valid in `generateContent` requests, causing 400 errors. Previously, the field-stripping logic incorrectly removed `generationConfig` and `systemInstruction`, which the `countTokens` endpoint actually supports and uses to produce accurate token counts. This PR fixes the set of fields being stripped and consolidates the logic into a dedicated helper.

## Changes

- Introduced `stripVertexCountTokensUnsupportedFields` to replace the inline ad-hoc field deletions in `CountTokens`. The new helper drops only the fields the `countTokens` endpoint actually rejects: `toolConfig`, `safetySettings`, `cachedContent`, `serviceTier`, and `labels`.
- Removed the incorrect stripping of `generationConfig` and `systemInstruction`, both of which are valid and meaningful to the `countTokens` endpoint.
- Added unit tests covering: fields that should be preserved, fields that should be dropped, and a nil/empty body edge case.

## Type of change

- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./core/providers/vertex/...
```

Verify that `TestStripVertexCountTokensUnsupportedFields` passes, confirming that `systemInstruction`, `tools`, and `generationConfig` survive stripping while `toolConfig`, `safetySettings`, `cachedContent`, `serviceTier`, and `labels` are removed.

## Screenshots/Recordings

N/A

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

No security implications. This change only affects which JSON fields are forwarded to the Vertex `countTokens` endpoint.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
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.

3 participants