Skip to content

AI review: collect the inputs from the bundle and Sourcify - #3061

Open
marcocastignoli wants to merge 8 commits into
ci/ai-reviewfrom
ci/ai-review-collect
Open

marcocastignoli wants to merge 8 commits into
ci/ai-reviewfrom
ci/ai-review-collect

Conversation

@marcocastignoli

Copy link
Copy Markdown
Member

Step 2 of #3057, onto ci/ai-review.

After the checks of step 1, the job runs .github/scripts/ai-review-collect.js from the base branch: for every deployment of every affected descriptor it fetches the verified sources, the ABI, the NatSpec and the proxy resolution from Sourcify (fields=all, one call per address, two at a time, a shared pause on 429), follows a proxy to its implementations, and decodes the constructor arguments with viem. Deployments that share an implementation source become one review unit; deployments with different code get their own, because two chains can run different code behind one descriptor.

Each unit is one JSON file: the descriptor at head and base, its format keys, its test cases and results from the bundle (rendered screens dropped where they equal the expected ones), the embedded calldata formats it declares, and the contracts with the subset of the Sourcify response the model needs. Bytecode, compiler input and output, metadata and storage layout are not kept. An input above the size cap (1.5 MB by default) loses its largest source files first, never the main file of a contract, and lists what it dropped.

The inputs are the artifact ai-review-inputs, kept 30 days. The bundle is no longer uploaded on its own. The same script runs locally from a bundle file (--bundle <file> --out <dir>), which is how step 3, the price simulation, gets its inputs.

Not in this pull request: the sources of the callees of embedded calldata formats, which need the raw transactions of the test cases; and the trimmed spec, which belongs to the system prompt of step 4.

Tested locally

  • The fork's test pull request (one EIP-712 descriptor, one deployment): one input, 41 KB, constructor argument decoded.
  • feat(symbiotic): add vault, RFQ, rewards and V2 curator descriptors #2990, Symbiotic, the worst case: 73 inputs for 66 descriptors, 26 MB in total, the largest 726 KB, nothing dropped, every contract verified. Seven descriptors split into several units because their implementations differ between chains. Without a Sourcify token, two 429 pauses.

A run on the fork with this version is linked in the comments.

🤖 Generated with Claude Code

@github-actions github-actions Bot added the ci Changes to continuous integration label Sep 28, 2026
@marcocastignoli marcocastignoli mentioned this pull request Sep 28, 2026
17 of 28 tasks
@marcocastignoli

Copy link
Copy Markdown
Member Author

Run on the fork with this version, through the real trigger and the approval: run 36441059487 on fork PR #2. Green 26 seconds after the click: checks, scripts from the base branch, npm ci --ignore-scripts, the Sourcify fetch, and the artifact ai-review-inputs (one input, the QuickSwap permit descriptor, 54 KB).

Posted with Claude Code

@marcocastignoli marcocastignoli self-assigned this Sep 30, 2026
@marcocastignoli

marcocastignoli commented Sep 30, 2026 •

Copy link
Copy Markdown
Member Author
  • how do we differentiate between different contracts
  • we should include also comments and pr description

@marcocastignoli

marcocastignoli commented Sep 30, 2026 •

Copy link
Copy Markdown
Member Author

how do we differentiate between different contracts? this is just a question

Every address of the descriptor is fetched from Sourcify; a proxy also brings its implementation, and each contract carries a role, deployment or implementation. Deployments are then grouped by a key: the SHA-256 of the ABI and of every verified source file (path and content, sorted by path) of the code a call runs, so the implementation for a proxy and the contract itself otherwise. Same key, one review unit; different keys, one unit each, reviewed on its own, and the comment names the implementation and the deployments of each unit. Two side effects worth knowing: the same code verified under different file paths gives two units, and deployments Sourcify does not know share one empty key. The docs page in #3071 spells this out.

we should include also comments and pr description

Done in #3081, stacked on this one: the workflow reads the title, description, comments, reviews and review comments through the API into a JSON file, and the collector adds them to every unit as pullRequest, bot comments removed.

This branch also gained the commit that focuses the sources on the reviewed functions (b155f40), which the benchmark of #3069 was run with.

Posted with Claude Code

marcocastignoli and others added 4 commits October 1, 2026 09:41
Step 2 of the AI review. After the checks, the job runs
ai-review-collect.js from the base branch: for every deployment of every
affected descriptor it fetches the verified sources, the ABI, the NatSpec and
the proxy resolution from Sourcify, follows a proxy to its implementations,
and decodes the constructor arguments. Deployments that share an
implementation source become one review unit; deployments with different
code get their own. Each unit is one JSON file with the descriptor, its test
cases and results from the bundle (rendered screens dropped where they equal
the expected ones) and the contracts. An input above the size cap loses its
largest source files first, never the main file, and lists what it dropped.

The inputs are the artifact ai-review-inputs, kept 30 days; the bundle is no
longer uploaded on its own. The same script runs locally from a bundle file
for the price simulation. On the Symbiotic pull request it produces 73
inputs for 66 descriptors, the largest 726 KB, with nothing dropped.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Keep, per contract, the files that define the functions of the descriptor
(or that hash its EIP-712 type), their base contracts and one level of
callees; drop interfaces, duplicate files and large pure libraries, listed by
name. Limit the ABI and the NatSpec to the reviewed functions, keep the main
file only for a proxy, and cap a unit at 400 KB, dropping callees before
bases and never the files that define the reviewed functions.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
rankSources gets a header that explains the three ranks and why they exist,
three numbered steps, and four small helpers with one job each: the main
file, the distinct files, which file declares a name, which names a file
uses. The output is unchanged: byte-identical on the 25 benchmark inputs.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sourcify returns, for every verified contract, the source maps of the
runtime and the creation bytecode and the file ids they refer to, from the
compilation that matched the chain. The files whose ids occur in the maps
are exactly the files the deployed code was compiled from: the contract,
its base contracts, the libraries inlined into it. That replaces the
regex-based ranking of entries, base contracts and callees, works for
Vyper as it does for Solidity, and makes duplicate files a non-issue.

Large pure libraries are still left out and listed by name. The size cap
drops the largest file first and never the main file. Measured on six
benchmark cases, the inputs are the same size or smaller.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@marcocastignoli
marcocastignoli marked this pull request as draft October 1, 2026 07:41
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
marcocastignoli and others added 3 commits October 1, 2026 10:02
The source maps are the filter: a contract keeps the files its deployed code
was compiled from, chosen as soon as the Sourcify response comes in, and no
further work is done on the sources. The pure-library heuristic goes with
it. What remains per review unit is the trimming of the ABI and the NatSpec
to the functions the descriptor covers, and the main-file-only rule for a
proxy. The prompt and the docs page say the same.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The team does not want source files dropped behind the model's back: either
the whole context goes, or the review does not run. The limit rises to
600 KB, about 200K tokens, the dropping code goes away, and a unit above the
limit is flagged in the index so the review fails for it and says so.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@marcocastignoli
marcocastignoli marked this pull request as ready for review October 1, 2026 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Changes to continuous integration

Projects

Status: Needs Review

Development

Successfully merging this pull request may close these issues.

1 participant