Skip to content

fix(csharp): add precision fallbacks to JsonElementComparer for float/double#14875

Merged
Swimburger merged 3 commits intomainfrom
devin/1775791160-fix-float-precision-mock-tests
Apr 10, 2026
Merged

fix(csharp): add precision fallbacks to JsonElementComparer for float/double#14875
Swimburger merged 3 commits intomainfrom
devin/1775791160-fix-float-precision-mock-tests

Conversation

@Swimburger
Copy link
Copy Markdown
Member

@Swimburger Swimburger commented Apr 10, 2026

Description

Fixes mock server test failures (e.g. GetJobPredictionsTest.MockServerTest in the Hume C# SDK) caused by a serialization format mismatch when fields use format: float.

Root cause: When a JSON response contains a float value like 0.10722749680280685, C# deserializes it into System.Single, then re-serializes it using System.Text.Json which outputs the shortest roundtrip representation: 0.1072275. The test comparator (JsonElementComparer) compares via GetDecimal(), which is string-based, so 0.10722749680280685 != 0.1072275 and the test fails — even though both represent the same float32 bits.

Changes Made

  • In JsonElementComparer.Template.cs, replaced the direct GetDecimal() fail with a 3-tier fallback chain: GetDecimal()GetDouble()GetSingle(). Only reports a mismatch if all three levels disagree. The intermediate GetDouble() check prevents GetSingle() from masking genuine differences in double-precision fields (e.g. integers 16777216 vs 16777217 are equal as Single but differ as Double)
  • Added version entry 2.59.4 in versions.yml
  • Updated all 287 seed snapshot JsonElementComparer.cs files

⚠️ Human Review Checklist

  • The fallback chain is applied unconditionally to all JSON numbers, not scoped to fields known to be format: float. The GetDouble() intermediate check should prevent false positives for double-precision and integer fields, but consider edge cases: if two decimal values differ only beyond double precision (>15 significant digits), GetDouble() would consider them equal and skip the GetSingle() check entirely, which is the correct behavior. However, no test currently exercises any of these fallback paths.
  • No seed test exercises this code path — no existing seed test fixtures use float primitives in mock server responses, so this change produces no behavioral snapshot diffs beyond the template update. The fix was validated externally against the Hume C# SDK (2 failures → 1, with the remaining failure being an unrelated empty path parameter issue).

Testing

  • Lint passes (pnpm run check)
  • Validated against Hume C# SDK — GetJobPredictionsTest.MockServerTest passes with this change

Link to Devin session: https://app.devin.ai/sessions/1dcf1274fbe74a7cbac9f76e9a65ed28
Requested by: @Swimburger


Open with Devin

…tests

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@github-actions
Copy link
Copy Markdown
Contributor

🌱 Seed Test Selector

Select languages to run seed tests for:

  • Python
  • TypeScript
  • Java
  • Go
  • Ruby
  • C#
  • PHP
  • Swift
  • Rust
  • OpenAPI

How to use: Click the ⋯ menu above → "Edit" → check the boxes you want → click "Update comment". Tests will run automatically and snapshots will be committed to this PR.

@github-actions
Copy link
Copy Markdown
Contributor

SDK Generation Benchmark Results

Comparing PR branch against latest nightly baseline on main (2026-04-09T04:46:50Z).

Full benchmark table (click to expand)
Generator Spec main (generator) main (E2E) PR (generator) Delta
csharp-sdk square 95s 138s 87s -8s (-8.4%)

main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via fern generate). main (E2E): full customer-observable time including build/test scripts (nightly baseline, informational). Delta is computed against generator-only baseline.
⚠️ = generation exited with a non-zero exit code (timing may not reflect a successful run).
Baseline from nightly runs on main (latest: 2026-04-09T04:46:50Z). Trigger benchmark-baseline to refresh.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

…t precision

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration devin-ai-integration Bot changed the title fix(csharp): round float examples to 32-bit precision in mock server tests fix(csharp): add GetSingle() fallback to JsonElementComparer for float precision Apr 10, 2026
devin-ai-integration[bot]

This comment was marked as resolved.

…llback

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration devin-ai-integration Bot changed the title fix(csharp): add GetSingle() fallback to JsonElementComparer for float precision fix(csharp): add precision fallbacks to JsonElementComparer for float/double Apr 10, 2026
@github-actions
Copy link
Copy Markdown
Contributor

SDK Generation Benchmark Results

Comparing PR branch against latest nightly baseline on main (2026-04-09T04:46:50Z).

Full benchmark table (click to expand)
Generator Spec main (generator) main (E2E) PR (generator) Delta
csharp-sdk square 95s 138s 92s -3s (-3.2%)

main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via fern generate). main (E2E): full customer-observable time including build/test scripts (nightly baseline, informational). Delta is computed against generator-only baseline.
⚠️ = generation exited with a non-zero exit code (timing may not reflect a successful run).
Baseline from nightly runs on main (latest: 2026-04-09T04:46:50Z). Trigger benchmark-baseline to refresh.

@Swimburger Swimburger enabled auto-merge (squash) April 10, 2026 04:21
@Swimburger Swimburger merged commit ae00795 into main Apr 10, 2026
87 checks passed
@Swimburger Swimburger deleted the devin/1775791160-fix-float-precision-mock-tests branch April 10, 2026 04:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants