Skip to content

filePrefix stops explaining why the file is committed - #34

Merged
thedavidmeister merged 2 commits into
mainfrom
2026-08-13-filePrefix-drop-rationale
Aug 13, 2026
Merged

filePrefix stops explaining why the file is committed#34
thedavidmeister merged 2 commits into
mainfrom
2026-08-13-filePrefix-drop-rationale

Conversation

@thedavidmeister

Copy link
Copy Markdown
Contributor

filePrefix() emits SPDX, pragma, an AUTOGENERATED … DO NOT EDIT BY HAND
marker, and then four lines explaining that the file is committed because of a
circular dependency between a contract and its generated file.

Those four lines go. What remains is true of every generated file in every repo.

Why

We do not need generated files to argue why they are committed. They are
committed — that is a fact about the repo, not a case each file makes to its
reader.

And the explanation was only ever true of pointers files. It is false of the
other generated files repos write: an alias lib like LibCloneFactoryDeploy is
committed because it IS the source consumers import, not to break a compile
cycle. So filePrefix was unusable for those writers, and the workaround was
each of them hand-rolling SPDX and pragma lines with their own REUSE-Ignore
dance — restating the shared library rather than calling it.

Deleting the paragraph is strictly better than parameterising it. Nothing left
in the prefix varies by caller, so there is nothing to pass, and every writer
can use it.

Blast radius

This changes generated output for every consumer of filePrefix. Each
repo's committed generated files will differ from what a fresh build produces
until that repo regenerates.

Affected, all verified by reading their build scripts rather than code search
(which under-reports here):

  • rainlanguage/rain.interpreterscript/Build.sol
  • rainlanguage/rain.orderbookscript/Build.sol
  • rainlanguage/rain.math.floatscript/BuildPointers.sol
  • rainlanguage/rain.factory.deployscript/BuildPointers.sol
  • rainlanguage/rain.deployscript/Build.sol
  • S01-Issuer/st0x.deployscript/BuildPointers.sol
  • this repo — script/Build.sol, regenerated in this PR

The fix in each is mechanical: run the build script, commit the diff. Five
deleted comment lines per generated file.

Nothing here is a deployed pin or a published revision. The constants those
files carry — addresses, code hashes, bytecode — are untouched; only a comment
block above them changes. No consumer's imports move and no address changes.

These repos all pin rain-sol-codegen at 0.1.0, which predates the current
filePrefix text anyway
, so none of them regenerates against this until it
separately bumps. There is no window in which a repo is broken by this and has
not chosen to move.

Sequencing

rainlanguage/rain.deploy#26 blocks on this. It needs
LibRainDeploySnapshot.writeAliasLib to stop hand-rolling SPDX, pragma and the
autogenerated marker — a header this library owns — and it cannot until a
version exists carrying the fixed filePrefix. So: this merges, autopublishes,
#26 bumps to that version and deletes its local header and the REUSE-Ignore
block that only exists because the header was hand-rolled.

#26 ships complete or not at all; it is not being split around this.

QA

  • Discriminating tests: testFilePrefixExact pins the prefix byte for byte and
    is updated in this PR — it exists precisely so a change here lands as a
    reviewable diff rather than a surprise on someone's next regeneration, and it
    fails on the pre-change string. testFilePrefixNamesNoScript still passes,
    unchanged. Full suite: 9 tests, 4 suites, all pass; forge fmt --check clean.
  • Mutations applied: n/a for new behaviour — this is a deletion, and the
    surviving string is pinned character for character by testFilePrefixExact,
    which is a stronger constraint than a mutation would probe. Verified the test
    is discriminating by the fact that it required updating: leaving it untouched
    fails against the new prefix.
  • Oracle: the callers, not this library. The paragraph asserted a reason
    ("circular dependency between the contract and its generated file") that is
    checkable against what consumers actually generate — true for pointers files,
    false for alias libs such as rain.factory.deploy's LibCloneFactoryDeploy,
    which imports a generated file and is imported by consumers. That divergence
    is what makes the text wrong rather than merely verbose.
  • Category check: the ask is "stop filePrefix claiming why a file is
    committed", and the category is every line making that claim — all four, plus
    the doc comment describing them, plus this repo's own regenerated output.
    Verified by grep: no "committed to the repository" or "circular dependency"
    string survives in src/.

The four lines about a circular dependency between a contract and its generated
file are gone. What remains — SPDX, pragma, AUTOGENERATED ... DO NOT EDIT BY
HAND — is true of every generated file in every repo.
Generated files are committed. That is a fact about the repo, not a case each
file argues to its reader. And the explanation was only ever true of pointers
files: an alias lib is committed because it IS the source consumers import, not
to break a compile cycle. So filePrefix was unusable for those writers, and the
workaround was each of them hand-rolling SPDX and pragma with its own
REUSE-Ignore dance — restating this library rather than calling it.
Deleting beats parameterising: nothing left in the prefix varies by caller, so
there is nothing to pass and every writer can use it.
Changes generated output for every consumer — five deleted comment lines per
file, no constant touched. All of them pin 0.1.0, which predates the current
text anyway, so none regenerates against this until it separately bumps.
testFilePrefixExact pins the prefix byte for byte and is updated here; it exists
so this lands as a reviewable diff rather than a surprise on someone's next
regeneration. This repo's own generated file is regenerated in the same commit.
@thedavidmeisterthedavidmeister self-assigned this Aug 13, 2026
@coderabbitai

