Skip to content

fix: preserve chat max_tokens during param filtering - #3992

Merged
akshaydeo merged 7 commits into
devfrom
06-02-fix_preserve_chat_max_tokens_during_param_filtering
Jun 2, 2026
Merged

fix: preserve chat max_tokens during param filtering#3992
akshaydeo merged 7 commits into
devfrom
06-02-fix_preserve_chat_max_tokens_during_param_filtering

Conversation

@sammaji

@sammaji sammaji commented Jun 2, 2026

Copy link
Copy Markdown
Member

Summary

When a request includes max_tokens, it gets converted to max_completion_tokens before the compat plugin's PreLLMHook runs. This meant that if a provider supported max_tokens but not max_completion_tokens, the parameter was being incorrectly dropped, resulting in token limit constraints being silently ignored.

Changes

  • Updated the dropUnsupportedParams logic to retain max_completion_tokens if either max_completion_tokens or max_tokens is listed as a supported parameter for the provider, reflecting the fact that max_tokens is normalized into max_completion_tokens prior to this check.

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

Send a request to a provider that supports max_tokens (but not max_completion_tokens) with a max_tokens value set. Verify that the parameter is not dropped and that the provider receives the token limit as expected.

go test ./plugins/compat/...

Breaking changes

  • Yes
  • No

Related issues

Security considerations

None.

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

Summary by CodeRabbit

  • Bug Fixes
    • Improved token parameter compatibility handling to ensure parameters are only dropped when neither supported variant is available, reducing unnecessary parameter removal.

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ba06c2b1-4b80-46b6-9870-0452e4b49e3c

📥 Commits

Reviewing files that changed from the base of the PR and between c4cac97 and b5b9afd.

📒 Files selected for processing (1)
  • plugins/compat/dropparams.go

📝 Walkthrough

Walkthrough

This PR refines parameter compatibility handling in the compat plugin by updating dropUnsupportedParams to preserve max_completion_tokens when max_tokens is supported. Inline comments clarify the prior max_tokensmax_completion_tokens conversion timing.

Changes

Max completion tokens handling

Layer / File(s) Summary
Max completion tokens parameter compatibility
plugins/compat/dropparams.go
Updated conditional logic for dropping max_completion_tokens to only drop it when neither max_completion_tokens nor max_tokens is supported. Added inline comments documenting the max_tokensmax_completion_tokens conversion timing before the PreLLMHook.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A token by any other name,
Now handled with more careful aim—
Preserve when the sister-param's near,
With comments bright and intent clear! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: preserving max_tokens during parameter filtering in the compat plugin.
Description check ✅ Passed The description follows the template structure with all critical sections completed: Summary, Changes, Type of change, Affected areas, How to test, Breaking changes, and Checklist all properly filled out.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 06-02-fix_preserve_chat_max_tokens_during_param_filtering

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


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

@sammaji
sammaji changed the base branch from 05-29-chore_updated_docs_for_extra_params_passthrough to graphite-base/3992 June 2, 2026 14:16
@sammaji
sammaji force-pushed the 06-02-fix_preserve_chat_max_tokens_during_param_filtering branch from 8d82eff to b5b9afd Compare June 2, 2026 14:17
@sammaji
sammaji changed the base branch from graphite-base/3992 to 06-01-chore_update_provider_management_docs June 2, 2026 14:17
@sammaji
sammaji marked this pull request as ready for review June 2, 2026 14:18
@greptile-apps

greptile-apps Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 4/5

The change is a narrow, correct guard extension in a single function; the logic is easy to reason about and the bug it fixes is well-scoped to providers whose catalog entry uses max_tokens instead of max_completion_tokens.

The fix correctly extends the drop condition to cover the aliased parameter name, and the rest of the file is untouched. The main gap is the complete absence of unit tests in the compat package — there is no automated check to catch a future regression on any of the four alias combinations.

