Skip to content

handles compaction message type - #3966

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

handles compaction message type#3966
akshaydeo merged 7 commits into
devfrom
06-02-handles_compaction_message_type

Conversation

@akshaydeo

@akshaydeo akshaydeo commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Summary

OpenAI's Responses API rejects requests containing a summary field on compaction input items with an "Unknown parameter" error. Because Bifrost has no dedicated compaction item model, encrypted_content is carried via the embedded *ResponsesReasoning struct, which re-injects "summary": null during marshaling due to the absence of omitempty. This PR strips the summary field from compaction items post-serialization while leaving it intact on reasoning items, where it is required by OpenAI.

Changes

  • Added ResponsesMessageTypeCompaction constant to the ResponsesMessageType enum.
  • Introduced stripCompactionItemSummary, which uses sjson.DeleteBytes to remove the summary key from any serialized item whose type is compaction.
  • Wired stripCompactionItemSummary into both marshaling paths inside OpenAIResponsesRequestInput.MarshalJSON (the fast path and the CacheControl copy path).
  • Added github.com/tidwall/sjson as a dependency for targeted JSON key deletion without full re-deserialization.
  • Added TestOpenAIResponsesRequest_MarshalJSON_CompactionSummaryStripped to verify that compaction items have summary removed and encrypted_content retained, while sibling reasoning items keep their summary array.

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/openai/... -run TestOpenAIResponsesRequest_MarshalJSON_CompactionSummaryStripped -v
go test ./...

The new test asserts:

  • Index 0 (compaction item): no summary key present, encrypted_content key present.
  • Index 1 (reasoning item): summary key present with value [].

Breaking changes

  • No

Security considerations

No auth, secrets, or PII implications. The change only affects JSON serialization of compaction items before they are sent to the OpenAI API.

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 Jun 1, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b525e74d-7f00-4a33-ba6b-51fcc5b5cde9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 06-02-handles_compaction_message_type

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

@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.
You have signed the CLA already but the status is still pending? Let us recheck it.

@akshaydeo
akshaydeo marked this pull request as ready for review June 1, 2026 19:39
@greptile-apps

greptile-apps Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

Safe to merge; the change is narrowly scoped to post-serialization key deletion for compaction items and does not alter any shared path for other message types.

The core fix — stripping summary from compaction items before sending to OpenAI — is correct and well-contained. The previously flagged silent error-swallow is the only behavior concern, and the remaining notes are test coverage and a redundant import. No logic regressions were found in the changed paths.

core/providers/openai/types.go — the stripCompactionItemSummary function silently drops deletion errors (already flagged in a prior review thread) and imports sjson directly instead of using the existing providerUtils.DeleteJSONField wrapper.

Important Files Changed

Filename Overview
core/schemas/responses.go Adds ResponsesMessageTypeCompaction = "compaction" constant to the ResponsesMessageType enum — a minimal, safe addition with no structural changes.
core/providers/openai/types.go Introduces stripCompactionItemSummary and wires it into both marshal paths. The function imports sjson directly despite providerUtils.DeleteJSONField wrapping the same call; error swallowing was flagged in a prior thread. The CacheControl copy path is also now touched by this logic but not covered by the new test.
core/providers/openai/responses_marshal_test.go New test covers the fast (no-CacheControl) marshal path only; the needsCopy = true code path where a compaction item co-exists with a CacheControl-carrying sibling is not exercised.

Reviews (6): Last reviewed commit: "handles compaction message type" | Re-trigger Greptile

Comment thread core/providers/openai/types.go
@akshaydeo
akshaydeo force-pushed the 06-02-handles_compaction_message_type branch from 1004e9a to fa6858c Compare June 2, 2026 06:59
@akshaydeo
akshaydeo force-pushed the 06-01-openai_integration_content_string_handling branch from 3497c10 to d365c69 Compare June 2, 2026 06:59
@akshaydeo
akshaydeo force-pushed the 06-02-handles_compaction_message_type branch from fa6858c to fd73fb4 Compare June 2, 2026 07:37
@akshaydeo
akshaydeo force-pushed the 06-01-openai_integration_content_string_handling branch from d365c69 to f282051 Compare June 2, 2026 07:37
@akshaydeo
akshaydeo force-pushed the 06-02-handles_compaction_message_type branch from fd73fb4 to c34c954 Compare June 2, 2026 07:47
@akshaydeo
akshaydeo force-pushed the 06-01-openai_integration_content_string_handling branch from f282051 to 1b092da Compare June 2, 2026 07:47

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Full review triggered.

@akshaydeo akshaydeo mentioned this pull request Jun 2, 2026
18 tasks
@akshaydeo
akshaydeo force-pushed the 06-01-openai_integration_content_string_handling branch from 1b092da to 58e1a89 Compare June 2, 2026 08:36
@akshaydeo
akshaydeo force-pushed the 06-02-handles_compaction_message_type branch from c34c954 to 31e5a99 Compare June 2, 2026 08:36

akshaydeo commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Merge activity

  • Jun 2, 8:41 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jun 2, 8:46 AM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo
akshaydeo changed the base branch from 06-01-openai_integration_content_string_handling to graphite-base/3966 June 2, 2026 08:45
@akshaydeo
akshaydeo changed the base branch from graphite-base/3966 to dev June 2, 2026 08:45
@akshaydeo
akshaydeo merged commit 15a7cd3 into dev Jun 2, 2026
9 of 10 checks passed
@akshaydeo
akshaydeo deleted the 06-02-handles_compaction_message_type branch June 2, 2026 08:46
akshaydeo added a commit that referenced this pull request Jun 2, 2026
## Summary

