Skip to content

refactor: standardize SDK JSON serialization on orjson#1185

Draft
dgarros wants to merge 17 commits into
stablefrom
dga/feat-orjson-pd5o6
Draft

refactor: standardize SDK JSON serialization on orjson#1185
dgarros wants to merge 17 commits into
stablefrom
dga/feat-orjson-pd5o6

Conversation

@dgarros

@dgarros dgarros commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Why

The SDK used two JSON libraries interchangeably — ujson (~16 modules) and stdlib json (~7; utils.py and playback.py imported both) — with no rule for which to use. This left a latent exception-handling hazard: ujson.JSONDecodeError and json.JSONDecodeError do not catch each other, so a future mismatched decode/except pair would silently fail to catch malformed input.

Goal: converge on a single JSON library — orjson (Rust-backed, faster, stricter) — for a no-behaviour-change speedup on the SDK's hot path (transforms, generators, checks, imports/exports, GraphQL round-trips).

Non-goals: no benchmark harness / CI perf gate; no change to the query-group naming scheme; no migration of JSON usage outside infrahub_sdk/.

Closes #1165
Closes #1171

What changed

Behavioral:

  • orjson is now the sole JSON library; ujson + types-ujson removed and all stdlib json usage eliminated in infrahub_sdk/.
  • JSON output is preserved byte-for-byte at every consumed/compared site (indent=2 → OPT_INDENT_2; infrahubctl datetime rendering preserved via OPT_PASSTHROUGH_DATETIME + default=str).
  • One documented behaviour change: the tracking-group name derived from query parameters containing non-ASCII characters shifts once on upgrade (orjson emits raw UTF-8 where the previous library emitted escaped sequences). Changelog entry added.

Implementation notes:

  • All decode/except pairs unified on orjson.JSONDecodeError (decoding switched to orjson.loads(response.content) at httpx sites) — this incidentally fixes a latent bug in the pytest plugin where except ujson.JSONDecodeError never caught the stdlib error raised by response.json().
  • orjson.dumps() returns bytes: .decode() added at str-consuming sites; OPT_NON_STR_KEYS preserves prior int-key coercion at arbitrary-data sites; file-object I/O (recorder/playback) rewritten since orjson has no load/dump.

What stayed the same: dict_hash public signature and str return type; ASCII/int/float hashes are byte-identical; API contract unchanged.