plugins/compat/dropparams.go — the fix is correct but the package has no test files, so the new aliasing behaviour is not verified by any automated test.

Important Files Changed

Filename Overview
plugins/compat/dropparams.go Single-line fix correctly extends the max_completion_tokens drop guard to also check isSupported["max_tokens"], fixing silent token-limit drops for providers that list max_tokens rather than max_completion_tokens; no test file exists in the package to cover this or regression scenarios.

Reviews (1): Last reviewed commit: "fix: preserve chat max_tokens during par..." | Re-trigger Greptile

Comment thread plugins/compat/dropparams.go

akshaydeo commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Merge activity

  • Jun 2, 4:44 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jun 2, 4:50 PM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo
akshaydeo changed the base branch from 06-01-chore_update_provider_management_docs to graphite-base/3992 June 2, 2026 16:49
@akshaydeo
akshaydeo changed the base branch from graphite-base/3992 to dev June 2, 2026 16:49
@akshaydeo
akshaydeo requested a review from a team as a code owner June 2, 2026 16:49
@akshaydeo
akshaydeo merged commit a852d9d into dev Jun 2, 2026
11 checks passed
@akshaydeo
akshaydeo deleted the 06-02-fix_preserve_chat_max_tokens_during_param_filtering branch June 2, 2026 16:50
akshaydeo pushed a commit that referenced this pull request Jun 2, 2026
## Summary

When a request includes `max_tokens`, it gets converted to `max_completion_tokens` before the compat plugin's `PreLLMHook` runs. This meant that if a provider supported `max_tokens` but not `max_completion_tokens`, the parameter was being incorrectly dropped, resulting in token limit constraints being silently ignored.

## Changes

- Updated the `dropUnsupportedParams` logic to retain `max_completion_tokens` if either `max_completion_tokens` or `max_tokens` is listed as a supported parameter for the provider, reflecting the fact that `max_tokens` is normalized into `max_completion_tokens` prior to this check.

## Type of change

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

## Affected areas

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

## How to test

Send a request to a provider that supports `max_tokens` (but not `max_completion_tokens`) with a `max_tokens` value set. Verify that the parameter is not dropped and that the provider receives the token limit as expected.

```sh
go test ./plugins/compat/...
```

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None.

## Checklist

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

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->

## Summary by CodeRabbit

* **Bug Fixes**
  * Improved token parameter compatibility handling to ensure parameters are only dropped when neither supported variant is available, reducing unnecessary parameter removal.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
akshaydeo pushed a commit that referenced this pull request Jun 4, 2026
## Summary

When a request includes `max_tokens`, it gets converted to `max_completion_tokens` before the compat plugin's `PreLLMHook` runs. This meant that if a provider supported `max_tokens` but not `max_completion_tokens`, the parameter was being incorrectly dropped, resulting in token limit constraints being silently ignored.

## Changes

- Updated the `dropUnsupportedParams` logic to retain `max_completion_tokens` if either `max_completion_tokens` or `max_tokens` is listed as a supported parameter for the provider, reflecting the fact that `max_tokens` is normalized into `max_completion_tokens` prior to this check.

## Type of change

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

## Affected areas

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

## How to test

Send a request to a provider that supports `max_tokens` (but not `max_completion_tokens`) with a `max_tokens` value set. Verify that the parameter is not dropped and that the provider receives the token limit as expected.

```sh
go test ./plugins/compat/...
```

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None.

## Checklist

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

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->

## Summary by CodeRabbit

* **Bug Fixes**
  * Improved token parameter compatibility handling to ensure parameters are only dropped when neither supported variant is available, reducing unnecessary parameter removal.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@akshaydeo akshaydeo mentioned this pull request Jun 5, 2026
18 tasks
akshaydeo added a commit that referenced this pull request Jun 6, 2026
## Summary