OpenAI's Responses API rejects requests containing a `summary` field on compaction input items with an "Unknown parameter" error. Because Bifrost has no dedicated compaction item model, `encrypted_content` is carried via the embedded `*ResponsesReasoning` struct, which re-injects `"summary": null` during marshaling due to the absence of `omitempty`. This PR strips the `summary` field from compaction items post-serialization while leaving it intact on reasoning items, where it is required by OpenAI.

## Changes

- Added `ResponsesMessageTypeCompaction` constant to the `ResponsesMessageType` enum.
- Introduced `stripCompactionItemSummary`, which uses `sjson.DeleteBytes` to remove the `summary` key from any serialized item whose type is `compaction`.
- Wired `stripCompactionItemSummary` into both marshaling paths inside `OpenAIResponsesRequestInput.MarshalJSON` (the fast path and the `CacheControl` copy path).
- Added `github.com/tidwall/sjson` as a dependency for targeted JSON key deletion without full re-deserialization.
- Added `TestOpenAIResponsesRequest_MarshalJSON_CompactionSummaryStripped` to verify that compaction items have `summary` removed and `encrypted_content` retained, while sibling reasoning items keep their `summary` array.

## 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/openai/... -run TestOpenAIResponsesRequest_MarshalJSON_CompactionSummaryStripped -v
go test ./...
```

The new test asserts:
- Index 0 (compaction item): no `summary` key present, `encrypted_content` key present.
- Index 1 (reasoning item): `summary` key present with value `[]`.

## Breaking changes

- [x] No

## Security considerations

No auth, secrets, or PII implications. The change only affects JSON serialization of compaction items before they are sent to the OpenAI API.

## 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
akshaydeo added a commit that referenced this pull request Jun 4, 2026
## Summary

OpenAI's Responses API rejects requests containing a `summary` field on compaction input items with an "Unknown parameter" error. Because Bifrost has no dedicated compaction item model, `encrypted_content` is carried via the embedded `*ResponsesReasoning` struct, which re-injects `"summary": null` during marshaling due to the absence of `omitempty`. This PR strips the `summary` field from compaction items post-serialization while leaving it intact on reasoning items, where it is required by OpenAI.

## Changes

- Added `ResponsesMessageTypeCompaction` constant to the `ResponsesMessageType` enum.
- Introduced `stripCompactionItemSummary`, which uses `sjson.DeleteBytes` to remove the `summary` key from any serialized item whose type is `compaction`.
- Wired `stripCompactionItemSummary` into both marshaling paths inside `OpenAIResponsesRequestInput.MarshalJSON` (the fast path and the `CacheControl` copy path).
- Added `github.com/tidwall/sjson` as a dependency for targeted JSON key deletion without full re-deserialization.
- Added `TestOpenAIResponsesRequest_MarshalJSON_CompactionSummaryStripped` to verify that compaction items have `summary` removed and `encrypted_content` retained, while sibling reasoning items keep their `summary` array.

## 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/openai/... -run TestOpenAIResponsesRequest_MarshalJSON_CompactionSummaryStripped -v
go test ./...
```

The new test asserts:
- Index 0 (compaction item): no `summary` key present, `encrypted_content` key present.
- Index 1 (reasoning item): `summary` key present with value `[]`.

## Breaking changes

- [x] No

## Security considerations

No auth, secrets, or PII implications. The change only affects JSON serialization of compaction items before they are sent to the OpenAI API.

## 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
akshaydeo added a commit that referenced this pull request Jun 7, 2026
## Summary

OpenAI's Responses API rejects requests containing a `summary` field on compaction input items with an "Unknown parameter" error. Because Bifrost has no dedicated compaction item model, `encrypted_content` is carried via the embedded `*ResponsesReasoning` struct, which re-injects `"summary": null` during marshaling due to the absence of `omitempty`. This PR strips the `summary` field from compaction items post-serialization while leaving it intact on reasoning items, where it is required by OpenAI.

## Changes

- Added `ResponsesMessageTypeCompaction` constant to the `ResponsesMessageType` enum.
- Introduced `stripCompactionItemSummary`, which uses `sjson.DeleteBytes` to remove the `summary` key from any serialized item whose type is `compaction`.
- Wired `stripCompactionItemSummary` into both marshaling paths inside `OpenAIResponsesRequestInput.MarshalJSON` (the fast path and the `CacheControl` copy path).
- Added `github.com/tidwall/sjson` as a dependency for targeted JSON key deletion without full re-deserialization.
- Added `TestOpenAIResponsesRequest_MarshalJSON_CompactionSummaryStripped` to verify that compaction items have `summary` removed and `encrypted_content` retained, while sibling reasoning items keep their `summary` array.

## 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/openai/... -run TestOpenAIResponsesRequest_MarshalJSON_CompactionSummaryStripped -v
go test ./...
```

The new test asserts:
- Index 0 (compaction item): no `summary` key present, `encrypted_content` key present.
- Index 1 (reasoning item): `summary` key present with value `[]`.

## Breaking changes

- [x] No

## Security considerations

No auth, secrets, or PII implications. The change only affects JSON serialization of compaction items before they are sent to the OpenAI API.

## 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.

2 participants