Skip to content

mod fixes - #5127

Merged
akshaydeo merged 1 commit into
v2.0.0from
07-12-mod_fixes
Jul 13, 2026
Merged

mod fixes#5127
akshaydeo merged 1 commit into
v2.0.0from
07-12-mod_fixes

Conversation

@akshaydeo

@akshaydeo akshaydeo commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Removes an unintended raw byte capture for tool_search_call messages during JSON unmarshalling, preventing the raw payload from being stored redundantly in rawToolSearch while still preserving the arguments field for downstream consumers.

Changes

  • Removed the m.rawToolSearch = append([]byte(nil), data...) assignment inside the isRawPreservedItem branch of UnmarshalJSON. The comment clarifying that MarshalJSON re-emits preserved bytes verbatim is retained, as the round-trip behavior is unchanged.
  • Bumped github.com/maximhq/bifrost/core from v1.6.3 to v1.7.0 across the test seed modules.

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/schemas/...

Verify that tool_search_call messages round-trip correctly through UnmarshalJSONMarshalJSON and that the Arguments field remains accessible to downstream consumers after unmarshalling.

Screenshots/Recordings

N/A

Breaking changes

  • Yes
  • No

Related issues

N/A

Security considerations

No security implications.

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 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Codex tool-item unmarshalling now normalizes and exposes arguments while retaining original JSON bytes for re-emission. Three seed modules update their core dependency from v1.6.3 to v1.7.0.

Changes

Codex tool argument preservation

Layer / File(s) Summary
Normalize arguments during raw preservation
core/schemas/responses.go
Codex tool items populate the embedded tool-message Arguments string before preserving their complete original JSON.
Align seed module dependencies
tests/cmd/e2eseed/go.mod, tests/cmd/seed/go.mod, tests/cmd/seedvks/go.mod
Seed modules update their github.com/maximhq/bifrost/core requirement to v1.7.0.

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

Possibly related PRs

  • maximhq/bifrost#4121 — Related Codex tool-search raw round-trip handling in ResponsesMessage.UnmarshalJSON.
  • maximhq/bifrost#4644 — Related normalization of tool-call arguments in response message unmarshalling.
  • maximhq/bifrost#5103 — Related preservation and extraction of Codex tool-item arguments.
🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR does not address #123 Files API Support; it changes JSON unmarshalling behavior and test dependency versions instead. Implement the file upload support requested in #123 or update the linked issue if this PR is meant for a different task.
Out of Scope Changes check ⚠️ Warning The unmarshalling fix and test dependency bumps are unrelated to the linked Files API Support objective. Remove unrelated changes or retarget the PR so it only covers the Files API work requested by #123.
Title check ❓ Inconclusive The title is too generic and does not clearly describe the change. Rename it to a concise, specific summary of the main change, such as the JSON unmarshalling fix or dependency bump.
✅ Passed checks (2 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The description matches the template well and includes summary, changes, testing, and impact sections.
✨ 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-12-mod_fixes

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.
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 July 13, 2026 01:59

Copy link
Copy Markdown
Contributor Author

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

akshaydeo commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Merge activity

  • Jul 13, 2:00 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jul 13, 2:01 AM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo
akshaydeo merged commit 6bef368 into v2.0.0 Jul 13, 2026
14 of 16 checks passed
@akshaydeo
akshaydeo deleted the 07-12-mod_fixes branch July 13, 2026 02:01
@greptile-apps

greptile-apps Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
core/schemas/responses.go Removes an obsolete raw field assignment while the active preserved-byte capture remains in place.
tests/cmd/e2eseed/go.mod Updates the indirect core module requirement to v1.7.0.
tests/cmd/seed/go.mod Updates the direct core module requirement to v1.7.0.
tests/cmd/seedvks/go.mod Updates the direct core module requirement to v1.7.0.

Reviews (1): Last reviewed commit: "mod fixes" | Re-trigger Greptile

akshaydeo added a commit that referenced this pull request Jul 14, 2026
## Summary

Removes an unintended raw byte capture for `tool_search_call` messages during JSON unmarshalling, preventing the raw payload from being stored redundantly in `rawToolSearch` while still preserving the `arguments` field for downstream consumers.

## Changes

- Removed the `m.rawToolSearch = append([]byte(nil), data...)` assignment inside the `isRawPreservedItem` branch of `UnmarshalJSON`. The comment clarifying that `MarshalJSON` re-emits preserved bytes verbatim is retained, as the round-trip behavior is unchanged.
- Bumped `github.com/maximhq/bifrost/core` from `v1.6.3` to `v1.7.0` across the test seed modules.

## Type of change

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

## Affected areas

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

## How to test

```sh
go test ./core/schemas/...
```

Verify that `tool_search_call` messages round-trip correctly through `UnmarshalJSON` → `MarshalJSON` and that the `Arguments` field remains accessible to downstream consumers after unmarshalling.

## Screenshots/Recordings

N/A

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

N/A

## Security considerations

No security implications.

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