This PR bumps the Go toolchain version from `1.26.3` to `1.26.4` across all modules and CI workflows, and cuts a new release (`core` v1.5.17, `framework` v1.3.17, `transports` v1.5.9, `plugins/compat` v0.1.16, `plugins/governance` v1.5.17, and associated plugin versions) incorporating a large batch of features and fixes accumulated since the previous release.

## Changes

- **Go 1.26.4** — Updated `go-version` in all GitHub Actions workflows (`e2e-tests`, `helm-release`, `pr-tests`, `release-cli`, `release-pipeline`, `snyk`) and all `go.mod` files (core, framework, transports, cli, all plugins, examples, and test modules).
- **Core (v1.5.17)** — OpenAI compaction support, multi-customer logs and usage tracking, multiple team/business unit support, `request_headers` wildcard pattern capture for OTel and Maxim plugins, xAI `x_search` tool, fetch URL validation with SSRF hardening, `file://` pricing URL scheme, virtual key provider fan-out filtering, and a broad set of fixes including Anthropic prompt cache key, empty thinking block stripping, OpenAI stream usage event cleanup, Gemini numeric schema constraints, stale connection retries, Azure Claude diagnostic strip, and passthrough budget handling.
- **Framework (v1.3.17)** — Scope-aware budgets and limits wired from model configs, provider-level governance, multiple customer budget support with `calendar_aligned` windows, paginated virtual key fetch, `config.json` source-of-truth flow, FTS index cap reduction, sync worker drift fix, cascade deletes for model configs, and high-scale virtual key flow improvements.
- **Transports (v1.5.9)** — Full changelog covering all of the above plus UI improvements (log navigation, customer detail sheet, `BudgetDisplay` component, inline loading shell, materialized view alias), SCIM provisioning fields, Helm/config schema additions (`roles`, `per_user_oauth`), client IP resolution from forwarded headers, and dependency upgrades (`recharts` to 3.8.1, `golang.org/x` CVE remediation).
- **Plugins** — `governance` v1.5.17 adds team budget/rate-limit exporters, ghost node reconciliation fix, and VK double usage counting fix; `logging` v1.5.17 adds wildcard header capture and file attachment rendering; `otel` v1.2.17 adds `disable_content_logging` and multiple collectors support; `maxim` v1.6.17 adds `request_headers` wildcard capture; `compat` v0.1.16 fixes `max_tokens` preservation during param filtering.

## Type of change

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

## Affected areas

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

## How to test

```sh
# Verify Go version
go version  # should report go1.26.4

# Run core tests
cd core && go test ./...

# Run framework tests
cd framework && go test ./...

# Run transports tests
cd transports && go test ./...

# Run plugin tests
cd plugins/governance && go test ./...
cd plugins/logging && go test ./...
cd plugins/otel && go test ./...

# UI
cd ui
pnpm i
pnpm build
pnpm test
```

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

#4053, #4066, #4041, #4012, #3976, #3947, #3991, #4045, #3957, #3938, #3937, #3939, #3981, #3998, #3997, #4092, #4091, #4079, #4080, #4086, #3929, #3994, #4028, #3970, #3919, #3861, #3664, #3999, #4088, #4070, #4051, #4043, #4057, #4023, #3941, #3955, #4024, #3956, #3967, #3925, #3992, #3900

## Security considerations

