Skip to content

test: update union type test to verify parametersJsonSchema passthrough instead of Gemini Schema conversion - #3520

Merged
akshaydeo merged 1 commit into
devfrom
05-15-fix_test_cases_for_gemini_union_type
May 15, 2026
Merged

test: update union type test to verify parametersJsonSchema passthrough instead of Gemini Schema conversion#3520
akshaydeo merged 1 commit into
devfrom
05-15-fix_test_cases_for_gemini_union_type

Conversation

@BearTS

@BearTS BearTS commented May 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Updates the Gemini tool conversion tests to reflect the shift from converting JSON Schema union types into Gemini-native Schema objects to passing them through unchanged via parametersJsonSchema. This aligns the tests with the current behavior where tool parameters with union types (e.g., ["integer", "null"]) are preserved as raw JSON Schema rather than being transformed into Gemini's Type/Nullable representation.

Changes

  • Updated TestConvertBifrostToolsToGemini_UnionTypeProperty to assert that fd.ParametersJSONSchema is populated and fd.Parameters is nil, reflecting the passthrough approach.
  • Replaced assertions checking for Gemini-native Type and Nullable fields with assertions that verify the original JSON Schema union array (["integer", "null"]) is preserved intact.
  • Updated TestConvertBifrostToolsToGemini_WirePayload to handle the new (tools, error) return signature of convertBifrostToolsToGemini.
  • Updated the test comment to accurately describe the current behavior rather than referencing the previous Goose+Vertex bug fix.

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

Expected: all tests pass, including TestConvertBifrostToolsToGemini_UnionTypeProperty and TestConvertBifrostToolsToGemini_WirePayload.

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

@coderabbitai

coderabbitai Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR updates Gemini tool conversion tests to validate union-typed parameters through wire JSON Schema format. The primary test now asserts that ParametersJSONSchema preserves union type arrays and descriptions instead of checking structured schema fields, and error handling is added to the wire payload test.

Changes

Gemini Union Type Parameter Validation

Layer / File(s) Summary
Union type parameter and error handling test updates
core/providers/gemini/uniontype_test.go
Updated TestConvertBifrostToolsToGemini_UnionTypeProperty to validate union-typed parameters by marshaling and asserting ParametersJSONSchema preserves the JSON Schema union type array ["integer","null"] and description fields; changed TestConvertBifrostToolsToGemini_WirePayload to capture and assert error return from convertBifrostToolsToGemini; clarified the test comment describing union type passthrough behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • maximhq/bifrost#3444: The main PR updates Gemini tool-conversion tests in uniontype_test.go to assert ParametersJSONSchema wire JSON (including union "type": ["integer","null"]) and to handle the new convertBifrostToolsToGemini error return—matching the functional changes introduced in #3444.

Suggested reviewers

  • akshaydeo

Poem

🐰 A union of types in the schema now flows,
Where JSON preserves the form as it goes,
No structured fields needed, just wire formats bright,
With proper error checks, everything's right! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The linked issue #123 (Files API Support) is unrelated to the code changes in this PR, which are test refactoring for Gemini union type handling. Remove the unrelated issue link or ensure the PR addresses Files API Support. This PR appears to be focused on test refactoring for Gemini tool conversion and does not implement Files API functionality.
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: updating tests to verify parametersJsonSchema passthrough instead of Gemini Schema conversion.
Out of Scope Changes check ✅ Passed All changes are limited to test file modifications in core/providers/gemini/uniontype_test.go, aligning with the PR's stated objective of updating test assertions.
Description check ✅ Passed The PR description is well-structured and comprehensive, covering all major template sections including Summary, Changes, Type of change, Affected areas, How to test, Breaking changes, and Checklist.

✏️ 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 05-15-fix_test_cases_for_gemini_union_type

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

BearTS commented May 15, 2026

Copy link
Copy Markdown
Contributor Author

@BearTS
BearTS marked this pull request as ready for review May 15, 2026 07:57
@BearTS BearTS changed the title fix: test cases for gemini union type test: update union type test to verify parametersJsonSchema passthrough instead of Gemini Schema conversion May 15, 2026
@coderabbitai
coderabbitai Bot requested a review from akshaydeo May 15, 2026 07:58
@greptile-apps

