Skip to content

fix: properly version json schemas#171

Merged
ascandone merged 3 commits into
mainfrom
fix/update-schemas
Jul 10, 2026
Merged

fix: properly version json schemas#171
ascandone merged 3 commits into
mainfrom
fix/update-schemas

Conversation

@ascandone

@ascandone ascandone commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

On the previous PRs we refactored the specs and inputs schema by doing breaking changes. The change isn't realised yet so it still doesn't break user codebases (and we are still in time to publish utilities to update the schema) but it will break editor diagnostics that use the $schema field.

Fixed it by adding a v1 prefix in the new format, so that we can leave the old format in the repo and user will be able to keep referring to that


Pr can be read commit by commit

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The generated specs_format.Specs metadata now points to the versioned v1.specs.schema.json URL instead of the unversioned schema URL.

Changes

Specs schema metadata

Layer / File(s) Summary
Update generated schema reference
internal/cmd/test_init.go
The Schema field now references the versioned v1 JSON schema URL.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Poem

I’m a bunny with a schema to share,
Now v1 hops through the air.
The specs file knows the way,
No more guessing where to stay.
Nibble, validate, hooray!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title accurately summarizes the schema versioning change in this pull request.
Description check ✅ Passed The description is clearly related to the schema URL versioning update and its compatibility motivation.
✨ 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 fix/update-schemas

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@NumaryBot

Copy link
Copy Markdown
Contributor

🛑 Changes requested — automated review

The patch introduces unversioned JSON schemas that no longer describe the JSON accepted by the CLI and specs runner. This breaks the schema contract even though the Go code itself is unchanged.

@NumaryBot NumaryBot 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.

NumaryBot posted 2 new inline findings.

Summary: #171 (comment)

Comment thread inputs.schema.json
@@ -21,35 +21,18 @@
},
"definitions": {
"Balances": {

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.

🟠 [major] Keep the inputs schema aligned with CLI JSON

When users follow inputs.schema.json, this now validates balances as an account-to-asset object map, but internal/cmd/run.go still unmarshals balances into interpreter.Balances ([]BalanceRow) and metadata into AccountsMetadata ([]AccountMetadataRow). A file accepted by this schema will therefore fail at runtime with a JSON unmarshal error, while the currently supported array form is marked invalid by the unversioned schema.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

false negative

Comment thread specs.schema.json
@@ -83,35 +83,18 @@
},

"Balances": {

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.

🟠 [major] Keep the specs schema aligned with test specs

This changes the default specs schema's Balances definition to an object map, but the specs runner still decodes balances, expect.endBalances, and related fields as interpreter.Balances slices. Specs that validate against specs.schema.json will fail to load in the current runner, and existing valid specs that do not opt into v1.specs.schema.json will be reported as invalid by the default schema.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

false negative

@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.75%. Comparing base (fadd1f8) to head (2b58f8f).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #171      +/-   ##
==========================================
+ Coverage   70.71%   70.75%   +0.03%     
==========================================
  Files          57       57              
  Lines        5262     5262              
==========================================
+ Hits         3721     3723       +2     
+ Misses       1328     1327       -1     
+ Partials      213      212       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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

@ascandone ascandone requested a review from flemzord July 10, 2026 08:41
@ascandone ascandone merged commit dc93829 into main Jul 10, 2026
6 of 7 checks passed
@ascandone ascandone deleted the fix/update-schemas branch July 10, 2026 08:49
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.

3 participants