Skip to content

Point the five tooling interface docstrings at a path the published package ships - #132

Closed
thedavidmeister wants to merge 5 commits into
mainfrom
2026-08-16-issue-95
Closed

Point the five tooling interface docstrings at a path the published package ships#132
thedavidmeister wants to merge 5 commits into
mainfrom
2026-08-16-issue-95

Conversation

@thedavidmeister

@thedavidmeisterthedavidmeister commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Important

OBSOLETE against post-#138main — this branch is now an empty diff.
Left open for the human to rule on; nothing here is a close decision.

Issue #95 is CLOSED, completed by #138
(#95). #138 rewrote the
same five docstrings this PR rewrites, to:

/// rainix's `rainix-copy-artifacts.yaml` reusable workflow is that test: it
/// regenerates from the consumer's `script/Build.sol` and fails on any diff
/// against the committed sources. Worked example of the generation side:
/// https://github.com/rainlanguage/rain.deploy/blob/main/script/Build.sol

This PR's replacement text is worse against the current tree, not merely
redundant: it says "script/Build.sol in this package is a worked example of
such a generation", and #138 deleted script/Build.sol from this package. The
human ruling behind #138 is that this repo carries no worked example at all —
rain.deploy is the org's — so restoring that sentence would reintroduce the
scope the ruling removed.

main was merged in and its docstrings taken wholesale.
git diff origin/main on the merge commit is empty. Nothing in this branch is
lost by that: every claim it makes about where the check lives, main now
makes more precisely.


Original PR description, kept for the record.

Closes#95

What

All five tooling-interface function docstrings ended See .github/workflows/build-pointers.yaml for an example of such a test.
.soldeerignore:4 is .github, so that path is guaranteed absent from the
directory consumers read these interfaces from
(dependencies/rain-sol-codegen-<version>/src/interface/).

The five sentences now point at script/Build.sol, which the published package
does carry, and state the check inline so the sentence stands on its own:

/// CI. `script/Build.sol` in this package is a worked example of such a
/// generation, and the test is to run it in CI and fail on any diff against
/// the committed source.

That is the whole change. Five docstrings, no behaviour.

No test, and what that costs

This change can regress silently. No test in this repo guards it. Anyone can
put .github/workflows/build-pointers.yaml — or any other .soldeerignored
path — back into one of these docstrings and the suite will stay green. Nothing
in forge test reads .soldeerignore, and after this PR nothing does.

An earlier revision of this PR added test/src/PublishedSourcePaths.t.sol (134
lines) which read .soldeerignore, walked src, and asserted no shipped .sol
named a stripped directory. That file is deleted here, along with the three
fs_permissions read grants (.soldeerignore, src, script) it needed, so
foundry.toml returns to its base three-entry block unchanged.

The check itself is worth having; the place for it is not a hand-rolled tree
walker written in Solidity and run by forge test. It is rehomed to
rainlanguage/rainix#317,
item 4 ("Published sources name no stripped path"), which will implement it once
in rainix-static in Rust with bats coverage, and every consumer repo inherits
it through the reusables instead of copying it. Until #317 lands, this class of
defect is unenforced everywhere, here included.

The deleted test also would not have caught the live instance of the very defect
it guards. It walked src only, so script/Build.sol — which ships, and whose
line 8 imports ../test/concrete/CodeGennable.sol while .soldeerignore:20 is
/test — was outside its scope by construction. README.md and REUSE.toml
carry the same defect and were likewise outside it. A src-only walk was the
scope that made the harness tractable, not the scope the problem has.
(script/Build.sol's dangling import is owned by #76.)

Where the issue's proposed fix was wrong

The issue proposed pointing at rainix's rainix-copy-artifacts.yaml reusable
workflow. That swaps one path a consumer cannot resolve for another: the
reusable lives in rainlanguage/rainix, not in this package, and naming it
bare gives a reader of dependencies/rain-sol-codegen-<version>/ nothing to
open. The replacement had to be a path the package actually ships.

Verified against the real published artifact, not the repo tree. Downloaded
rain-sol-codegen~0.1.16 (the latest revision on the Soldeer registry at the
time of writing) from
https://soldeer-revisions.s3.amazonaws.com/rain-sol-codegen/0_1_16_16-08-2026_18:45:19_rain.sol.zip
and listed it:

.audit/scope.json README.md src/generated/CodeGennable.sol
.audit/runs.jsonl REUSE.toml src/interface/IIntegrityToolingV1.sol
LICENSE src/lib/LibCodeGen.sol src/interface/IOpcodeToolingV1.sol
LICENSES/ src/lib/LibFs.sol src/interface/IParserToolingV1.sol
LicenseRef-DCL-1.0.txt src/lib/LibHexString.sol src/interface/ISubParserToolingV1.sol
script/Build.sol

No .github/ in the archive; script/Build.sol is in it. Grepping the
extracted archive for .github returns the five interface hits this PR fixes
(plus README and metadata hits, left alone — see "Not touched").

Why this PR shipped a test at all

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 in this repo — 1,173 lines of TOML parsing,
glob expansion and tree walking, catalogued in rainix#317. This PR was one of
them. The docstring fix stands on its own; the harness does not.

Merged origin/main

#92 landed while this was open, changing all four interfaces' mutability from
pure to view and adding a mutability note to each of the five docstrings.
origin/main is merged in (never rebased). Each conflict is resolved by keeping
#92's view signature and its whole note verbatim, and replacing only the
trailing .github/... sentence with this PR's. Nothing from #92, #93 or #94 is
reverted or reworded.

CodeRabbit

CodeRabbit's check reports pass, but its comment on this PR is
Review limit reached — it never started a review. That green is an absence of
review, not a passed one, and there are zero review threads (resolved or
unresolved) to address.

Not touched

QA

  • Full suite on the merge commit with the test deleted:
    Ran 18 test suites in 1.71s (17.06s CPU time): 142 tests passed, 0 failed, 0 skipped (142 total tests). Was 19 suites / 144 tests with
    PublishedSourcePaths.t.sol present; the drop of one suite and two tests is
    exactly what was removed and nothing else.
  • nix develop -c forge fmt --check: exits 0, no diff.
  • foundry.tomlfs_permissions is byte-identical to origin/main's.
  • No new test. See "No test, and what that costs" above — this is a
    five-docstring change with no behaviour, and the repo-wide check that would
    have guarded it belongs in rainix#317, not in forge test.
  • Oracle for the fix itself: the published artifact, not the repo tree —
    rain-sol-codegen~0.1.16 downloaded from the Soldeer registry and its central
    directory listed; no .github/ present, script/Build.sol present.
  • Category check: All five interface docstrings point consumers at .github/workflows/build-pointers.yaml, which .soldeerignore strips from the published package #95 asks for the dangling
    .github/workflows/build-pointers.yaml reference to be removed from all five
    docstrings (IIntegrityToolingV1.sol:18, IOpcodeToolingV1.sol:17,
    IParserToolingV1.sol:17,26-27, ISubParserToolingV1.sol:16) and replaced
    with a durable one; all five covered, and the replacement is verified present
    in the published package.

@thedavidmeisterthedavidmeister self-assigned this Aug 16, 2026
@coderabbitai

coderabbitaiBot 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: 227c3893-5612-4a0f-a98f-0d8c7de282c8

📥 Commits

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

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

thedavidmeisterand others added 3 commits August 16, 2026 19:03
Resolves the four src/interface conflicts by keeping origin/main's view
mutability and its accompanying note, and replacing the stripped .github path
sentence with the script/Build.sol one.
`test/src/PublishedSourcePaths.t.sol` walked `src` in Solidity and parsed
`.soldeerignore` to assert no shipped source names a stripped path. The check is
worth having once, in `rainix-static`, not per-repo in `forge test`:
rainlanguage/rainix#317 item 4 owns it now.
Deleting it returns `foundry.toml`'s `fs_permissions` to its base three entries;
the three read grants (`.soldeerignore`, `src`, `script`) existed only for this
test.
The five docstring rewrites this PR is for are untouched. They now have no test
guarding them and can regress silently, which the PR body states plainly.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This branch rewrote the five tooling-interface docstrings to point at
`script/Build.sol` "in this package" as a worked example. #138 deleted that
file, and rewrote the same five docstrings to point at rainix's
`rainix-copy-artifacts.yaml` as the test and at `rain.deploy`'s
`script/Build.sol` as the worked example — closing issue #95 in the process.
Taking `main`'s docstrings wholesale. Keeping this branch's text would
reintroduce a claim that the package ships a worked example, which is exactly
what #138 removed. This branch is now a no-op against `main`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@thedavidmeister

Copy link
Copy Markdown
ContributorAuthor

Closing: this PR is now an empty diff, and the issue it served is closed.

#138 rewrote the same five docstrings and closed #95 (2026-08-17T03:33Z). This
branch's final commit took main's version wholesale, so it changes 0 files.

Closing rather than merging, because its original text was not merely redundant
after #138 — it was wrong. a5d4b1a wrote into each of the five docstrings:

script/Build.sol in this package is a worked example of such a generation

script/Build.sol no longer exists in this package. The human ruling behind
#138 was that the worked example leaves this repo entirely and
rainlanguage/rain.deploy is the org's worked example, so landing that sentence
would have reintroduced exactly the claim the ruling removed.

What is on main instead names rainix's rainix-copy-artifacts.yaml as the CI
test and links rain.deploy's script/Build.sol as the worked example — both
of which resolve from the published package, which is what #95 asked for.

Nothing here is lost. The branch is left in place as the audit trail.

Sign up for freeto 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 five interface docstrings point consumers at .github/workflows/build-pointers.yaml, which .soldeerignore strips from the published package

1 participant

@thedavidmeister