greptile-apps Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 3/5

The production implementation is not changed, but three sub-tests in WirePayload assert wire-format expectations that directly contradict the passthrough behavior now verified by UnionTypeProperty — merging will result in test failures.

The TestConvertBifrostToolsToGemini_WirePayload cases for union types were not updated to reflect the move from Gemini-schema conversion to raw JSON passthrough. Their wantContains strings will not appear in the wire when ParametersJSONSchema holds the raw array, and the wantAbsent strings will actually be present, causing assertion failures.

core/providers/gemini/uniontype_test.go — specifically the wantContains/wantAbsent slices for the three union-type cases in TestConvertBifrostToolsToGemini_WirePayload.

Important Files Changed

Filename Overview
core/providers/gemini/uniontype_test.go Updates TestConvertBifrostToolsToGemini_UnionTypeProperty to match the new ParametersJSONSchema passthrough behavior and adds error-return handling throughout, but leaves TestConvertBifrostToolsToGemini_WirePayload cases 1, 3, and 4 with stale expectations from the old Gemini-schema-conversion path, which will cause those sub-tests to fail.

Comments Outside Diff (1)

  1. core/providers/gemini/uniontype_test.go, line 293-321 (link)

    P1 WirePayload test cases now contradict the passthrough implementation

    TestConvertBifrostToolsToGemini_UnionTypeProperty was correctly updated to expect ParametersJSONSchema passthrough (raw ["integer","null"] preserved), but the three union-related cases in TestConvertBifrostToolsToGemini_WirePayload were left with expectations from the old Gemini-Schema-conversion path. Since convertBifrostToolsToGemini now emits fd.ParametersJSONSchema = json.RawMessage(raw) and leaves fd.Parameters nil, the serialized wire JSON will contain the raw "type":["integer","null"] array — not "type":"integer" + "nullable":true (case 1) or "anyOf":[…] (cases 3 & 4). All three test cases will fail at the assert.Contains / assert.NotContains checks.

Reviews (1): Last reviewed commit: "fix: test cases for gemini union type" | Re-trigger Greptile

Comment thread core/providers/gemini/uniontype_test.go

akshaydeo commented May 15, 2026

Copy link
Copy Markdown
Contributor

Merge activity

  • May 15, 1:09 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • May 15, 1:10 PM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo
akshaydeo merged commit 1d80b08 into dev May 15, 2026
12 of 14 checks passed
@akshaydeo
akshaydeo deleted the 05-15-fix_test_cases_for_gemini_union_type branch May 15, 2026 13:10
akshaydeo pushed a commit that referenced this pull request May 15, 2026
…ough instead of Gemini Schema conversion (#3520)

## Summary

Updates the Gemini tool conversion tests to reflect the shift from converting JSON Schema union types into Gemini-native `Schema` objects to passing them through unchanged via `parametersJsonSchema`. This aligns the tests with the current behavior where tool parameters with union types (e.g., `["integer", "null"]`) are preserved as raw JSON Schema rather than being transformed into Gemini's `Type`/`Nullable` representation.

## Changes

- Updated `TestConvertBifrostToolsToGemini_UnionTypeProperty` to assert that `fd.ParametersJSONSchema` is populated and `fd.Parameters` is nil, reflecting the passthrough approach.
- Replaced assertions checking for Gemini-native `Type` and `Nullable` fields with assertions that verify the original JSON Schema union array (`["integer", "null"]`) is preserved intact.
- Updated `TestConvertBifrostToolsToGemini_WirePayload` to handle the new `(tools, error)` return signature of `convertBifrostToolsToGemini`.
- Updated the test comment to accurately describe the current behavior rather than referencing the previous Goose+Vertex bug fix.

## Type of change

- [ ] Bug fix
- [ ] Feature
- [x] 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/gemini/...
```

Expected: all tests pass, including `TestConvertBifrostToolsToGemini_UnionTypeProperty` and `TestConvertBifrostToolsToGemini_WirePayload`.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None.

## 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 pushed a commit that referenced this pull request May 20, 2026
…ough instead of Gemini Schema conversion (#3520)

## Summary

Updates the Gemini tool conversion tests to reflect the shift from converting JSON Schema union types into Gemini-native `Schema` objects to passing them through unchanged via `parametersJsonSchema`. This aligns the tests with the current behavior where tool parameters with union types (e.g., `["integer", "null"]`) are preserved as raw JSON Schema rather than being transformed into Gemini's `Type`/`Nullable` representation.

## Changes

- Updated `TestConvertBifrostToolsToGemini_UnionTypeProperty` to assert that `fd.ParametersJSONSchema` is populated and `fd.Parameters` is nil, reflecting the passthrough approach.
- Replaced assertions checking for Gemini-native `Type` and `Nullable` fields with assertions that verify the original JSON Schema union array (`["integer", "null"]`) is preserved intact.
- Updated `TestConvertBifrostToolsToGemini_WirePayload` to handle the new `(tools, error)` return signature of `convertBifrostToolsToGemini`.
- Updated the test comment to accurately describe the current behavior rather than referencing the previous Goose+Vertex bug fix.

## Type of change

- [ ] Bug fix
- [ ] Feature
- [x] 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/gemini/...
```

Expected: all tests pass, including `TestConvertBifrostToolsToGemini_UnionTypeProperty` and `TestConvertBifrostToolsToGemini_WirePayload`.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None.

## 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
akhsaul pushed a commit to akhsaul/bifrost that referenced this pull request Aug 27, 2026
…ough instead of Gemini Schema conversion (maximhq#3520)

## Summary

Updates the Gemini tool conversion tests to reflect the shift from converting JSON Schema union types into Gemini-native `Schema` objects to passing them through unchanged via `parametersJsonSchema`. This aligns the tests with the current behavior where tool parameters with union types (e.g., `["integer", "null"]`) are preserved as raw JSON Schema rather than being transformed into Gemini's `Type`/`Nullable` representation.

## Changes

- Updated `TestConvertBifrostToolsToGemini_UnionTypeProperty` to assert that `fd.ParametersJSONSchema` is populated and `fd.Parameters` is nil, reflecting the passthrough approach.
- Replaced assertions checking for Gemini-native `Type` and `Nullable` fields with assertions that verify the original JSON Schema union array (`["integer", "null"]`) is preserved intact.
- Updated `TestConvertBifrostToolsToGemini_WirePayload` to handle the new `(tools, error)` return signature of `convertBifrostToolsToGemini`.
- Updated the test comment to accurately describe the current behavior rather than referencing the previous Goose+Vertex bug fix.

## Type of change

- [ ] Bug fix
- [ ] Feature
- [x] 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/gemini/...
```

Expected: all tests pass, including `TestConvertBifrostToolsToGemini_UnionTypeProperty` and `TestConvertBifrostToolsToGemini_WirePayload`.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None.

## 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
occcat pushed a commit to occcat/bifrost that referenced this pull request Sep 2, 2026
…ough instead of Gemini Schema conversion (maximhq#3520)

## Summary

Updates the Gemini tool conversion tests to reflect the shift from converting JSON Schema union types into Gemini-native `Schema` objects to passing them through unchanged via `parametersJsonSchema`. This aligns the tests with the current behavior where tool parameters with union types (e.g., `["integer", "null"]`) are preserved as raw JSON Schema rather than being transformed into Gemini's `Type`/`Nullable` representation.

## Changes

- Updated `TestConvertBifrostToolsToGemini_UnionTypeProperty` to assert that `fd.ParametersJSONSchema` is populated and `fd.Parameters` is nil, reflecting the passthrough approach.
- Replaced assertions checking for Gemini-native `Type` and `Nullable` fields with assertions that verify the original JSON Schema union array (`["integer", "null"]`) is preserved intact.
- Updated `TestConvertBifrostToolsToGemini_WirePayload` to handle the new `(tools, error)` return signature of `convertBifrostToolsToGemini`.
- Updated the test comment to accurately describe the current behavior rather than referencing the previous Goose+Vertex bug fix.

## Type of change

- [ ] Bug fix
- [ ] Feature
- [x] 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/gemini/...
```

Expected: all tests pass, including `TestConvertBifrostToolsToGemini_UnionTypeProperty` and `TestConvertBifrostToolsToGemini_WirePayload`.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None.

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