- Fetch URL validation hardened against SSRF by tightening IP checks for private networks and link-local addresses (#4092, #3947, #3991).
- Transitive `golang.org/x` dependencies (crypto, net, sys, text) bumped to address Docker Scout CVEs (#3900).

## Checklist

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

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit

* **New Features**
  * OpenAI compaction, multi-customer/team logstore support, request-header wildcard capture, enhanced governance (provider-level & scope-aware limits), disable-content-logging option, support for multiple OpenTelemetry collectors, SSRF hardening and URL validation.

* **Chores**
  * Bumped Go toolchain across modules and updated component/plugin version releases.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@akshaydeo akshaydeo mentioned this pull request Jun 7, 2026
akshaydeo pushed a commit that referenced this pull request Jun 7, 2026
## Summary

When a request includes `max_tokens`, it gets converted to `max_completion_tokens` before the compat plugin's `PreLLMHook` runs. This meant that if a provider supported `max_tokens` but not `max_completion_tokens`, the parameter was being incorrectly dropped, resulting in token limit constraints being silently ignored.

## Changes

- Updated the `dropUnsupportedParams` logic to retain `max_completion_tokens` if either `max_completion_tokens` or `max_tokens` is listed as a supported parameter for the provider, reflecting the fact that `max_tokens` is normalized into `max_completion_tokens` prior to this check.

## Type of change

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

## Affected areas

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

## How to test

Send a request to a provider that supports `max_tokens` (but not `max_completion_tokens`) with a `max_tokens` value set. Verify that the parameter is not dropped and that the provider receives the token limit as expected.

```sh
go test ./plugins/compat/...
```

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None.

## Checklist

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

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->

## Summary by CodeRabbit

* **Bug Fixes**
  * Improved token parameter compatibility handling to ensure parameters are only dropped when neither supported variant is available, reducing unnecessary parameter removal.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
akshaydeo added a commit that referenced this pull request Jun 7, 2026
## ✨ Features

- **OpenAI Compaction** — Added OpenAI conversation compaction support
across core, framework, logging, and the API surface (#4053)
- **Multi-Customer & Org Hierarchy** — Logs and usage tracking now
support multiple customers, teams, and business units, including
business unit CRUD, team assignment, and governance endpoints in the
OpenAPI spec (#4066, #4041, #4082)
- **Provider-Level Governance** — Budgets & limits are now scope-aware
and can be applied at the virtual-key top level and per provider, wired
from the model configs table, with UI filters for scope and providers
(#3938, #3937, #3939, #3981, #3962)
- **Customer Budgets** — Customers support multiple budgets and
`calendar_aligned` budget windows (#3998, #3997)
- **Virtual Key Attribution & Controls** — Added a `created_by` user
attribution column and a `blacklisted_models` column for virtual key
provider configs (#3672, #3653)
- **Request Header Capture** — OTel and Maxim observability plugins
capture `request_headers` by pattern, with wildcard support (e.g.
`x-custom-*`); logging gained the same wildcard header capture (#4012,
#3958)
- **OTel Content Controls & Collectors** — New `disable_content_logging`
option drops message/tool content from exported spans, plus support for
multiple OTel collectors (#4064, #3894)
- **xAI x_search** — Added xAI `x_search` tool support (#3976)
- **URL Validation** — Added fetch URL validation with private-network
configuration and link-local blocking (#3947, #3991)
- **File Scheme Pricing URLs** — Pricing source URLs now accept the
`file://` scheme for air-gapped and self-hosted deployments (#4045)
- **Paginated Virtual Keys** — Virtual key fetching is paginated to
handle deployments with very large numbers of keys (#3957)
- **Client IP Resolution** — Resolve client IP from
`X-Forwarded-For`/`X-Real-IP` headers
- **SCIM Provisioning** — Added `attributeType`/`attributeValue` SCIM
provisioning fields
- **Helm/Config Schema** — Added `roles` RBAC governance config and
`per_user_oauth` MCP auth to the Helm chart and config schema (#4004,
#4009)
- **Log Navigation UI** — Added a "View logs" menu item to customer,
team, and virtual key tables, clickable links in log detail views, a
customer detail sheet, and a reusable `BudgetDisplay` component (#4073,
#4054, #4026, #4055)
- **Faster First Paint** — Added an inline loading shell to `#root`
before React mounts (#4063)
- **Materialized View Alias** — Added an `alias` column to the
materialized view with filter support (#4078)

## 🐞 Fixed

- **Fetch URL IP Checks** — Hardened fetch URL IP checks against SSRF
(#4092)
- **Mantle Model Matching** — Broadened Mantle model matching to all
`gpt` variants (#4091)
- **Empty Thinking Blocks** — Strip thinking blocks when the signature
is empty (#4079)
- **OpenAI Stream Usage** — Removed usage from the `responses.created`
event in the OpenAI stream (#4080)
- **Prompt Cache Key** — Set the prompt cache key from the Anthropic
integration (#4086)
- **Upstream Failure Status** — Map upstream connection failures to 502
instead of 400 (#3929) (thanks
[@chris-colinsky](https://github.com/chris-colinsky)!)
- **Gemini Schema Constraints** — Accept numeric schema integer
constraints for Gemini (#3994) (thanks
[@yanhao98](https://github.com/yanhao98)!)
- **Files Provider Param** — Accept the `?provider=` query param on `GET
/v1/files` (#3971) (thanks [@alexef](https://github.com/alexef)!)
- **Optional Batch Model** — Made the `model` field optional on `POST
/v1/batches` (#3973) (thanks [@alexef](https://github.com/alexef)!)
- **Helm Azure Config** — Added missing `azure_key_config` fields to the
Helm schema (#3996) (thanks
[@axelray-dev](https://github.com/axelray-dev)!)
- **Text Completion Chunk Model** — Added the missing `Model` field to
`TextCompletionChunkResponse` (#3970) (thanks
[@kuishou68](https://github.com/kuishou68)!)
- **MCP Inline stdio Env** — MCP stdio server configs accept inline
environment variable assignments (#3861) (thanks
[@Shushmitaaaa](https://github.com/Shushmitaaaa)!)
- **Orphaned Tool Results** — Orphaned tool results in the OpenAI to
Anthropic conversion flow are no longer rejected by the Anthropic API
(#3919)
- **Node Usage Reconciliation** — Added a monotonic `inc_number` log
cursor so node usage reconciliation does not skip late async log writes
(#3664)
- **Bedrock Output Assessments** — Corrected the type of
`outputAssessments` in Bedrock responses (#4028)
- **Model Pool Pricing Reloads** — Preserve non-pricing model pool
entries across pricing reloads (#3999)
- **Ghost Node Reconciliation** — Replicate the VK hierarchy flow for
ghost node reconciliation (#4088)
- **VK Double Usage Counting** — Fixed double usage counting when
creating a virtual key (#4070)
- **Model Config Lifecycle** — Cascade deletes for model configs and
removal of stale in-memory model configs (#4051, #4043)
- **FTS Index Cap** — Reduced the FTS index `left()` cap from 800k to
250k chars to stay within the tsvector limit (#4057)
- **Sync Worker Drift** — Reduced the sync worker ticker period to 5m to
prevent threshold drift (#4023)
- **Passthrough** — Fixed passthrough budgets, gated passthrough models
per VK, model extraction for Azure passthrough, and restricted
fallbacks/provider selection to the VK boundary (#3941, #3988, #3983,
#3924)
- **Provider Response Headers** — Strip provider response headers and
add a content-type filter (#3955, #4024)
- **Stream Handling** — Drain non-SSE stream readers and retry stale
connections (#3956, #3967)
- **Azure Claude** — Strip Azure diagnostic property for Claude models
(#3925)
- **Compat max_tokens** — Preserve chat `max_tokens` during param
filtering (#3992)
- **Raw Request Flag** — Removed the raw request flag from providers
that don't support it (#4058)
- **UI Fixes** — Standardized page container layout, virtual key model
configs UI, and dashboard chart tooltips (#4046, #4052, #4044)

## 🔧 Maintenance

- **Dependency Upgrades** — Bumped transitive `golang.org/x`
dependencies (crypto, net, sys, text) for Docker Scout CVE remediation
and `recharts` to 3.8.1; cascaded version bumps across all modules
(#3900, #4003)
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.

2 participants