Skip to content

test(lotus_json): assert LotusJson<Option<T>> is nullable in schema#7347

Open
0xDevNinja wants to merge 1 commit into
ChainSafe:mainfrom
0xDevNinja:0xdevninja/issue-4331-optional-nullable-schema
Open

test(lotus_json): assert LotusJson<Option<T>> is nullable in schema#7347
0xDevNinja wants to merge 1 commit into
ChainSafe:mainfrom
0xDevNinja:0xdevninja/issue-4331-optional-nullable-schema

Conversation

@0xDevNinja

@0xDevNinja 0xDevNinja commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #4331.

LotusJson<Option<T>> delegates its schema to Option<T::LotusJson>, which schemars already renders as nullable ("type": [..., "null"] / a null branch in anyOf). This adds a small regression test locking that behaviour in, as suggested in the issue — verifying that:

  • LotusJson<Option<T>> produces a schema that permits null (i.e. is optional in the OpenRPC spec), and
  • the non-optional LotusJson<T> counterpart does not permit null.

The helper schema_allows_null accepts both the "type" union form (used for primitives/arrays) and the anyOf/oneOf form (used for $ref object types), so the test stays robust across inner types.

No production code changes — test only.

Reference tests

cargo test -p forest-filecoin --lib lotus_json::opt:: — 3 passed (existing snapshot + quickcheck, plus the new optional_is_nullable_in_schema).

Other information and links

Issue originally raised by @ruseinov in #4330 (comment).

Summary by CodeRabbit

  • Tests
    • Added regression coverage ensuring optional Lotus JSON values accept null in generated JSON/OpenRPC schemas.
    • Verified non-optional Lotus JSON values do not permit null in the corresponding schemas.

@0xDevNinja
0xDevNinja requested a review from a team as a code owner July 13, 2026 10:07
@0xDevNinja
0xDevNinja requested review from hanabi1224 and sudo-shashank and removed request for a team July 13, 2026 10:07
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: adda3c4f-e1a7-4a4f-a6c8-69686bd232ac

📥 Commits

Reviewing files that changed from the base of the PR and between de41e74 and 375530d.

📒 Files selected for processing (1)
  • src/lotus_json/opt.rs
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • filecoin-project/lotus (manual)

Walkthrough

Changes

LotusJson schema nullability

Layer / File(s) Summary
Schema nullability regression coverage
src/lotus_json/opt.rs
Adds schema_allows_null and verifies that LotusJson<Option<Cid>> accepts null, while LotusJson<Cid> does not.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: hanabi1224

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly states the main change: a regression test asserting LotusJson<Option> is nullable in schema.
Linked Issues check ✅ Passed The new regression test verifies LotusJson<Option> is nullable in the OpenRPC schema as requested by #4331.
Out of Scope Changes check ✅ Passed The changes stay within scope by adding only a test and test helper for schema nullability.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

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.

@hanabi1224

Copy link
Copy Markdown
Contributor

LGTM, thanks! There is an outstanding lint issue to fix tho.

@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.95238% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.05%. Comparing base (17c49db) to head (375530d).
⚠️ Report is 29 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/lotus_json/opt.rs 80.95% 4 Missing ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
src/lotus_json/opt.rs 83.33% <80.95%> (-3.34%) ⬇️

... and 62 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 17c49db...375530d. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sudo-shashank sudo-shashank 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.

lint:spellcheck failing

Add a regression test verifying that the JSON/OpenRPC schema generated
for LotusJson<Option<T>> permits null (is optional), while the
non-optional LotusJson<T> counterpart does not.

Closes ChainSafe#4331
@0xDevNinja
0xDevNinja force-pushed the 0xdevninja/issue-4331-optional-nullable-schema branch from de41e74 to 375530d Compare July 17, 2026 13:38
@0xDevNinja

Copy link
Copy Markdown
Contributor Author

Good catch, thanks. Fixed the spellcheck failure by rewording the doc comment to avoid the word that tripped the dictionary. Should be green now.

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.

Ensure LotusJson<Option<T>> is marked as optional in the OpenRPC spec

3 participants