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 to ensure 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: ec5a7a19-98fb-40e9-8d8e-21186f79a8c8

📥 Commits

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

📒 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)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lotus_json/opt.rs

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 accurately describes the added regression test for LotusJson<Option> schema nullability.
Linked Issues check ✅ Passed The new test verifies that LotusJson<Option> is nullable in schema while LotusJson is not, matching issue #4331.
Out of Scope Changes check ✅ Passed The PR only adds a focused regression test and helper, with no unrelated code changes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ 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 94.73684% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 65.05%. Comparing base (17c49db) to head (afd870c).
⚠️ Report is 36 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/lotus_json/opt.rs 94.73% 1 Missing ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
src/lotus_json/opt.rs 91.17% <94.73%> (+4.50%) ⬆️

... and 66 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...afd870c. 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

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

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

@0xDevNinja lint:clippy check is 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 375530d to afd870c Compare July 21, 2026 12:51
@0xDevNinja

Copy link
Copy Markdown
Contributor Author

Thanks, fixed. The collapsible_match lint was on my helper in opt.rs, so I reworked it to compute the result without the nested if. Verified locally with clippy on the lib and test targets plus the unit tests, all 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