Skip to content

Emit a contract level notice for the four tooling interfaces - #131

Merged
thedavidmeister merged 3 commits into
mainfrom
2026-08-16-issue-93
Aug 17, 2026
Merged

Emit a contract level notice for the four tooling interfaces#131
thedavidmeister merged 3 commits into
mainfrom
2026-08-16-issue-93

Conversation

@thedavidmeister

@thedavidmeister thedavidmeister commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Closes #93

What changed

@notice inserted before the descriptive paragraph in the four tooling
interfaces, exactly as the issue proposed. NatSpec appends untagged /// lines
to the tag above them, so the whole paragraph was being emitted as the @title
and no contract-level userdoc notice was emitted at all, in every published
ABI artifact.

The issue's proposed diff was correct and is applied verbatim: only the one line
per file changes, and the paragraph is deliberately not rewrapped. The
longest resulting line is 89 characters, well inside the repo's 120.

origin/main is merged in (not rebased), so this branch now carries
#124 view mutability
change on the same four files. The diff against main is now four lines: one
per interface.

This can regress silently and no test guards it

Delete any of these four @notice tags and forge test stays green. What
the fix changes is which NatSpec tag solc emits the paragraph under, and after
this revision nothing in this repo's suite reads the emitted NatSpec at all. The
gap is stated here rather than covered.

This PR previously shipped test/lib/LibNatSpec.sol and four
test/src/interface/*.notice.t.sol files — 231 lines that read devdoc.title
and userdoc.notice back out of the build artifact — plus the
{ access = "read", path = "out" } fs_permissions grant they needed. All of
it is deleted here, and fs_permissions is back to main's three-entry block
exactly.

A NatSpec tag-presence rule is generic static analysis over any Solidity source,
not a property of this library, and it was written in Solidity and executed by
forge test. Its home is
rainlanguage/rainix#317, which collects six such
hand-rolled harnesses out of this repo into rainix-static subcommands with
bats coverage, inherited by every consumer through the rainix reusables instead
of copied per repo. Until that lands, this fix is verified by having been read,
and by nothing else.

Cause: the fixer brief driving this fleet said "every fix ships a test,
verify-by-hand is never acceptable", with no exemption for changes that have no
behaviour. That instruction was wrong and has been corrected. It produced
hand-rolled static analysis in Solidity across nine PRs.

Scope left to the sibling issues

Confined to the contract-level @notice that #93 names. Untouched in the same
four files: the missing @return tags (#94) and the
.github/workflows/build-pointers.yaml references in the function docstrings
(#95). The view mutability (#92) landed on main in #124 and is merged in
here rather than edited.

QA

Every command run as nix develop -c ... from the repo root, so the toolchain is
the flake's. Logs at /home/gildlab/artifacts/work/rain.sol.codegen-fix-93-logs/.

  • nix develop -c forge testRan 18 test suites in 1.55s (16.83s CPU time): 142 tests passed, 0 failed, 0 skipped (142 total tests).
  • The same merged tree with the deleted files restored — Ran 22 test suites in 2.53s (23.65s CPU time): 150 tests passed, 0 failed, 0 skipped (150 total tests). The drop is exactly the 4 suites and 8 tests removed by this
    revision, and nothing else moved.
  • nix develop -c forge fmt --check — exit 0, no diff.
  • No new test, by ruling. See the section above.
  • CI on 49db181: build-pointers / copy-artifacts, rainix / test,
    rainix / static and rainix / legal all pass.
  • CodeRabbit did not review this PR. Its check reports pass with
    Review rate limited, and its only comment is Review limit reached ... we couldn't start this review. There are zero review threads, resolved or
    unresolved (queried over GraphQL). That green is an absence of review, not a
    passed one.

Each interface opened with a title tag and continued in untagged `///`
lines, which NatSpec appends to the tag above them. The whole description
was emitted as the title and the contract level notice was emitted not at
all, in every published ABI artifact.

`test/lib/LibNatSpec.sol` reads the title and the notice back out of the
build artifact, and each interface has a test asserting its title is the
interface name alone and that a non-empty notice is emitted. The artifact
is read through `rawMetadata`, because the `metadata` object beside it is
forge's own deserialization and carries no title key.

Closes #93

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@thedavidmeister thedavidmeister self-assigned this Aug 16, 2026
@coderabbitai

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@thedavidmeister, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 1 minute

Limit details: You’ve used all 1 included review currently available under your plan.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: aac87125-d91e-4993-a2f6-c35fef3d9b2e

📥 Commits

Reviewing files that changed from the base of the PR and between b422d97 and 49db181.

📒 Files selected for processing (4)
  • src/interface/IIntegrityToolingV1.sol
  • src/interface/IOpcodeToolingV1.sol
  • src/interface/IParserToolingV1.sol
  • src/interface/ISubParserToolingV1.sol

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.

thedavidmeister and others added 2 commits August 16, 2026 19:20
The four `@notice` insertions are the fix and stay. The tests that read them
back out of the build artifact do not.

`test/lib/LibNatSpec.sol` and the four `test/src/interface/*.notice.t.sol`
files are deleted, and the `{ access = "read", path = "out" }` grant they
needed is reverted, returning `fs_permissions` to its base three entries.

A NatSpec tag presence check is a generic static rule over any Solidity
source, not a property of this library. Its home is rainlanguage/rainix#317.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@thedavidmeister
thedavidmeister merged commit 4ac3f59 into main Aug 17, 2026
5 checks passed
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.

All four tooling interfaces fold their whole description into @title, emitting no contract-level @notice

2 participants