Suggested review order

  1. infrahub_sdk/utils.pydict_hash + decode_json.
  2. Decode/except sites (schema/__init__.py, ctl/parsers.py, template/infrahub_filters.py, pytest_plugin/items/*).
  3. Encode/.decode() sites and pyproject.toml.
  4. Tests + dev/specs/002-orjson-json-migration/ (decision trail).

How to review

Focus scrutiny on except-clause correctness at decode sites and the non-ASCII dict_hash change. The rest is mechanical import swaps + .decode().

How to test

uv sync --all-groups --all-extras
uv run pytest tests/unit/
grep -rn "import ujson" infrahub_sdk/   # returns nothing
uv run invoke lint-code

Note: 8 pre-existing unit failures reproduce identically on stable (a worktree-path artifact where Rich strips [...] from console output) — unrelated to this change. Integration tests require a live Infrahub server (run in CI).

Impact & rollout

  • Backward compatibility: No public API change; dict_hash still returns str. One-time non-ASCII tracking-group rename (documented); orphaned-group cleanup is manual.
  • Performance: ad-hoc benchmark — orjson dumps ~11.8x, round-trip ~2.1x faster than stdlib json.
  • Config/env changes: new runtime dependency orjson>=3.10; ujson + types-ujson removed.
  • Deployment notes: safe to deploy.

Checklist

  • Tests added/updated
  • Changelog entry added (changelog/+orjson-json-migration.changed.md)
  • External docs updated (verified no user-facing doc referenced the JSON library)
  • Internal .md docs updated (spec artifacts under dev/specs/)

Summary by cubic

Standardized all SDK JSON serialization on orjson to unify error handling and improve performance. Output remains functionally the same; non‑ASCII JSON is now emitted as raw UTF‑8 and tracking‑group names with non‑ASCII query params shift once.

  • Refactors

    • Replaced ujson and stdlib json with orjson across infrahub_sdk/ (encode/decode, CLI, Jinja filters, pytest plugin).
    • Unified decoding via orjson.loads(response.content) and except orjson.JSONDecodeError; fixes the pytest plugin mismatch on HTTP JSON errors.
    • Preserved formatting/behavior: 2‑space indent, sorted keys where used, datetime via default=str with OPT_PASSTHROUGH_DATETIME; added .decode() where strings are required.
    • Preserved non‑string key coercion by using OPT_NON_STR_KEYS at arbitrary‑data sites (CLI transform/formatter, GraphQL multipart, exporter, playback request payload).
    • Updated recorder/playback and transfer import/export to orjson; GraphQL string escaping now uses orjson.dumps.
  • Migration

    • No API changes. JSON with non‑ASCII characters is now written as raw UTF‑8 (not \uXXXX escapes); bytes differ but content is equivalent.
    • One‑time shift for tracking‑group names when query params contain non‑ASCII; clean up old groups manually if needed.
    • Playback recordings made with older SDKs may not be found due to filename differences; re‑record if needed.
    • Dependencies: add orjson>=3.10; remove ujson and types-ujson.

Written for commit 51ff7a4. Summary will update on new commits.

Review in cubic

dgarros and others added 14 commits July 14, 2026 16:22
Consolidates the JSON library standardization and orjson adoption
tech-debt items into a single migration spec (specify phase).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Plan, research, data-model (call-site migration map), serialization
contract, and quickstart for the orjson JSON standardization (plan phase).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Dual-lens critique (verdict: PROCEED, no must-address). Applied low-risk
recommendations to spec/plan: NaN/Infinity + special-type edge cases,
characterization-test acceptance, CI-matrix wheel note, orphan-cleanup note.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Dependency-ordered task breakdown (34 tasks, single atomic P1 story) for
the orjson JSON standardization (tasks phase).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add orjson>=3.10 to project dependencies as the first step of the JSON
serialization migration. ujson is retained for now so unmigrated modules
keep importing during the atomic migration; its removal is deferred to the
final polish step.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace ujson/stdlib-json usage in utils.py with orjson:
- dict_hash now uses orjson.dumps(option=OPT_SORT_KEYS) (already bytes)
- decode_json decodes via orjson.loads(response.content) and catches
  orjson.JSONDecodeError, keeping decode + except internally consistent
- adapt the existing decode_json tests to drive response.content

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace ujson/stdlib json encode calls with orjson across eight modules:
client debug print, checks log print, graphql multipart and renderers,
ctl validate, cli_commands, telemetry export, and the JSON formatter.
orjson.dumps returns bytes, so .decode() is added wherever a str is
consumed; indent/sort_keys/non-str-key/datetime behaviors are preserved
via orjson options.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Swap json/ujson decode call sites to orjson across parsers, Jinja
filters, schema response parsing, JSON importer, and the pytest plugin
models. Schema response parsing now decodes explicitly via
orjson.loads(response.content) so the paired except uses
orjson.JSONDecodeError precisely. Update the from_json filter test to
assert the library-stable message prefix.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Swap ujson/json serialization for orjson in recorder.py, playback.py,
and transfer/exporter/json.py. Use OPT_NON_STR_KEYS on export sites to
preserve prior silent non-str-key coercion.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Swap ujson for orjson across the pytest plugin item classes. Decode
HTTP error responses explicitly via orjson.loads(response.content) so
the paired except matches the raising library.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add and confirm tests that lock in the orjson migration's behavioral
contract: non-ASCII dict_hash vector, CLI datetime str() rendering,
recorder->playback round-trip, and malformed-JSON decode error.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Remove the deferred ujson runtime dependency and types-ujson stub now
that all of infrahub_sdk/ is migrated to orjson; refresh the lockfile.
Migrate remaining ujson.loads call sites in the test suite to orjson and
update the client query-echo expected output for the OPT_INDENT_2 (4->2
space) indent shift. Add a release note for the one-time non-ASCII
tracking-group-name change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Implementation + review tail complete: 34/34 tasks, 8 chunks, no high+
review findings, all local-pass evidence present.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dgarros dgarros added the type/tech-debt Item we know we need to improve way it is implemented label Jul 16, 2026

@cubic-dev-ai cubic-dev-ai 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.

11 issues found across 45 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="infrahub_sdk/pytest_plugin/items/check.py">

<violation number="1" location="infrahub_sdk/pytest_plugin/items/check.py:52">
P3: HTTP error reports now render non-ASCII payload characters as raw UTF-8 instead of the previous `\uXXXX` escapes, changing user-visible failure text for localized or non-ASCII API errors. Preserving the old output requires an explicit ASCII-escaping step or documenting and testing this additional behavior change.</violation>
</file>

<file name="infrahub_sdk/transfer/exporter/json.py">

<violation number="1" location="infrahub_sdk/transfer/exporter/json.py:155">
P2: Exported `nodes.json` files no longer retain the previous escaped-Unicode byte representation: any non-ASCII node value is written as raw UTF-8 by these new `orjson.dumps` calls. This affects transfer-file diffs/checksums and contradicts the stated byte-preservation contract beyond the documented tracking-group case; preserving the old escaping at this file boundary would avoid the unannounced format change.</violation>
</file>

<file name="infrahub_sdk/template/infrahub_filters.py">

<violation number="1" location="infrahub_sdk/template/infrahub_filters.py:167">
P2: Large JSON integers can silently lose precision through this filter: `from_json('{"id": 18446744073709551616}')` used to return an exact `int`, while `orjson.loads` returns a `float`. Since the PR documents only tracking-group renames as behavior changes, preserving arbitrary-integer parsing here or documenting and testing this additional compatibility change would avoid corrupted template data.</violation>
</file>

<file name="infrahub_sdk/pytest_plugin/items/graphql_query.py">

<violation number="1" location="infrahub_sdk/pytest_plugin/items/graphql_query.py:32">
P3: Non-UTF-8 JSON error responses are no longer parsed and pretty-printed: the raw byte payload is passed directly to `orjson.loads`, so UTF-16/UTF-32 responses fall through to the unformatted text fallback. Preserving HTTPX's charset-aware decoding would retain the prior behavior for these responses.</violation>
</file>

<file name="infrahub_sdk/ctl/formatters/json.py">

<violation number="1" location="infrahub_sdk/ctl/formatters/json.py:43">
P2: Non-ASCII values now have a different CLI JSON representation: the previous `json.dumps` escaped them, whereas these calls emit raw UTF-8. This breaks the stated byte-for-byte output guarantee and can affect snapshot or string-based consumers; preserving the old escaping or explicitly updating the output contract for both formatter methods would avoid the compatibility regression.</violation>
</file>

<file name="infrahub_sdk/pytest_plugin/items/jinja2_transform.py">

<violation number="1" location="infrahub_sdk/pytest_plugin/items/jinja2_transform.py:64">
P2: HTTP error output no longer preserves the previous JSON representation: `ujson.dumps(..., indent=4, sort_keys=True)` emitted four-space indentation and escaped non-ASCII by default, while `OPT_INDENT_2` emits two-space indentation and raw UTF-8. This changes the diagnostic text and conflicts with the stated byte-for-byte preservation; retaining the old formatting/escaping would avoid the regression.</violation>
</file>

<file name="infrahub_sdk/ctl/telemetry.py">

<violation number="1" location="infrahub_sdk/ctl/telemetry.py:127">
P2: Telemetry exports now contain raw non-ASCII characters instead of the `\uXXXX` escapes produced by the previous serializer, so `telemetry-export.json` is no longer byte-compatible for names or labels containing non-ASCII text. Preserving the previous escaping or explicitly documenting this additional export-format change would keep the migration contract accurate.</violation>
</file>

<file name="infrahub_sdk/checks.py">

<violation number="1" location="infrahub_sdk/checks.py:112">
P3: Check stdout containing non-ASCII text now changes from escaped JSON to raw UTF-8, breaking the stated byte-for-byte output guarantee and potentially affecting consumers that compare or record these log lines. A compatibility-preserving encoding path or an explicit documented/tested exception for check stdout would keep this behavior intentional.</violation>
</file>

<file name="infrahub_sdk/ctl/validate.py">

<violation number="1" location="infrahub_sdk/ctl/validate.py:107">
P2: The `--out` JSON file changes byte representation for any non-ASCII response value: `ujson` escaped Unicode while `orjson` emits raw UTF-8. This can break consumers or snapshots that compare the CLI output bytes, so the output path needs an explicit compatibility escaping strategy or the behavior change should be documented and covered.</violation>
</file>

<file name="infrahub_sdk/ctl/cli_commands.py">

<violation number="1" location="infrahub_sdk/ctl/cli_commands.py:358">
P2: Non-ASCII transform output is no longer byte-compatible: `ujson` escaped characters above ASCII by default, while `orjson` emits raw UTF-8. Preserving the previous escaping or explicitly documenting and testing this as an additional behavior change would keep the CLI serialization contract accurate.</violation>
</file>

<file name="infrahub_sdk/schema/__init__.py">

<violation number="1" location="infrahub_sdk/schema/__init__.py:276">
P2: Schema loading now rejects otherwise decodable JSON responses that use a BOM or UTF-16/32 encoding, because `orjson.loads` only accepts UTF-8 bytes while the previous `response.json()` decoder handled those encodings. Decoding the response using HTTPX's selected text encoding before passing it to orjson would preserve the previous compatibility.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic


output_path = Path(output)
output_path.write_text(json.dumps(snapshots, indent=2), encoding="utf-8")
output_path.write_text(orjson.dumps(snapshots, option=orjson.OPT_INDENT_2).decode(), encoding="utf-8")

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.

P2: Telemetry exports now contain raw non-ASCII characters instead of the \uXXXX escapes produced by the previous serializer, so telemetry-export.json is no longer byte-compatible for names or labels containing non-ASCII text. Preserving the previous escaping or explicitly documenting this additional export-format change would keep the migration contract accurate.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At infrahub_sdk/ctl/telemetry.py, line 127:

<comment>Telemetry exports now contain raw non-ASCII characters instead of the `\uXXXX` escapes produced by the previous serializer, so `telemetry-export.json` is no longer byte-compatible for names or labels containing non-ASCII text. Preserving the previous escaping or explicitly documenting this additional export-format change would keep the migration contract accurate.</comment>

<file context>
@@ -124,5 +124,5 @@ async def export_snapshots(
 
     output_path = Path(output)
-    output_path.write_text(json.dumps(snapshots, indent=2), encoding="utf-8")
+    output_path.write_text(orjson.dumps(snapshots, option=orjson.OPT_INDENT_2).decode(), encoding="utf-8")
     console.print(f"Exported {len(snapshots)} snapshots to {output_path}")
</file context>

"graphql_json": ujson.dumps(n.get_raw_graphql_data()),
}
)
"graphql_json": orjson.dumps(n.get_raw_graphql_data(), option=orjson.OPT_NON_STR_KEYS).decode(),

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.

P2: Exported nodes.json files no longer retain the previous escaped-Unicode byte representation: any non-ASCII node value is written as raw UTF-8 by these new orjson.dumps calls. This affects transfer-file diffs/checksums and contradicts the stated byte-preservation contract beyond the documented tracking-group case; preserving the old escaping at this file boundary would avoid the unannounced format change.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At infrahub_sdk/transfer/exporter/json.py, line 155:

<comment>Exported `nodes.json` files no longer retain the previous escaped-Unicode byte representation: any non-ASCII node value is written as raw UTF-8 by these new `orjson.dumps` calls. This affects transfer-file diffs/checksums and contradicts the stated byte-preservation contract beyond the documented tracking-group case; preserving the old escaping at this file boundary would avoid the unannounced format change.</comment>

<file context>
@@ -148,13 +148,14 @@ async def export(
-                        "graphql_json": ujson.dumps(n.get_raw_graphql_data()),
-                    }
-                )
+                        "graphql_json": orjson.dumps(n.get_raw_graphql_data(), option=orjson.OPT_NON_STR_KEYS).decode(),
+                    },
+                    option=orjson.OPT_NON_STR_KEYS,
</file context>

try:
return json.loads(value)
except (json.JSONDecodeError, TypeError) as exc:
return orjson.loads(value)

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.

P2: Large JSON integers can silently lose precision through this filter: from_json('{"id": 18446744073709551616}') used to return an exact int, while orjson.loads returns a float. Since the PR documents only tracking-group renames as behavior changes, preserving arbitrary-integer parsing here or documenting and testing this additional compatibility change would avoid corrupted template data.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At infrahub_sdk/template/infrahub_filters.py, line 167:

<comment>Large JSON integers can silently lose precision through this filter: `from_json('{"id": 18446744073709551616}')` used to return an exact `int`, while `orjson.loads` returns a `float`. Since the PR documents only tracking-group renames as behavior changes, preserving arbitrary-integer parsing here or documenting and testing this additional compatibility change would avoid corrupted template data.</comment>

<file context>
@@ -164,8 +164,8 @@ def from_json(value: str) -> dict | list:
     try:
-        return json.loads(value)
-    except (json.JSONDecodeError, TypeError) as exc:
+        return orjson.loads(value)
+    except (orjson.JSONDecodeError, TypeError) as exc:
         raise JinjaFilterError(filter_name="from_json", message=f"invalid JSON: {exc}") from exc
</file context>

Comment thread infrahub_sdk/ctl/formatters/json.py Outdated
Comment thread infrahub_sdk/ctl/formatters/json.py Outdated
"""
items = [extract_node_data(node, schema) for node in nodes]
return json.dumps(items, indent=2, default=str)
return orjson.dumps(items, option=orjson.OPT_INDENT_2 | orjson.OPT_PASSTHROUGH_DATETIME, default=str).decode()

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.

P2: Non-ASCII values now have a different CLI JSON representation: the previous json.dumps escaped them, whereas these calls emit raw UTF-8. This breaks the stated byte-for-byte output guarantee and can affect snapshot or string-based consumers; preserving the old escaping or explicitly updating the output contract for both formatter methods would avoid the compatibility regression.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At infrahub_sdk/ctl/formatters/json.py, line 43:

<comment>Non-ASCII values now have a different CLI JSON representation: the previous `json.dumps` escaped them, whereas these calls emit raw UTF-8. This breaks the stated byte-for-byte output guarantee and can affect snapshot or string-based consumers; preserving the old escaping or explicitly updating the output contract for both formatter methods would avoid the compatibility regression.</comment>

<file context>
@@ -39,7 +40,7 @@ def format_list(
         """
         items = [extract_node_data(node, schema) for node in nodes]
-        return json.dumps(items, indent=2, default=str)
+        return orjson.dumps(items, option=orjson.OPT_INDENT_2 | orjson.OPT_PASSTHROUGH_DATETIME, default=str).decode()
 
     def format_detail(self, node: InfrahubNode, schema: MainSchemaTypesAPI) -> str:
</file context>

response_content = ujson.dumps(excinfo.value.response.json(), indent=4, sort_keys=True)
except ujson.JSONDecodeError:
response_content = orjson.dumps(
orjson.loads(excinfo.value.response.content), option=orjson.OPT_INDENT_2 | orjson.OPT_SORT_KEYS

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.

P2: HTTP error output no longer preserves the previous JSON representation: ujson.dumps(..., indent=4, sort_keys=True) emitted four-space indentation and escaped non-ASCII by default, while OPT_INDENT_2 emits two-space indentation and raw UTF-8. This changes the diagnostic text and conflicts with the stated byte-for-byte preservation; retaining the old formatting/escaping would avoid the regression.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At infrahub_sdk/pytest_plugin/items/jinja2_transform.py, line 64:

<comment>HTTP error output no longer preserves the previous JSON representation: `ujson.dumps(..., indent=4, sort_keys=True)` emitted four-space indentation and escaped non-ASCII by default, while `OPT_INDENT_2` emits two-space indentation and raw UTF-8. This changes the diagnostic text and conflicts with the stated byte-for-byte preservation; retaining the old formatting/escaping would avoid the regression.</comment>

<file context>
@@ -60,8 +60,10 @@ def get_result_differences(self, computed: Any) -> str | None:
-                response_content = ujson.dumps(excinfo.value.response.json(), indent=4, sort_keys=True)
-            except ujson.JSONDecodeError:
+                response_content = orjson.dumps(
+                    orjson.loads(excinfo.value.response.content), option=orjson.OPT_INDENT_2 | orjson.OPT_SORT_KEYS
+                ).decode()
+            except orjson.JSONDecodeError:
</file context>

try:
response_content = ujson.dumps(excinfo.value.response.json(), indent=4)
except ujson.JSONDecodeError:
response_content = orjson.dumps(

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.

P3: HTTP error reports now render non-ASCII payload characters as raw UTF-8 instead of the previous \uXXXX escapes, changing user-visible failure text for localized or non-ASCII API errors. Preserving the old output requires an explicit ASCII-escaping step or documenting and testing this additional behavior change.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At infrahub_sdk/pytest_plugin/items/check.py, line 52:

<comment>HTTP error reports now render non-ASCII payload characters as raw UTF-8 instead of the previous `\uXXXX` escapes, changing user-visible failure text for localized or non-ASCII API errors. Preserving the old output requires an explicit ASCII-escaping step or documenting and testing this additional behavior change.</comment>

<file context>
@@ -49,8 +49,10 @@ def run_check(self, variables: dict[str, Any]) -> Any:
             try:
-                response_content = ujson.dumps(excinfo.value.response.json(), indent=4)
-            except ujson.JSONDecodeError:
+                response_content = orjson.dumps(
+                    orjson.loads(excinfo.value.response.content), option=orjson.OPT_INDENT_2
+                ).decode()
</file context>

Comment thread dev/specs/002-orjson-json-migration/quickstart.md Outdated
Comment thread infrahub_sdk/checks.py

if self.output == "stdout":
print(ujson.dumps(log_message))
print(orjson.dumps(log_message).decode())

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.

P3: Check stdout containing non-ASCII text now changes from escaped JSON to raw UTF-8, breaking the stated byte-for-byte output guarantee and potentially affecting consumers that compare or record these log lines. A compatibility-preserving encoding path or an explicit documented/tested exception for check stdout would keep this behavior intentional.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At infrahub_sdk/checks.py, line 112:

<comment>Check stdout containing non-ASCII text now changes from escaped JSON to raw UTF-8, breaking the stated byte-for-byte output guarantee and potentially affecting consumers that compare or record these log lines. A compatibility-preserving encoding path or an explicit documented/tested exception for check stdout would keep this behavior intentional.</comment>

<file context>
@@ -109,7 +109,7 @@ def _write_log_entry(
 
         if self.output == "stdout":
-            print(ujson.dumps(log_message))
+            print(orjson.dumps(log_message).decode())
 
     def log_error(self, message: str, object_id: str | None = None, object_type: str | None = None) -> None:
</file context>

response_content = ujson.dumps(excinfo.value.response.json(), indent=4)
except ujson.JSONDecodeError:
response_content = orjson.dumps(
orjson.loads(excinfo.value.response.content), option=orjson.OPT_INDENT_2

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.

P3: Non-UTF-8 JSON error responses are no longer parsed and pretty-printed: the raw byte payload is passed directly to orjson.loads, so UTF-16/UTF-32 responses fall through to the unformatted text fallback. Preserving HTTPX's charset-aware decoding would retain the prior behavior for these responses.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At infrahub_sdk/pytest_plugin/items/graphql_query.py, line 32:

<comment>Non-UTF-8 JSON error responses are no longer parsed and pretty-printed: the raw byte payload is passed directly to `orjson.loads`, so UTF-16/UTF-32 responses fall through to the unformatted text fallback. Preserving HTTPX's charset-aware decoding would retain the prior behavior for these responses.</comment>

<file context>
@@ -28,8 +28,10 @@ def execute_query(self) -> Any:
-                response_content = ujson.dumps(excinfo.value.response.json(), indent=4)
-            except ujson.JSONDecodeError:
+                response_content = orjson.dumps(
+                    orjson.loads(excinfo.value.response.content), option=orjson.OPT_INDENT_2
+                ).decode()
+            except orjson.JSONDecodeError:
</file context>

Wrap the __init__.py file reference in a code span so rumdl no longer
parses the double underscores as strong emphasis.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 16, 2026

Copy link
Copy Markdown

Deploying infrahub-sdk-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: 51ff7a4
Status: ✅  Deploy successful!
Preview URL: https://af331dd2.infrahub-sdk-python.pages.dev
Branch Preview URL: https://dga-feat-orjson-pd5o6.infrahub-sdk-python.pages.dev

View logs

@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.19298% with 13 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
infrahub_sdk/pytest_plugin/items/check.py 33.33% 2 Missing ⚠️
infrahub_sdk/pytest_plugin/items/graphql_query.py 33.33% 2 Missing ⚠️
...frahub_sdk/pytest_plugin/items/jinja2_transform.py 33.33% 2 Missing ⚠️
...frahub_sdk/pytest_plugin/items/python_transform.py 33.33% 2 Missing ⚠️
infrahub_sdk/checks.py 50.00% 1 Missing ⚠️
infrahub_sdk/ctl/parsers.py 66.66% 1 Missing ⚠️
infrahub_sdk/ctl/telemetry.py 50.00% 1 Missing ⚠️
infrahub_sdk/ctl/validate.py 50.00% 1 Missing ⚠️
infrahub_sdk/schema/__init__.py 66.66% 1 Missing ⚠️
@@            Coverage Diff             @@
##           stable    #1185      +/-   ##
==========================================
+ Coverage   82.49%   82.50%   +0.01%     
==========================================
  Files         139      138       -1     
  Lines       12210    12183      -27     
  Branches     1823     1833      +10     
==========================================
- Hits        10073    10052      -21     
+ Misses       1576     1569       -7     
- Partials      561      562       +1     
Flag Coverage Δ
integration-tests 40.58% <42.10%> (-0.16%) ⬇️
python-3.10 56.43% <45.61%> (+0.45%) ⬆️
python-3.11 56.43% <45.61%> (+0.43%) ⬆️
python-3.12 56.43% <45.61%> (+0.43%) ⬆️
python-3.13 56.43% <45.61%> (+0.43%) ⬆️
python-3.14 56.42% <45.61%> (+0.41%) ⬆️
python-filler-3.12 22.20% <28.07%> (-0.31%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
infrahub_sdk/client.py 75.54% <100.00%> (-1.00%) ⬇️
infrahub_sdk/ctl/cli_commands.py 72.54% <100.00%> (-0.43%) ⬇️
infrahub_sdk/ctl/formatters/json.py 100.00% <100.00%> (ø)
infrahub_sdk/graphql/multipart.py 90.90% <100.00%> (ø)
infrahub_sdk/graphql/renderers.py 95.18% <100.00%> (ø)
infrahub_sdk/playback.py 95.65% <100.00%> (+7.65%) ⬆️
infrahub_sdk/pytest_plugin/items/base.py 78.37% <100.00%> (ø)
infrahub_sdk/pytest_plugin/models.py 79.09% <100.00%> (ø)
infrahub_sdk/recorder.py 90.47% <100.00%> (+4.76%) ⬆️
infrahub_sdk/template/infrahub_filters.py 93.58% <100.00%> (ø)
... and 12 more

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

dgarros and others added 2 commits July 16, 2026 08:59
ujson and stdlib json silently coerced non-string dict keys to strings;
orjson raises without OPT_NON_STR_KEYS. Add the option at the remaining
arbitrary-data serialization sites (CLI transform output, CLI JSON
formatter, playback request payload) to match prior behaviour, alongside
the exporter/multipart sites that already had it. Add a formatter test
covering int-keyed values.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Correct the overclaimed 'byte-for-byte' contract: non-ASCII data is now
emitted as raw UTF-8 (not \uXXXX escapes) at every JSON output site.
Document pre-migration playback recording incompatibility, the from_json
large-integer float coercion, and UTF-8-only decoding. Broaden the
quickstart import-verification grep to catch 'from json import' too.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dgarros

dgarros commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Response to cubic-dev-ai review (16 findings)

Thanks — the findings collapse into three root causes (all inherent orjson vs ujson/stdlib differences). Addressed in 0eb537c (code) and 51ff7a4 (docs).

✅ Incorporated — code (0eb537c)

Missing OPT_NON_STR_KEYS (a real regression: stdlib/ujson silently coerced non-string dict keys, orjson raises). Added the option at the remaining arbitrary-data sites, matching the exporter/multipart sites that already had it:

  • ctl/cli_commands.py:358 — transform output (strongest case: user transforms can return int-keyed dicts)
  • ctl/formatters/json.py — both format_list and format_detail
  • playback.py:51 — request payload

Added a formatter unit test covering int-keyed values (also closes a previously-deferred coverage gap).

✅ Incorporated — docs (51ff7a4)

Non-ASCII \uXXXX → raw UTF-8 (9 findings). This is correct and our contract overclaimed "byte-for-byte." orjson has no ensure_ascii option, and raw UTF-8 is valid JSON that round-trips through the SDK — re-escaping would add hot-path overhead for no functional gain. So we corrected the documentation instead of the behaviour: the changelog and spec now state non-ASCII is emitted as raw UTF-8 at every output site (CLI, validate --out, telemetry/transfer exports, check logs, pytest messages). Also broadened the quickstart import-check grep to catch from json import.

📝 Documented as known limitations (accepted, not fixed)

  • from_json large-int → float (infrahub_filters.py): real but silent only for integers > 2⁶⁴ (Infrahub IDs are UUID strings); guarding needs a custom parser. Documented.
  • Playback recordings from older SDK versions won't be located (request-body hash changed): recordings are regenerated test fixtures. Documented.
  • orjson.loads UTF-8-only (schema/__init__.py, pytest items): the Infrahub API always responds in UTF-8; the pytest cases only affect pretty-printing of an error fallback (text still shown).

↩️ Declined (with reason)

  • Re-adding \uXXXX escaping anywhere — contrary to adopting orjson; adds post-processing on the hot path; raw UTF-8 is standard and generally more readable.
  • NaN/Infinity handling — orjson's stricter, spec-valid behaviour is preferable; not expected in API traffic.

@cubic-dev-ai cubic-dev-ai 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.

1 issue found across 7 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="changelog/+orjson-json-migration.changed.md">

<violation number="1" location="changelog/+orjson-json-migration.changed.md:1">
P3: ASCII JSON containing an integer larger than 64 bits is not behavior-preserving: `from_json` now routes through `orjson.loads`, and the limitation documented below changes the value's type/precision. Qualify the opening claim or explicitly exclude this documented large-integer case so the migration note does not contradict itself.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

@@ -0,0 +1,7 @@
Standardized JSON serialization across the SDK on `orjson`, replacing `ujson` and stdlib `json`. Behaviour is preserved for ASCII data, with two categories of change for non-ASCII data and legacy on-disk artifacts:

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.

P3: ASCII JSON containing an integer larger than 64 bits is not behavior-preserving: from_json now routes through orjson.loads, and the limitation documented below changes the value's type/precision. Qualify the opening claim or explicitly exclude this documented large-integer case so the migration note does not contradict itself.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At changelog/+orjson-json-migration.changed.md, line 1:

<comment>ASCII JSON containing an integer larger than 64 bits is not behavior-preserving: `from_json` now routes through `orjson.loads`, and the limitation documented below changes the value's type/precision. Qualify the opening claim or explicitly exclude this documented large-integer case so the migration note does not contradict itself.</comment>

<file context>
@@ -1 +1,7 @@
-Standardized JSON serialization across the SDK on `orjson`, replacing `ujson` and stdlib `json`. Observable behaviour is preserved except for one documented change: the tracking-group name derived from query parameters containing non-ASCII characters shifts once on upgrade (`orjson` emits raw UTF-8 where the previous library emitted escaped sequences). The previously-created group is orphaned and a new one is created; any cleanup of the orphaned group is manual.
+Standardized JSON serialization across the SDK on `orjson`, replacing `ujson` and stdlib `json`. Behaviour is preserved for ASCII data, with two categories of change for non-ASCII data and legacy on-disk artifacts:
+
+- **Non-ASCII characters are now emitted as raw UTF-8** rather than `\uXXXX` escapes, across all JSON the SDK writes (CLI output, `infrahubctl validate --out`, telemetry and transfer exports, check logs, and pytest failure messages). The output is still valid JSON and round-trips correctly through the SDK; only its exact bytes differ, which may affect external tools that byte-compare or checksum these files.
</file context>
Suggested change
Standardized JSON serialization across the SDK on `orjson`, replacing `ujson` and stdlib `json`. Behaviour is preserved for ASCII data, with two categories of change for non-ASCII data and legacy on-disk artifacts:
Standardized JSON serialization across the SDK on `orjson`, replacing `ujson` and stdlib `json`. Behaviour is preserved for ASCII data except for the large-integer limitation documented below, with two categories of change for non-ASCII data and legacy on-disk artifacts:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/tech-debt Item we know we need to improve way it is implemented

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tech-debt: migrate JSON serialization to orjson for performance tech-debt: standardize JSON library usage (json vs ujson) across the SDK

1 participant