Skip to content

fix(config): address jsonschema/v6 Go Fan review — Draft hint, errors.New, UnmarshalJSON precision - #9403

Merged
lpcox merged 4 commits into
mainfrom
copilot/go-fan-review-jsonschema-v6
Jul 15, 2026
Merged

fix(config): address jsonschema/v6 Go Fan review — Draft hint, errors.New, UnmarshalJSON precision#9403
lpcox merged 4 commits into
mainfrom
copilot/go-fan-review-jsonschema-v6

Conversation

Copilot AI commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Three quick-win improvements surfaced by the Go Fan module review of santhosh-tekuri/jsonschema/v6.

Changes

  • Fix misleading Draft 7 error hint (validation_server.go): The compile-failure hint incorrectly said "Draft 7" when the compiler defaults to Draft 2020-12. Updated to "valid JSON Schema document (Draft 2020-12 or earlier)".

  • Replace fmt.Errorf("%s", ...) with errors.New (validation_schema.go): No format verbs, no wrapping — errors.New(sb.String()) is the correct idiom.

  • Use jsonschema.UnmarshalJSON in validateServerAgainstSchema (validation_server.go): Aligns the custom-schema validation path with the embedded-schema path, avoiding potential number-precision loss from encoding/json.Unmarshal on large integers or high-precision floats. serverMap is initialized before the type assertion to guarantee it is never nil.

// Before
if err := json.Unmarshal(serverJSON, &serverMap); err != nil { ... }

// After
serverObj, parseErr := jsonschema.UnmarshalJSON(bytes.NewReader(serverJSON))
if parseErr != nil { ... }
if obj, ok := serverObj.(map[string]interface{}); ok {
    serverMap = obj
} else {
    logValidation.Printf("unexpected: server config parsed to non-object type, using empty map for validation: name=%s", name)
}

The test for the unknown ErrorKind default path in TestFormatErrorContext already existed (kind.Not{} case), so no new test was needed.

Copilot AI linked an issue Jul 15, 2026 that may be closed by this pull request
5 tasks
Copilot AI changed the title [WIP] Review Go module santhosh-tekuri/jsonschema/v6 fix(config): address jsonschema/v6 Go Fan review — Draft hint, errors.New, UnmarshalJSON precision Jul 15, 2026
Copilot finished work on behalf of lpcox July 15, 2026 20:38
Copilot AI requested a review from lpcox July 15, 2026 20:38
@lpcox
lpcox marked this pull request as ready for review July 15, 2026 21:20
Copilot AI review requested due to automatic review settings July 15, 2026 21:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates JSON Schema validation messaging and error construction while attempting precision-safe custom-schema parsing.

Changes:

  • Corrects the supported JSON Schema draft hint.
  • Uses errors.New for static error construction.
  • Switches server validation parsing to jsonschema.UnmarshalJSON.
Show a summary per file
File Description
internal/config/validation_server.go Updates schema guidance and parsing behavior.
internal/config/validation_schema.go Simplifies formatted error construction.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment on lines +333 to +334
// Parse using jsonschema.UnmarshalJSON for number-precision consistency with embedded schema path
serverObj, parseErr := jsonschema.UnmarshalJSON(bytes.NewReader(serverJSON))
@lpcox

lpcox commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

@copilot address review feedback

Copilot AI commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

@copilot address review feedback

Fixed in the latest commit. The root cause was in StdinServerConfig.UnmarshalJSON (config_stdin.go): it was using encoding/json.Unmarshal to build the allFields map, which decodes all numbers as float64 — losing precision for integers beyond 2^53. The merged AdditionalProperties then clobbered the precise values from jsonschema.UnmarshalJSON in the validation path.

Change: replaced the json.Unmarshal(data, &allFields) call with jsonschema.UnmarshalJSON(bytes.NewReader(data)), which uses decoder.UseNumber() internally. Numbers in AdditionalProperties are now stored as json.Number (exact string representation) instead of float64.

Regression test added in validate_server_against_schema_test.go (TestValidateServerAgainstSchema_LargeIntegerPrecision): it round-trips 9007199254740993 through UnmarshalJSON and validates it against a "const": 9007199254740993 schema — this test would have failed before the fix. The pre-existing coverage test in config_stdin_unmarshal_coverage_test.go was updated to assert json.Number("42") instead of float64(42).

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.

[go-fan] Go Module Review: santhosh-tekuri/jsonschema/v6

3 participants