Skip to content

Remove go-json-experiment dependency#680

Merged
apstndb merged 2 commits into
mainfrom
deps/remove-go-json-experiment-json
Jun 17, 2026
Merged

Remove go-json-experiment dependency#680
apstndb merged 2 commits into
mainfrom
deps/remove-go-json-experiment-json

Conversation

@apstndb

@apstndb apstndb commented Jun 17, 2026

Copy link
Copy Markdown
Owner

Summary:

  • Replace go-json-experiment/json usage in query profile parsing with encoding/json.RawMessage and encoding/json.Unmarshal.
  • Rewrite JSONL streaming to preserve column order using standard encoding/json string encoding and raw JSON validation.
  • Remove github.com/go-json-experiment/json from go.mod/go.sum.

Future encoding/json/v2 plan:

  • Keep this PR on stable encoding/json because the local Go toolchain currently exposes encoding/json but not encoding/json/v2.
  • When encoding/json/v2 is available in the supported Go baseline, evaluate it only for the small JSONL string/raw-value helper boundary and QueryProfile RawMessage decoding path.
  • Before migrating, compare JSONL byte output for column order, RawJSONCell values, invalid raw JSON errors, HTML-sensitive characters, and newline escaping against the tests in this PR.
  • Avoid reintroducing an experimental external module in library-facing packages; any v2 adoption should come from the standard library once the project Go baseline supports it.

Validation:

  • go test ./internal/mycli/format -run 'TestFormatJSONL|TestJSONLFormatterLifecycle' -count=1
  • go test ./internal/mycli -run 'TestParseQueryStats|TestParseQueryProfile' -count=1
  • go mod why -m github.com/go-json-experiment/json
  • make check

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request removes the third-party github.com/go-json-experiment/json dependency, refactoring the JSONLFormatter to manually construct JSON lines using the standard library's encoding/json and updating query profile structs to use json.RawMessage. The reviewer feedback advises against this change, recommending that jsontext.Encoder be retained and reused to ensure efficient streaming, avoid repeated allocations, and guarantee column order preservation in the JSONL output.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread internal/mycli/format/streaming_jsonl.go
Comment thread internal/mycli/format/streaming_jsonl.go Outdated
Comment thread internal/mycli/format/streaming_jsonl.go Outdated
Comment thread internal/mycli/format/streaming_jsonl.go Outdated
@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown

Code Metrics Report

📊 View detailed coverage report (available for 7 days)

main (800eb51) #680 (1883d6b) +/-
Coverage 70.2% 70.2% +0.0%
Code to Test Ratio 1:1.2 1:1.2 -0.1
Test Execution Time 1m36s 50s -46s
Details
  |                     | main (800eb51) | #680 (1883d6b) |  +/-  |
  |---------------------|----------------|----------------|-------|
+ | Coverage            |          70.2% |          70.2% | +0.0% |
  |   Files             |             84 |             84 |     0 |
  |   Lines             |           7497 |           7517 |   +20 |
+ |   Covered           |           5263 |           5281 |   +18 |
- | Code to Test Ratio  |          1:1.2 |          1:1.2 |  -0.1 |
  |   Code              |          17253 |          17283 |   +30 |
+ |   Test              |          22302 |          22323 |   +21 |
+ | Test Execution Time |          1m36s |            50s |  -46s |

Code coverage of files in pull request scope (24.7% → 35.1%)

Files Coverage +/- Status
internal/mycli/format/streaming_jsonl.go 84.7% +4.0% modified
internal/mycli/statements_query_profile.go 6.3% 0.0% modified

Reported by octocov

@apstndb

apstndb commented Jun 17, 2026

Copy link
Copy Markdown
Owner Author

/gemini review

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request removes the external dependency github.com/go-json-experiment/json and refactors the codebase to use the standard library's encoding/json package. Specifically, JSONLFormatter is updated to use json.Encoder with HTML escaping disabled (SetEscapeHTML(false)) to ensure HTML-sensitive characters are preserved in the JSONL output. It also manually formats JSON objects to preserve column order and validates raw JSON cells using json.Valid. Additionally, queryProfiles and rawQueryProfiles are updated to use json.RawMessage instead of jsontext.Value. Unit tests have been added to verify HTML-sensitive character handling and invalid raw JSON error reporting. There are no review comments, so I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@apstndb apstndb merged commit 576e32c into main Jun 17, 2026
3 checks passed
@apstndb apstndb deleted the deps/remove-go-json-experiment-json branch June 17, 2026 08:16
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.

1 participant