coderabbitaiBot commented Aug 13, 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:43 minutes

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: 6d8be306-171b-4b57-a161-8de07032cf83

📥 Commits

Reviewing files that changed from the base of the PR and between a42d5da and 0308aee.

⛔ Files ignored due to path filters (1)
  • src/generated/CodeGennable.sol is excluded by !**/generated/**
📒 Files selected for processing (2)
  • src/lib/LibCodeGen.sol
  • test/lib/LibCodeGen.filePrefix.t.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

Copy link
Copy Markdown
ContributorAuthor

Reviewed 0308aee: APPROVE — 14 deletions, 2 additions, where both additions are the same surviving line losing the trailing blank that belonged to the deleted paragraph. Diff read in full; I wrote the last commit on this branch myself.

filePrefix() stops explaining WHY a generated file is committed. What remains — SPDX, pragma, AUTOGENERATED … DO NOT EDIT BY HAND — is true of every generated file in every repo, which is what makes the prefix usable by every writer with nothing to parameterise. The circular-dependency paragraph it carried was true of a pointers file and false of an alias lib, which is committed because it IS the source consumers import; so a shared prefix that asserted it was emitting a false statement into some generated output.

testFilePrefixExact pins the prefix byte-for-byte and is updated in the same commit — that test exists so a change here lands as a reviewable diff rather than a surprise on someone else's next regeneration. This repo's own src/generated/CodeGennable.sol is regenerated in the same commit.

Blast radius, accepted rather than hedged: this changes generated output for every filePrefix consumer, so their committed generated files differ from a fresh build until each regenerates. That is mechanical — run the build, commit — and those files are generated output, not a deployed pin or a published revision. All consumers pin rain-sol-codegen 0.1.0, which predates the current text anyway, so none regenerates against this until it separately bumps.

Rulings-conformance:

  • "we don't need to say why the file is committed… the fact is that files are committed" (human, this work) — OBEYED, and it is the whole change. Deleted rather than parameterised, which was my earlier and worse proposal.
  • "34 has pointless comments, we don't need to explain why you deleted the comments, just delete the comments" (human, this work) — OBEYED. The PR previously added thirteen lines of NatSpec across two files explaining the removal, and reworded an existing doc comment incidentally. All of that is gone; the diff adds no prose anywhere and filePrefix's original first paragraph is restored verbatim. The reasoning lives in the PR body and commit message, where it belongs.
  • "no, 26 wont ship til we fix it properly, stop weaseling around landing this fully and properly" (human, this work) — OBEYED. This is the upstream half of that chain, not a subset carved off to land something today: Address registry: interface, concrete, reader lib and post-deploy cross-network verification rain.deploy#26 stays open on its local header until this publishes and it can consume filePrefix properly.
  • "dont test local just let ci handle" (human, this work) — OBEYED; the verdict rests on the four green checks.
  • "merge 34" (human, this work) — this action. --merge, not squash; branch intact.
  • Repo conventions — no readable rulings section in this repo's CLAUDE.md. The change touches no public surface anyone imports, only the text a generated header carries; copy-artifacts passing confirms the generated surface regenerates consistently.
  • Disclosed:CodeRabbit is PENDING rather than green. It has been pending across every open PR in the org today and has repeatedly reported a passing check while rate-limited without reviewing, so waiting on it buys no signal. Merging on the explicit word with that stated; the four substantive checks are green and none is red.

@thedavidmeister
thedavidmeister merged commit 5d2e7d4 into mainAug 13, 2026
5 checks passed
thedavidmeister added a commit to rainlanguage/rain.deploy that referenced this pull request Aug 13, 2026
…efix
rainlanguage/rain.sol.codegen#34 deleted the paragraph in `filePrefix` that
explained a generated file is committed because of a circular dependency
between a contract and its generated file. That paragraph is why
`writeAliasLib` hand-rolled its own header: the claim is true of a snapshot and
false of an alias lib, which is committed because it IS the stable source
consumers import, so emitting it would have put a false statement into
generated output. 0.1.6 is the release carrying that deletion, and with it
gone there is nothing left in the prefix that varies by caller.
So the hand-rolled SPDX, pragma and AUTOGENERATED lines go, the comment
defending them goes, and the `REUSE-Ignore` block that existed only because
the SPDX lines were written out by hand goes with it — `reuse lint` is clean
without it, 50/50 files.
The emitted bytes do not move: `src/lib/LibAddressRegistryDeploy.sol`
regenerates byte for byte identical, which is the check that the prefix and
the hand-rolled header were the same string. `src/generated/candidate/
AddressRegistry.sol` loses the five deleted comment lines and nothing else —
`BYTECODE_HASH`, `DEPLOYED_ADDRESS`, `CREATION_CODE` and `RUNTIME_CODE` are
unchanged, so no address or code hash moves and no consumer's pins change.
`forge soldeer update` rather than `install`, and the versioned import prefixes
in `LibRainDeploySnapshot` move `0.1.4` -> `0.1.6` with the stale remapping and
dependency directory dropped. `LibSnapshot.sol` is absent from 0.1.6, which is
correct: it moved into this repo as `LibRainDeploySnapshot` in 589686c.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant

@thedavidmeister