From ffacfdc24b426092f2c91c9db485df81785ee83b Mon Sep 17 00:00:00 2001 From: sohey Date: Mon, 5 Oct 2026 19:16:09 +0200 Subject: [PATCH 1/6] test(docs-sync): add eval harness and the base-std@3820cf0 case run-eval.mjs runs the working-tree (or --code-ref) sync code against a pinned docs commit in a throwaway worktree and scores the result against the case's expectations: pages that must, may, and must not change, edit budgets for secondary pages, and restatement of full rules outside their owner pages. The first case is base-std#234 (3820cf0), a NatSpec-only clarification of isAuthorized that the sync spread across ~10 pages (#2054, #2069, #2072). Co-authored-by: Toshi --- .gitignore | 3 + .../__tests__/eval-score.test.mjs | 44 +++ .../cases/3820cf0-isauthorized-natspec.json | 37 +++ scripts/sync-from-base-std/eval/run-eval.mjs | 262 ++++++++++++++++++ ...e-change-3820cf0-isauthorized-natspec.json | 13 + 5 files changed, 359 insertions(+) create mode 100644 scripts/sync-from-base-std/__tests__/eval-score.test.mjs create mode 100644 scripts/sync-from-base-std/eval/cases/3820cf0-isauthorized-natspec.json create mode 100644 scripts/sync-from-base-std/eval/run-eval.mjs create mode 100644 scripts/sync-from-base-std/fixtures/code-change-3820cf0-isauthorized-natspec.json diff --git a/.gitignore b/.gitignore index 56b5d5cb4..2b71f7e2b 100644 --- a/.gitignore +++ b/.gitignore @@ -52,3 +52,6 @@ next-env.d.ts # PR preview screenshots (uploaded via draft GitHub releases, never committed) /screenshots/ + +# docs-sync eval output (scripts/sync-from-base-std/eval) +/.sync-eval/ diff --git a/scripts/sync-from-base-std/__tests__/eval-score.test.mjs b/scripts/sync-from-base-std/__tests__/eval-score.test.mjs new file mode 100644 index 000000000..69991e483 --- /dev/null +++ b/scripts/sync-from-base-std/__tests__/eval-score.test.mjs @@ -0,0 +1,44 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { addedLinesByFile, parseNumstat, scoreRun } from "../eval/run-eval.mjs"; + +const EXPECT = { + must_touch: ["docs/ref/is-authorized.mdx"], + may_touch: { "docs/guide/seize.mdx": 3 }, + must_not_touch: ["docs/guide/hold.mdx"], + unlisted_pages: "fail", + restatement: { owners: ["docs/ref/is-authorized.mdx"], keywords: ["malformed", "empty set", "inverted"], min_keywords: 2 }, +}; + +test("parseNumstat and addedLinesByFile read git output", () => { + assert.deepEqual(parseNumstat("3\t1\tdocs/a.mdx\n-\t-\tdocs/b.png\n"), { + "docs/a.mdx": { added: 3, removed: 1 }, + "docs/b.png": { added: 0, removed: 0 }, + }); + const diff = "diff --git a/docs/a.mdx b/docs/a.mdx\n--- a/docs/a.mdx\n+++ b/docs/a.mdx\n@@ -1 +1,2 @@\n line\n+new\n"; + assert.deepEqual(addedLinesByFile(diff), { "docs/a.mdx": ["new"] }); +}); + +test("scoreRun passes a run that edits only what it should", () => { + const r = scoreRun(EXPECT, { "docs/ref/is-authorized.mdx": { added: 20, removed: 5 }, "docs/guide/seize.mdx": { added: 1, removed: 0 } }); + assert.equal(r.pass, true, r.violations.join("; ")); +}); + +test("scoreRun flags missing, forbidden, oversized, unlisted, and restated edits", () => { + const r = scoreRun( + EXPECT, + { + "docs/guide/hold.mdx": { added: 2, removed: 0 }, + "docs/guide/seize.mdx": { added: 9, removed: 0 }, + "docs/other.mdx": { added: 1, removed: 0 }, + }, + { "docs/guide/seize.mdx": ["A malformed ID returns false; an unknown ID is an empty set."] }, + ); + assert.equal(r.pass, false); + const text = r.violations.join("\n"); + assert.match(text, /missing required edit: docs\/ref\/is-authorized\.mdx/); + assert.match(text, /should leave alone: docs\/guide\/hold\.mdx/); + assert.match(text, /edit too large: docs\/guide\/seize\.mdx adds 9 lines \(budget 3\)/); + assert.match(text, /unlisted page: docs\/other\.mdx/); + assert.match(text, /restates the full rules outside their owner pages: docs\/guide\/seize\.mdx/); +}); diff --git a/scripts/sync-from-base-std/eval/cases/3820cf0-isauthorized-natspec.json b/scripts/sync-from-base-std/eval/cases/3820cf0-isauthorized-natspec.json new file mode 100644 index 000000000..d12be9853 --- /dev/null +++ b/scripts/sync-from-base-std/eval/cases/3820cf0-isauthorized-natspec.json @@ -0,0 +1,37 @@ +{ + "description": "base-std#234 (3820cf0): NatSpec-only clarification of IPolicyRegistry.isAuthorized for malformed, unknown, and inverted IDs. No code or behavior change. The owning reference page and its interface-index row should change; guides that only mention isAuthorized in passing, and historical upgrade or older-fork changelog pages, should not.", + "payload": "../../fixtures/code-change-3820cf0-isauthorized-natspec.json", + "docs_base_sha": "5ceae2479cdd2ac09267bdcfb10342b933406cbc", + "expect": { + "must_touch": [ + "docs/specifications/b20/reference/interfaces/i-policy-registry/is-authorized.mdx", + "docs/specifications/b20/reference/interfaces/i-policy-registry/index.mdx" + ], + "may_touch": { + "docs/specifications/b20/concepts/policies.mdx": 20, + "docs/specifications/b20/reference/constants.mdx": 4, + "docs/base-chain/specs/reference/b20/changelog/03-denim-policyregistry-not-policy.mdx": 20, + "docs/build-on-base/issue-rwa/seize-and-cancel-units.mdx": 3, + "docs/build-on-base/issue-stablecoins/block-an-account.mdx": 3, + "docs/specifications/b20/introduction.mdx": 3 + }, + "must_not_touch": [ + "docs/build-on-base/issue-rwa/restrict-transfer-initiators.mdx", + "docs/build-on-base/issue-stablecoins/restrict-who-can-hold.mdx", + "docs/build-on-base/integrate-defi/list-tokenized-stocks.mdx", + "docs/base-chain/specs/reference/b20/changelog/02-cobalt-policyregistry-composite-policy.mdx", + "docs/base-chain/specs/reference/b20/changelog/03-denim-b20-transfer-executor-enforcement.mdx", + "docs/upgrades/beryl/b20.mdx" + ], + "unlisted_pages": "fail", + "restatement": { + "owners": [ + "docs/specifications/b20/reference/interfaces/i-policy-registry/is-authorized.mdx", + "docs/specifications/b20/concepts/policies.mdx", + "docs/base-chain/specs/reference/b20/changelog/03-denim-policyregistry-not-policy.mdx" + ], + "keywords": ["malformed", "empty set", "INTERSECT", "inverted"], + "min_keywords": 3 + } + } +} diff --git a/scripts/sync-from-base-std/eval/run-eval.mjs b/scripts/sync-from-base-std/eval/run-eval.mjs new file mode 100644 index 000000000..723d0086f --- /dev/null +++ b/scripts/sync-from-base-std/eval/run-eval.mjs @@ -0,0 +1,262 @@ +#!/usr/bin/env node +/** + * run-eval.mjs — measure the docs sync against recorded real-world cases. + * + * Each case pins a dispatch payload and the docs commit it ran against, plus + * the outcome we want (which pages must, may, and must not change, and how + * large edits to secondary pages may be). The harness runs the CURRENT + * working-tree sync code (prompts, routing, route table) against the pinned + * docs tree in a throwaway git worktree, so prompt changes can be compared + * run-for-run instead of judged by eye. + * + * Usage (from the repo root): + * LLM_GATEWAY_API_KEY=... node scripts/sync-from-base-std/eval/run-eval.mjs \ + * [--case 3820cf0-isauthorized-natspec] [--runs 3] [--keep] + * + * --case case file name under eval/cases (without .json); default: all + * --runs independent runs per case (model output varies run to run) + * --keep keep the worktrees for inspection instead of removing them + * --code-ref take the sync code from a git ref (e.g. master) instead + * of the working tree, to get a baseline for comparison + * + * SOURCE_REPO_TOKEN defaults to `gh auth token` so changelog entries can read + * their full source file, the same as in CI. + * + * Results print as a table and are written to .sync-eval/.json. + * Exit code is 1 if any run fails its expectations. + */ + +import { execFileSync, spawn } from "node:child_process"; +import fs from "node:fs/promises"; +import { existsSync } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; + +const HERE = path.dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = path.resolve(HERE, "..", "..", ".."); +const CASES_DIR = path.join(HERE, "cases"); + +// ------------------------------------------------------------- scoring (pure) + +/** Parse `git diff --numstat` into {path: {added, removed}}. */ +export function parseNumstat(text) { + const out = {}; + for (const line of String(text).split("\n")) { + const m = /^(\d+|-)\t(\d+|-)\t(.+)$/.exec(line); + if (!m) continue; + out[m[3]] = { added: Number(m[1]) || 0, removed: Number(m[2]) || 0 }; + } + return out; +} + +/** Added lines per file from a unified diff. */ +export function addedLinesByFile(diff) { + const out = {}; + let file = null; + for (const line of String(diff).split("\n")) { + if (line.startsWith("+++ ")) { + file = line.startsWith("+++ b/") ? line.slice(6) : null; + if (file) out[file] = out[file] || []; + } else if (file && line.startsWith("+") ) { + out[file].push(line.slice(1)); + } + } + return out; +} + +/** + * Score one run against a case's expectations. + * @returns {{pass: boolean, violations: string[], restated: string[]}} + */ +export function scoreRun(expect, numstat, addedByFile = {}) { + const violations = []; + const touched = Object.keys(numstat); + const may = expect.may_touch || {}; + const mustNot = new Set(expect.must_not_touch || []); + const must = expect.must_touch || []; + + for (const p of must) { + if (!numstat[p]) violations.push(`missing required edit: ${p}`); + } + for (const p of touched) { + if (mustNot.has(p)) { + violations.push(`edited a page it should leave alone: ${p} (+${numstat[p].added} -${numstat[p].removed})`); + } else if (p in may) { + if (numstat[p].added > may[p]) { + violations.push(`edit too large: ${p} adds ${numstat[p].added} lines (budget ${may[p]})`); + } + } else if (!must.includes(p) && (expect.unlisted_pages || "fail") === "fail") { + violations.push(`edited an unlisted page: ${p} (+${numstat[p].added} -${numstat[p].removed})`); + } + } + + const restated = []; + const r = expect.restatement; + if (r) { + const owners = new Set(r.owners || []); + for (const [p, lines] of Object.entries(addedByFile)) { + if (owners.has(p)) continue; + const text = lines.join("\n").toLowerCase(); + const hits = r.keywords.filter((k) => text.includes(k.toLowerCase())).length; + if (hits >= (r.min_keywords || r.keywords.length)) restated.push(p); + } + for (const p of restated) violations.push(`restates the full rules outside their owner pages: ${p}`); + } + return { pass: violations.length === 0, violations, restated }; +} + +// ------------------------------------------------------------------ running + +function parseArgs(argv) { + const out = { runs: 1, keep: false, case: null, codeRef: null }; + for (let i = 0; i < argv.length; i++) { + const a = argv[i]; + if (a === "--runs") out.runs = Math.max(1, Number(argv[++i]) || 1); + else if (a === "--case") out.case = argv[++i]; + else if (a === "--keep") out.keep = true; + else if (a === "--code-ref") out.codeRef = argv[++i]; + else throw new Error(`unknown argument '${a}'`); + } + return out; +} + +function git(args, cwd = REPO_ROOT) { + return execFileSync("git", args, { cwd, encoding: "utf8", maxBuffer: 64 * 1024 * 1024 }); +} + +function sourceToken() { + if (process.env.SOURCE_REPO_TOKEN) return process.env.SOURCE_REPO_TOKEN; + try { + return execFileSync("gh", ["auth", "token"], { encoding: "utf8" }).trim(); + } catch { + return ""; + } +} + +async function prepareWorktree(baseSha, label, codeRef) { + // realpath: on macOS os.tmpdir() is under /var, a symlink to /private/var. + // The sync only runs main() when argv[1] equals its resolved module path. + const dir = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), `sync-eval-${label}-`))); + git(["worktree", "add", "--detach", "--force", dir, baseSha]); + if (codeRef) { + // Baseline mode: sync code from a committed ref, docs still pinned. + git(["checkout", codeRef, "--", "scripts/sync-from-base-std", "scripts/lib", "scripts/package.json"], dir); + git(["reset", "-q"], dir); + await fs.rm(path.join(dir, "scripts", "node_modules"), { recursive: true, force: true }); + await fs.symlink(path.join(REPO_ROOT, "scripts", "node_modules"), path.join(dir, "scripts", "node_modules"), "dir"); + return dir; + } + // Overlay the code under test (working tree, including uncommitted edits) + // on top of the pinned docs tree. Docs content and guidelines stay pinned. + const srcScripts = path.join(REPO_ROOT, "scripts"); + const dstScripts = path.join(dir, "scripts"); + await fs.rm(path.join(dstScripts, "sync-from-base-std"), { recursive: true, force: true }); + await fs.cp(path.join(srcScripts, "sync-from-base-std"), path.join(dstScripts, "sync-from-base-std"), { + recursive: true, + }); + await fs.cp(path.join(srcScripts, "lib"), path.join(dstScripts, "lib"), { recursive: true, force: true }); + await fs.cp(path.join(srcScripts, "package.json"), path.join(dstScripts, "package.json")); + await fs.rm(path.join(dstScripts, "node_modules"), { recursive: true, force: true }); + await fs.symlink(path.join(srcScripts, "node_modules"), path.join(dstScripts, "node_modules"), "dir"); + return dir; +} + +function runSync(dir, payloadPath, logPath) { + return new Promise((resolve) => { + const env = { + ...process.env, + SOURCE_REPO_TOKEN: sourceToken(), + GITHUB_OUTPUT: path.join(dir, ".eval-github-output"), + RUNNER_TEMP: path.join(dir, ".eval-runner-temp"), + }; + delete env.DRY_RUN; + execFileSync("mkdir", ["-p", env.RUNNER_TEMP]); + const child = spawn( + process.execPath, + [path.join(dir, "scripts/sync-from-base-std/index.mjs"), "--payload", payloadPath], + { cwd: dir, env }, + ); + const chunks = []; + child.stdout.on("data", (d) => chunks.push(d)); + child.stderr.on("data", (d) => chunks.push(d)); + child.on("close", async (code) => { + await fs.writeFile(logPath, Buffer.concat(chunks)); + resolve(code); + }); + }); +} + +async function runOnce(caseName, spec, runIndex, outDir, keep, codeRef) { + const label = `${caseName}-${runIndex}`; + const dir = await prepareWorktree(spec.docs_base_sha, label, codeRef); + const logPath = path.join(outDir, `${label}.log`); + try { + const payloadPath = path.resolve(CASES_DIR, spec.payload); + const code = await runSync(dir, payloadPath, logPath); + // Only docs/ content counts; ignore the overlaid scripts and eval scratch. + const numstat = parseNumstat(git(["diff", "--numstat", "--", "docs"], dir)); + const untracked = git(["ls-files", "--others", "--exclude-standard", "--", "docs"], dir) + .split("\n") + .filter(Boolean); + for (const p of untracked) { + const added = (await fs.readFile(path.join(dir, p), "utf8")).split("\n").length; + numstat[p] = { added, removed: 0 }; + } + const diff = git(["diff", "--", "docs"], dir); + await fs.writeFile(path.join(outDir, `${label}.diff`), diff); + const score = scoreRun(spec.expect, numstat, addedLinesByFile(diff)); + if (code !== 0) score.violations.unshift(`sync exited with code ${code} (see ${logPath})`); + score.pass = score.pass && code === 0; + return { case: caseName, run: runIndex, exitCode: code, numstat, ...score }; + } finally { + if (!keep) { + git(["worktree", "remove", "--force", dir]); + } else { + console.log(`[eval] kept worktree ${dir}`); + } + } +} + +function printSummary(results) { + for (const r of results) { + const files = Object.entries(r.numstat) + .map(([p, s]) => ` ${p.replace(/^docs\//, "")} +${s.added} -${s.removed}`) + .join("\n"); + console.log(`\n${r.pass ? "PASS" : "FAIL"} ${r.case} run ${r.run} — ${Object.keys(r.numstat).length} page(s)`); + if (files) console.log(files); + for (const v of r.violations) console.log(` ✗ ${v}`); + } + const passed = results.filter((r) => r.pass).length; + console.log(`\n${passed}/${results.length} run(s) passed`); +} + +async function main() { + const args = parseArgs(process.argv.slice(2)); + if (!process.env.LLM_GATEWAY_API_KEY) throw new Error("LLM_GATEWAY_API_KEY is not set"); + const caseFiles = args.case + ? [`${args.case}.json`] + : (await fs.readdir(CASES_DIR)).filter((f) => f.endsWith(".json")); + const stamp = new Date().toISOString().replace(/[:.]/g, "-"); + const outDir = path.join(REPO_ROOT, ".sync-eval", `${stamp}-${args.codeRef ? args.codeRef.replace(/[^\w.-]/g, "_") : "worktree"}`); + await fs.mkdir(outDir, { recursive: true }); + + const jobs = []; + for (const file of caseFiles) { + const caseName = file.replace(/\.json$/, ""); + const spec = JSON.parse(await fs.readFile(path.join(CASES_DIR, file), "utf8")); + for (let i = 1; i <= args.runs; i++) jobs.push(runOnce(caseName, spec, i, outDir, args.keep, args.codeRef)); + } + const results = await Promise.all(jobs); + await fs.writeFile(path.join(outDir, "results.json"), JSON.stringify(results, null, 2)); + printSummary(results); + console.log(`\nLogs, diffs, and results.json: ${path.relative(REPO_ROOT, outDir)}/`); + if (results.some((r) => !r.pass)) process.exitCode = 1; +} + +if (import.meta.url === pathToFileURL(process.argv[1] ?? "").href) { + main().catch((err) => { + console.error(`[eval] ${err.message}`); + process.exit(1); + }); +} diff --git a/scripts/sync-from-base-std/fixtures/code-change-3820cf0-isauthorized-natspec.json b/scripts/sync-from-base-std/fixtures/code-change-3820cf0-isauthorized-natspec.json new file mode 100644 index 000000000..8faf33038 --- /dev/null +++ b/scripts/sync-from-base-std/fixtures/code-change-3820cf0-isauthorized-natspec.json @@ -0,0 +1,13 @@ +{ + "changed_paths": [ + "src/interfaces/IPolicyRegistry.sol" + ], + "diff": "diff --git a/src/interfaces/IPolicyRegistry.sol b/src/interfaces/IPolicyRegistry.sol\nindex 454fd84..3495b8b 100644\n--- a/src/interfaces/IPolicyRegistry.sol\n+++ b/src/interfaces/IPolicyRegistry.sol\n@@ -224,13 +224,16 @@ interface IPolicyRegistry {\n AUTHORIZATION QUERIES\n //////////////////////////////////////////////////////////////*/\n \n- /// @notice Returns whether `account` is authorized under `policyId`. Never reverts; unknown\n- /// or malformed IDs collapse to empty-member-set semantics (ALLOWLIST -> false,\n- /// BLOCKLIST -> true).\n+ /// @notice Returns whether `account` is authorized under `policyId`. Never reverts.\n ///\n+ /// @dev A malformed ID returns false. The invert flag (bit 63) is cleared before the type\n+ /// check, so an ID is malformed when the remaining top byte is outside `PolicyType`.\n+ /// @dev A well-formed but unknown ID is an empty set: ALLOWLIST and UNION return false;\n+ /// BLOCKLIST and INTERSECT return true.\n /// @dev Callers that store policy IDs MUST validate `policyExists(policyId)` at write time.\n /// @dev Invert: `isAuthorized(invertedPolicyId(id), account)` returns the negated\n- /// result of the base. Applies to every policy type.\n+ /// result of the base when that base exists. Applies to every policy type. An\n+ /// inverted unknown or malformed base returns false.\n ///\n /// @param policyId Policy to query.\n /// @param account Account to check.\n", + "diff_truncated": false, + "kind": "code-change", + "pr_body": "## Summary\n- Document that `isAuthorized` returns false for a malformed policy ID, after clearing the invert flag.\n- Separate that from empty-set semantics for a well-formed unknown ID, and state that an inverted ID is negated only when the base exists.\n\n## Test plan\n- [ ] Confirm the NatSpec matches `MockPolicyRegistry._isAuthorized` for malformed, unknown, and inverted IDs.\n- [ ] `forge test -vv --match-test test_isAuthorized_success_invertUnknown`\n\n\nMade with [Cursor](https://cursor.com)", + "pr_number": "234", + "pr_title": "docs(policy): document malformed isAuthorized results", + "sha": "3820cf0d260a2d7b0fdd8f580ab4942fc7856fbb", + "source_repo": "base/base-std" +} From e6cd9dc4ae66f420055dacec260f67180a223694 Mon Sep 17 00:00:00 2001 From: sohey Date: Mon, 5 Oct 2026 19:16:10 +0200 Subject: [PATCH 2/6] fix(docs-sync): stop guide pages being edited for passing mentions - Rule 5: guides and concept pages change only when the source change makes a step, outcome, revert, or recommended setting on the page wrong; otherwise they are returned unchanged. Add 'one owner per fact': full behavior lives on the owning reference page; other pages link to it. - Rule 6: inventory documented surfaces, not mentions. A manifest entry that matches only a mention is not an intersection. Comment-only diffs clarify behavior; edit only statements they show to be wrong. - Step 4: polish only edited paragraphs; callouts only for real behavior changes, never for clarifications. - The anti-noop rule now applies to reference and changelog-entry pages that document the changed symbol, not to pages that only mention it. Co-authored-by: Toshi --- scripts/sync-from-base-std/llm/prompts.mjs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/scripts/sync-from-base-std/llm/prompts.mjs b/scripts/sync-from-base-std/llm/prompts.mjs index 64c32b715..b7f9274ec 100644 --- a/scripts/sync-from-base-std/llm/prompts.mjs +++ b/scripts/sync-from-base-std/llm/prompts.mjs @@ -104,16 +104,18 @@ const SHARED_RULES = `Hard requirements for your output: • FUNCTION REFERENCE pages under \`.../reference/interfaces//.mdx\` own the Solidity signature, selector, parameters, returns, revert conditions, and behavior for exactly one callable surface. Update only claims grounded in the current page or verified Base Std inputs. • INTERFACE INDEX pages such as \`.../reference/interfaces/ib20/index.mdx\` own the function/event/error inventory and links to function pages. Keep selector and topic tables consistent with the verified source diff. • SPECIFICATION / SHARED REFERENCE pages own cross-interface concepts such as roles, policies, addresses, common errors, and events. Do not duplicate those full explanations on every function page. - • GUIDES / PLAYGROUND / DEMO pages explain user workflows. Update them only when the source change alters a command, call sequence, supported behavior, or developer-facing recommendation. Do not copy full ABI tables into guides. + • GUIDES / CONCEPT / PLAYGROUND / DEMO pages (role \`guide\`) explain workflows and ideas, not API contracts. Naming a symbol does not make a guide document it. Edit a guide only when the change makes something on the page wrong or misleading for a reader following it: a command, call sequence, expected outcome, revert, or recommended setting. Otherwise return it unchanged. When an edit is needed, fix the affected sentence. If the reader needs a new detail to make a decision on this page, add at most one sentence and link to the owning reference page. Do not copy full ABI tables into guides. + • ONE OWNER PER FACT. A symbol's full behavior — edge cases, result tables, revert lists, parameter semantics — lives only on the page that owns it: its function reference page, or the shared-reference page that defines the concept. Every other page links there instead of restating it. Never add a result table, an edge-case list, or a multi-sentence summary of another page's symbol. • CHANGELOG ENTRY pages (one per feature per hardfork) own the migration record for exactly one change: Abstract, Motivation, What changed (Solidity code blocks, new errors/events, before/after diffs — not prose), Migration, and optionally Alternatives considered and Test cases, as defined in the content guidelines. They pull facts and code from the source entry but follow the docs page shape, never the source's section layout. They link to the reference pages for the full current state instead of restating it. • CHANGELOG SUMMARY pages own one table row per feature per hardfork (product, change, affected interfaces, link to the entry page) and nothing else. Never add sections, callouts, or code to a summary page. • The sync updates existing files only, with one exception: a changelog entry page the prompt explicitly marks as new. Never invent or link to any other page that is not present in the candidate route set. 6. STRUCTURED REFLECTION. Before producing the output, run this 4-step enumeration internally (silently): - STEP 1 — INVENTORY. Read \`\` and list every API surface it documents. Method names, type names, field names with their current types, parameter signatures, response-shape entries, error codes, default values. Be explicit. + STEP 1 — INVENTORY. Read \`\` and list every API surface it DOCUMENTS: signatures, parameters and return values, behavior and revert conditions, errors, events, and defaults that the page states as facts. A symbol that is only mentioned — used in an example call, linked to, or named in a sentence without describing its behavior — is a mention, not a documented surface. Keep mentions out of the inventory. STEP 2 — INTERSECT. For each item from step 1, identify the changes in the source that touch it. - • If a \`\` block is present below, it has already been extracted from the diff by a pre-pass. Every entry in it that names something from your step-1 inventory IS an intersection — treat the manifest as the authoritative starting list. Do not skip a manifest entry that matches an inventory item. + • If a \`\` block is present below, it has already been extracted from the diff by a pre-pass. Every entry in it that names a surface from your step-1 inventory is an intersection — treat the manifest as the starting list. A manifest entry that matches only a mention is not an intersection. + • If the diff changes only comments or NatSpec and no code, it clarifies existing behavior rather than changing it. Treat a statement on the page as an intersection only if the clarification shows it is wrong or incomplete in a way that would mislead the reader. • Always cross-check the manifest against \`\`: the manifest may miss something, especially newly-added fields buried in large diffs. If you find an additional intersection in the diff that isn't in the manifest, add it to your list and apply it in step 3. • If \`\` is absent or empty, fall back to scanning \`\` directly for: – Solidity parameter or return-type changes @@ -129,13 +131,13 @@ const SHARED_RULES = `Hard requirements for your output: • parameter or return type changed → update the signature and relevant table row • function signature changed → update the signature, selector, parameters, returns, and examples consistently • error/event/role/policy changed → update only the index or shared-reference page that owns it - • behavior or revert condition changed → update the owning function page and any explicitly routed guide + • behavior or revert condition changed → update the owning function page; update a guide or concept page only under the rule-5 test • default value changed → update the default column or the prose that states the default You must make EVERY edit step 2 surfaced. A single missed intersection is a defect, regardless of how minor. - STEP 4 — POLISH (apply the documentation guidelines). After step-3 edits, look at the page with a writer's eye. The block below contains the canonical content and information-architecture rules. Apply them. Ask yourself "what am I trying to say?" for each paragraph that touched a step-3 edit, and rewrite if the answer reveals a clearer way to say it. Add transition phrasing where it helps; remove transition phrasing where it makes the page stilted. If a step-3 edit changed a contract consumers depend on, add a brief — except on changelog pages, where the change is recorded in the entry's Migration section instead, never as a callout. If a new field needs an example to be understood, add one. If a conceptual or quickstart page hand-waves around something step 3 just changed in a reference page, tighten the conceptual page's prose to match — link to the reference page for the specifics. Clarity beats tone; useful information in a clear and direct way is the most important part. Editorial work that earns its place is welcome; filler that doesn't help the reader isn't. + STEP 4 — POLISH (apply the documentation guidelines). The block below contains the canonical content and information-architecture rules. Apply them only to the paragraphs you changed in step 3: make each one say what it means clearly and directly. Do not reword, reorder, or restructure any other part of the page. Add a brief only when a step-3 edit records a behavior change that breaks or surprises consumers (a new revert, a changed result, a changed signature) — never for a clarification of existing behavior, never on a page that only links to the owning reference, and never on changelog pages, where the change belongs in the entry's Migration section. If a new field needs an example to be understood, add one. Filler that doesn't help the reader isn't welcome. - Return the page UNCHANGED only when step 2 found ZERO intersections — i.e., the page genuinely documents APIs that the diff does not touch. If step 2 found ANY intersection, you MUST output the modified page with the step-3 edits applied. Returning the page byte-equal to current after step 2 surfaced intersections is the failure mode this rule exists to prevent. + Return the page UNCHANGED when step 2 found no intersection with a surface the page documents. On function-reference, interface-index, and changelog-entry pages that document a changed symbol, returning the page byte-equal to current is the failure mode this rule exists to prevent: apply every step-3 edit. On guide and shared-reference pages that only mention the symbol, returning the page unchanged is the correct result. 7. Keep prose terse. Do not add filler. 8. Internal links MUST use a full route that already exists under \`docs/\`. Correct: \`/specifications/b20/reference/interfaces\`. Never invent a route for a newly added Solidity symbol; this workflow edits existing pages only. 9. CRITICAL — source-grounded claims. Every concrete identifier you write — interface and function names, selectors, parameter and return types, errors, events, roles, policies, addresses, versions, and file paths — MUST appear verbatim in the verified source diff, release notes, listed source files, or current page. Omit information that is not grounded rather than guessing. @@ -187,7 +189,7 @@ function changeManifestSection(manifest) { }); return ` -A pre-pass over the diff extracted the following API-level changes that originate in files this page documents. Treat this as the authoritative intersection list for STEP 2 of rule #6: every entry below that names something in your step-1 inventory IS a required edit. The manifest may be incomplete; cross-check against \`\` and add anything you find that the manifest missed. +A pre-pass over the diff extracted the following API-level changes that originate in files this page documents. Use it as the starting intersection list for STEP 2 of rule #6: an entry that names a surface this page documents (not just mentions) is a required edit. The manifest may be incomplete; cross-check against \`\` and add anything you find that the manifest missed. ${lines.join("\n")} From 23f68ac26da171bcfe204c8eb0e192bb4fe21ac7 Mon Sep 17 00:00:00 2001 From: sohey Date: Mon, 5 Oct 2026 19:16:10 +0200 Subject: [PATCH 3/6] fix(docs-sync): route comment-only changes to reference pages; skip historical pages - isCommentOnlyChange: a dispatch whose source diff only edits Solidity comments is a clarification. Symbol-mention routing then reaches only function-reference and interface-index pages, and the prompt is told the change type. - symbolRouteGate: pages found only by symbol mention are not routed when they are changelog entries for an earlier hardfork or upgrades// pages for a fork other than the newest. Path-routed pages are unaffected. Co-authored-by: Toshi --- .../__tests__/symbol-route-gate.test.mjs | 88 +++++++++++++++++++ scripts/sync-from-base-std/index.mjs | 25 +++++- scripts/sync-from-base-std/llm/prompts.mjs | 5 +- scripts/sync-from-base-std/release-utils.mjs | 71 +++++++++++++++ 4 files changed, 187 insertions(+), 2 deletions(-) create mode 100644 scripts/sync-from-base-std/__tests__/symbol-route-gate.test.mjs diff --git a/scripts/sync-from-base-std/__tests__/symbol-route-gate.test.mjs b/scripts/sync-from-base-std/__tests__/symbol-route-gate.test.mjs new file mode 100644 index 000000000..34f439eb2 --- /dev/null +++ b/scripts/sync-from-base-std/__tests__/symbol-route-gate.test.mjs @@ -0,0 +1,88 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { + isCommentOnlyChange, + newestChangelogFork, + splitDiffByFile, + symbolRouteGate, +} from "../release-utils.mjs"; + +const HERE = path.dirname(fileURLToPath(import.meta.url)); +const fixture = (name) => JSON.parse(fs.readFileSync(path.join(HERE, "..", "fixtures", name), "utf8")); + +test("isCommentOnlyChange: base-std#234 (NatSpec-only) is a clarification", () => { + const payload = fixture("code-change-3820cf0-isauthorized-natspec.json"); + assert.equal(isCommentOnlyChange(splitDiffByFile(payload.diff)), true); +}); + +test("isCommentOnlyChange: a code line makes it a real change", () => { + const diff = [ + "diff --git a/src/I.sol b/src/I.sol", + "--- a/src/I.sol", + "+++ b/src/I.sol", + "@@ -1,2 +1,3 @@", + "- /// @notice old", + "+ /// @notice new", + "+ function foo() external;", + ].join("\n"); + assert.equal(isCommentOnlyChange(splitDiffByFile(diff)), false); +}); + +test("isCommentOnlyChange: block comments count, tests are ignored, non-Solidity is not", () => { + const sol = [ + "diff --git a/src/I.sol b/src/I.sol", + "--- a/src/I.sol", + "+++ b/src/I.sol", + "@@ -1 +1,3 @@", + "+/**", + "+ * text", + "+ */", + "diff --git a/test/I.t.sol b/test/I.t.sol", + "--- a/test/I.t.sol", + "+++ b/test/I.t.sol", + "@@ -1 +1 @@", + "+ assertTrue(x);", + ].join("\n"); + assert.equal(isCommentOnlyChange(splitDiffByFile(sol)), true); + const md = ["diff --git a/docs/a.md b/docs/a.md", "--- a/docs/a.md", "+++ b/docs/a.md", "@@ -1 +1 @@", "+// text"].join("\n"); + assert.equal(isCommentOnlyChange(splitDiffByFile(md)), false); + assert.equal(isCommentOnlyChange(new Map()), false); +}); + +const ENTRY_DIR = "docs/base-chain/specs/reference/b20/changelog"; + +test("newestChangelogFork picks the highest-numbered fork", () => { + const pages = [ + `${ENTRY_DIR}/02-cobalt-policyregistry-composite-policy.mdx`, + `${ENTRY_DIR}/03-denim-policyregistry-not-policy.mdx`, + "docs/upgrades/denim/overview.mdx", + ]; + assert.equal(newestChangelogFork(pages, ENTRY_DIR), "denim"); + assert.equal(newestChangelogFork(pages, ""), ""); +}); + +test("symbolRouteGate: comment-only changes route only reference pages", () => { + const opts = { commentOnly: true, newestFork: "denim" }; + assert.equal(symbolRouteGate("docs/x/reference/interfaces/i-policy-registry/is-authorized.mdx", { ...opts, role: "function-reference" }), null); + assert.equal(symbolRouteGate("docs/x/reference/interfaces/i-policy-registry/index.mdx", { ...opts, role: "interface-index" }), null); + for (const role of ["guide", "shared-reference", "changelog-entry"]) { + assert.match(symbolRouteGate("docs/build-on-base/a.mdx", { ...opts, role }), /comment-only/); + } +}); + +test("symbolRouteGate: historical changelog entries and upgrade pages are not routed", () => { + const opts = { commentOnly: false, newestFork: "denim" }; + assert.match( + symbolRouteGate(`${ENTRY_DIR}/02-cobalt-policyregistry-composite-policy.mdx`, { ...opts, role: "changelog-entry" }), + /earlier hardfork \(cobalt/, + ); + assert.equal(symbolRouteGate(`${ENTRY_DIR}/03-denim-policyregistry-not-policy.mdx`, { ...opts, role: "changelog-entry" }), null); + assert.match(symbolRouteGate("docs/upgrades/beryl/b20.mdx", { ...opts, role: "guide" }), /historical upgrade page \(beryl/); + assert.equal(symbolRouteGate("docs/upgrades/denim/overview.mdx", { ...opts, role: "guide" }), null); + assert.equal(symbolRouteGate("docs/build-on-base/a.mdx", { ...opts, role: "guide" }), null); + // No fork data → no fork gating. + assert.equal(symbolRouteGate("docs/upgrades/beryl/b20.mdx", { role: "guide", newestFork: "" }), null); +}); diff --git a/scripts/sync-from-base-std/index.mjs b/scripts/sync-from-base-std/index.mjs index 869c20302..f2b6f3278 100644 --- a/scripts/sync-from-base-std/index.mjs +++ b/scripts/sync-from-base-std/index.mjs @@ -79,6 +79,9 @@ import { firstHeading, decideCall, changedLines, + isCommentOnlyChange, + newestChangelogFork, + symbolRouteGate, } from "./release-utils.mjs"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); @@ -1542,6 +1545,7 @@ async function processPage(item, shared, useGroups) { source_entry: sourceEntry, source_entry_path: sourceEntry ? entrySource : undefined, create, + comment_only: !!shared.commentOnly, }; } const prompt = buildClaudePrompt(kind, ctx); @@ -1754,6 +1758,8 @@ async function main() { // Per-file diff sections so each page only sees the hunks that routed it. const diffByFile = splitDiffByFile(typeof payload.diff === "string" ? payload.diff : ""); + // Set for code-change dispatches whose source diff only edits comments. + let commentOnly = false; let work = []; // What the route table did not handle (code-change only). Rendered into the @@ -1817,6 +1823,11 @@ async function main() { if (content != null) pageContents.set(rel, content); } + const layout = changelogLayout(route); + commentOnly = isCommentOnlyChange(diffByFile); + if (commentOnly) { + console.log("[symbols] source diff changes only comments/NatSpec: a clarification, routed to reference pages only"); + } const symbols = routingSymbols(manifest); if (symbols.length === 0) { console.log("[symbols] no routing symbols (no manifest); path routing only"); @@ -1824,6 +1835,18 @@ async function main() { console.log(`[symbols] ${symbols.length} routing symbol(s): ${symbols.slice(0, 30).join(", ")}${symbols.length > 30 ? ", …" : ""}`); const pages = [...pageContents].map(([p, content]) => ({ path: p, content })); const mentions = findSymbolMentions(pages, symbols); + // Gate pages found only by mention. Path-routed pages are explicit + // route-table decisions and are never dropped here. + const pathRouted = new Set(work.map((w) => w.page)); + const newestFork = newestChangelogFork([...pageContents.keys()], layout.entryDir); + for (const page of [...mentions.keys()]) { + if (pathRouted.has(page)) continue; + const why = symbolRouteGate(page, { role: pageRoleFor(page, layout), commentOnly, newestFork }); + if (why) { + mentions.delete(page); + console.log(`[symbols] not routing ${page} — ${why}`); + } + } const before = work.length; work = mergeSymbolRoutes(work, mentions, manifest); console.log(`[symbols] ${mentions.size} page(s) mention a routing symbol; ${work.length - before} added beyond path routing`); @@ -1834,7 +1857,6 @@ async function main() { // One decision per page, made here and logged with the work list: // does this page need a model call at all? - const layout = changelogLayout(route); for (const w of work) { const content = pageContents.get(w.page); if (content == null) continue; // missing pages are handled (created or skipped) in processPage @@ -1949,6 +1971,7 @@ async function main() { knownRoutes, route, diffByFile, + commentOnly, layout: changelogLayout(route), snippetComponents: await listSnippetComponents(), }; diff --git a/scripts/sync-from-base-std/llm/prompts.mjs b/scripts/sync-from-base-std/llm/prompts.mjs index b7f9274ec..f74d171dc 100644 --- a/scripts/sync-from-base-std/llm/prompts.mjs +++ b/scripts/sync-from-base-std/llm/prompts.mjs @@ -216,6 +216,9 @@ ${lines.join("\n")} */ export function codeChangePrompt(ctx) { const roleLine = ctx.pageRole ? `\n- This page's role: ${ctx.pageRole} (see rule #5 for what this role owns).` : ""; + const changeTypeLine = ctx.comment_only + ? `\n- Change type: the source diff edits only comments/NatSpec. It clarifies existing behavior; no code changed. Edit only statements the clarification shows to be wrong or incomplete (see rule #6 step 2).` + : ""; const createNote = ctx.create ? `\n- THIS PAGE DOES NOT EXIST YET. The block holds only a frontmatter stub. Write the complete page from in the changelog-entry shape from the documentation guidelines. Fill in the frontmatter description (one sentence, value-first). Keep the title unless the source entry's heading is clearer.` : ""; @@ -233,7 +236,7 @@ ${ctx.diff || "(diff omitted — over size limit)"} return `You are editing one page of Base Docs, a Mintlify MDX documentation site. Context: -- A change just landed on ${ctx.source_repo || "base/base-std"}@${ctx.sha}.${roleLine}${createNote} +- A change just landed on ${ctx.source_repo || "base/base-std"}@${ctx.sha}.${roleLine}${changeTypeLine}${createNote} - Changed source files in Base Std (the ones that affect THIS page): ${(ctx.sourceFiles || []).map((s) => ` - ${s}`).join("\n")} ${changeManifestSection(ctx.manifest)} diff --git a/scripts/sync-from-base-std/release-utils.mjs b/scripts/sync-from-base-std/release-utils.mjs index ea03cf3c8..b8e320f52 100644 --- a/scripts/sync-from-base-std/release-utils.mjs +++ b/scripts/sync-from-base-std/release-utils.mjs @@ -638,3 +638,74 @@ export function changedLines(diff) { .filter((l) => (l.startsWith("+") || l.startsWith("-")) && !l.startsWith("+++") && !l.startsWith("---")) .join("\n"); } + +// ------------------------------------------------------- symbol-route gating + +/** + * True when every changed line in the dispatch's source files is a comment + * (Solidity `//`, `///`, or a block-comment line). Such a change clarifies + * documented behavior; it does not change it. Test files are ignored, the + * same as for routing. Any non-Solidity source file, or no changed lines at + * all, returns false so ordinary routing applies. + * + * @param {Map} diffByFile path → unified diff section + */ +export function isCommentOnlyChange(diffByFile) { + let sawChange = false; + for (const [file, section] of diffByFile || []) { + if (file.startsWith("test/")) continue; + if (!file.endsWith(".sol")) return false; + for (const line of changedLines(section).split("\n")) { + const body = line.slice(1).trim(); + if (body === "") continue; + sawChange = true; + if (!/^(\/\/|\/\*|\*)/.test(body)) return false; + } + } + return sawChange; +} + +const CHANGELOG_ENTRY_RE = /\/(\d+)-([a-z]+)-[^/]+\.mdx$/; + +/** + * The newest hardfork with changelog entries, from entry file names + * (`/--.mdx`, highest NN wins). Returns "" when + * the layout has no entries, which disables fork gating. + */ +export function newestChangelogFork(pages, entryDir) { + if (!entryDir) return ""; + const dir = entryDir.replace(/\/?$/, "/"); + let best = { n: -1, fork: "" }; + for (const p of pages || []) { + if (!String(p).startsWith(dir)) continue; + const m = CHANGELOG_ENTRY_RE.exec(p); + if (m && Number(m[1]) > best.n) best = { n: Number(m[1]), fork: m[2] }; + } + return best.fork; +} + +/** + * Decide whether a page found only by symbol mention should be routed. + * Returns null to route, or the reason to drop it. Path-routed pages never + * reach this gate: an explicit route-table rule always wins. + * + * - comment-only source change → only the reference pages that document + * the symbol (function-reference, interface-index) are routed + * - a changelog entry for an earlier hardfork, or an `upgrades//` + * page for any hardfork other than the newest, is a historical record + * and is not edited for a change landing now + */ +export function symbolRouteGate(page, { role, commentOnly = false, newestFork = "" }) { + if (commentOnly && role !== "function-reference" && role !== "interface-index") { + return "comment-only source change; symbol routing limited to reference pages"; + } + if (newestFork) { + if (role === "changelog-entry") { + const m = CHANGELOG_ENTRY_RE.exec(page); + if (m && m[2] !== newestFork) return `changelog entry for an earlier hardfork (${m[2]}; newest is ${newestFork})`; + } + const up = /^docs\/upgrades\/([^/]+)\//.exec(page); + if (up && up[1] !== newestFork) return `historical upgrade page (${up[1]}; newest is ${newestFork})`; + } + return null; +} From 5499b772e3cb6085c8a3bc2921a8128e81ace852 Mon Sep 17 00:00:00 2001 From: sohey Date: Mon, 5 Oct 2026 21:54:39 +0200 Subject: [PATCH 4/6] test(docs-sync): add base-std@1505323 recall case; must_mention and warn 1505323-token-self-recipient replays base-std#232, a real behavior change (transfers, mints, and seizes to the token's own address now revert) that reaches src/ only as NatSpec. It guards the other direction from the 3820cf0 case: the five function references that list InvalidReceiver must change, and their added lines must state the new condition. - must_mention: a regex the added lines of a page must match, so a cosmetic edit to a required page does not count. - unlisted_pages: "warn" reports pages outside the lists without failing. - README: how to run the evals and what each case guards. Co-authored-by: Toshi --- scripts/sync-from-base-std/README.md | 26 ++++++++++ .../__tests__/eval-score.test.mjs | 19 ++++++++ .../cases/1505323-token-self-recipient.json | 48 +++++++++++++++++++ scripts/sync-from-base-std/eval/run-eval.mjs | 22 +++++++-- ...e-change-1505323-token-self-recipient.json | 21 ++++++++ 5 files changed, 132 insertions(+), 4 deletions(-) create mode 100644 scripts/sync-from-base-std/eval/cases/1505323-token-self-recipient.json create mode 100644 scripts/sync-from-base-std/fixtures/code-change-1505323-token-self-recipient.json diff --git a/scripts/sync-from-base-std/README.md b/scripts/sync-from-base-std/README.md index b4213dfcc..9bdd10540 100644 --- a/scripts/sync-from-base-std/README.md +++ b/scripts/sync-from-base-std/README.md @@ -101,6 +101,32 @@ Configuration knobs are optional positive numbers: - `CLAUDE_MAX_TOKENS` and `CLAUDE_MODEL` - The bounded release manifest/selection settings documented in `index.mjs` +## Evals + +`eval/run-eval.mjs` replays recorded dispatches against a pinned docs commit +in a throwaway worktree and scores which pages changed. Run it before and +after any change to prompts, routing, or the route table: + +```bash +LLM_GATEWAY_API_KEY=... node scripts/sync-from-base-std/eval/run-eval.mjs --runs 3 +# baseline: the same cases with the sync code from master +LLM_GATEWAY_API_KEY=... node scripts/sync-from-base-std/eval/run-eval.mjs --runs 3 --code-ref master +``` + +Each case in `eval/cases/` names a payload fixture, a `docs_base_sha`, and +expectations: `must_touch`, `must_mention` (a regex the added lines of a +required page must match), `may_touch` (page → added-line budget), +`must_not_touch`, `unlisted_pages` (`fail` or `warn`), and an optional +`restatement` check. Output goes to `.sync-eval/` (gitignored). + +| Case | What it guards | +| --- | --- | +| `3820cf0-isauthorized-natspec` | Precision: a NatSpec-only clarification edits only the owning reference page and its index row. | +| `1505323-token-self-recipient` | Recall: a real behavior change expressed as NatSpec plus mocks still reaches every function reference that documents it. | + +Model output varies, so compare several runs, and read the diffs: the +scores check scope, not wording. + ## Source PR attribution For `code-change` dispatches the workflow @mentions the source PR author in diff --git a/scripts/sync-from-base-std/__tests__/eval-score.test.mjs b/scripts/sync-from-base-std/__tests__/eval-score.test.mjs index 69991e483..82ec11b22 100644 --- a/scripts/sync-from-base-std/__tests__/eval-score.test.mjs +++ b/scripts/sync-from-base-std/__tests__/eval-score.test.mjs @@ -42,3 +42,22 @@ test("scoreRun flags missing, forbidden, oversized, unlisted, and restated edits assert.match(text, /unlisted page: docs\/other\.mdx/); assert.match(text, /restates the full rules outside their owner pages: docs\/guide\/seize\.mdx/); }); + +test("scoreRun: must_mention fails a required edit that does not state the change", () => { + const expect = { + must_touch: ["a.mdx"], + must_mention: { "a.mdx": "address\\(this\\)" }, + }; + const numstat = { "a.mdx": { added: 1, removed: 1 } }; + const cosmetic = scoreRun(expect, numstat, { "a.mdx": ["Reverts when `to` is zero."] }); + assert.equal(cosmetic.pass, false); + assert.match(cosmetic.violations[0], /does not state the change/); + const real = scoreRun(expect, numstat, { "a.mdx": ["Reverts when `to == address(this)`."] }); + assert.equal(real.pass, true); +}); + +test("scoreRun: unlisted_pages warn reports without failing", () => { + const r = scoreRun({ must_touch: [], unlisted_pages: "warn" }, { "x.mdx": { added: 2, removed: 0 } }); + assert.equal(r.pass, true); + assert.equal(r.warnings.length, 1); +}); diff --git a/scripts/sync-from-base-std/eval/cases/1505323-token-self-recipient.json b/scripts/sync-from-base-std/eval/cases/1505323-token-self-recipient.json new file mode 100644 index 000000000..50a3f7082 --- /dev/null +++ b/scripts/sync-from-base-std/eval/cases/1505323-token-self-recipient.json @@ -0,0 +1,48 @@ +{ + "description": "base-std#232 (1505323): a real behavior change. Transfers, mints, batch mints, and seizes now revert with InvalidReceiver when the recipient is the token's own address. In Base Std the interface edits are NatSpec only; the mocks and a changelog entry carry the change. Pinned to a docs commit where humans had already added the changelog entry and updated both seize guides, but the five function reference pages still list only address(0) (errors.mdx already covers it as a B20-prefix address). This case guards recall: the sync must still edit the owning reference pages for a real change. announce-a-distribution and issue-units list batchMint's revert reasons, so a one-line addition there is correct.", + "payload": "../../fixtures/code-change-1505323-token-self-recipient.json", + "docs_base_sha": "1169505620155cc1186cf3ce4ed56320de137c0b", + "expect": { + "must_touch": [ + "docs/specifications/b20/reference/interfaces/ib20/transfer.mdx", + "docs/specifications/b20/reference/interfaces/ib20/transfer-from.mdx", + "docs/specifications/b20/reference/interfaces/ib20/mint.mdx", + "docs/specifications/b20/reference/interfaces/ib20/seize-with-memo.mdx", + "docs/specifications/b20/reference/interfaces/ib20-asset/batch-mint.mdx" + ], + "must_mention": { + "docs/specifications/b20/reference/interfaces/ib20/transfer.mdx": "address\\(this\\)|own address|token itself|token's address", + "docs/specifications/b20/reference/interfaces/ib20/transfer-from.mdx": "address\\(this\\)|own address|token itself|token's address", + "docs/specifications/b20/reference/interfaces/ib20/mint.mdx": "address\\(this\\)|own address|token itself|token's address", + "docs/specifications/b20/reference/interfaces/ib20/seize-with-memo.mdx": "address\\(this\\)|own address|token itself|token's address", + "docs/specifications/b20/reference/interfaces/ib20-asset/batch-mint.mdx": "address\\(this\\)|own address|token itself|token's address" + }, + "may_touch": { + "docs/specifications/b20/reference/interfaces/ib20/index.mdx": 6, + "docs/specifications/b20/reference/interfaces/ib20-asset/index.mdx": 4, + "docs/specifications/b20/reference/interfaces.mdx": 4, + "docs/specifications/b20/reference/events.mdx": 2, + "docs/base-chain/specs/reference/b20/changelog/03-denim-b20-token-receiver.mdx": 6, + "docs/build-on-base/issue-rwa/seize-and-cancel-units.mdx": 4, + "docs/build-on-base/issue-stablecoins/recover-funds.mdx": 4, + "docs/specifications/b20/changelog.mdx": 4, + "docs/docs.json": 4, + "docs/specifications/b20/reference/errors.mdx": 2, + "docs/build-on-base/issue-rwa/announce-a-distribution.mdx": 2, + "docs/build-on-base/issue-rwa/issue-units.mdx": 2 + }, + "must_not_touch": [ + "docs/build-on-base/accept-payments/request-a-payment.mdx", + "docs/build-on-base/accept-payments/send-a-payout.mdx", + "docs/build-on-base/accept-payments/split-a-payment.mdx", + "docs/build-on-base/accept-payments/verify-a-payment.mdx", + "docs/build-on-base/issue-rwa/create-an-asset-token.mdx", + "docs/build-on-base/issue-rwa/pause-transfers.mdx", + "docs/build-on-base/issue-stablecoins/issue-your-stablecoin.mdx", + "docs/build-on-base/issue-stablecoins/restrict-who-can-hold.mdx", + "docs/base-chain/specs/reference/b20/changelog/02-cobalt-b20-seize.mdx", + "docs/base-chain/specs/reference/b20/changelog/02-cobalt-b20asset-multiplier.mdx" + ], + "unlisted_pages": "warn" + } +} diff --git a/scripts/sync-from-base-std/eval/run-eval.mjs b/scripts/sync-from-base-std/eval/run-eval.mjs index 723d0086f..4aecffa83 100644 --- a/scripts/sync-from-base-std/eval/run-eval.mjs +++ b/scripts/sync-from-base-std/eval/run-eval.mjs @@ -67,10 +67,11 @@ export function addedLinesByFile(diff) { /** * Score one run against a case's expectations. - * @returns {{pass: boolean, violations: string[], restated: string[]}} + * @returns {{pass: boolean, violations: string[], warnings: string[], restated: string[]}} */ export function scoreRun(expect, numstat, addedByFile = {}) { const violations = []; + const warnings = []; const touched = Object.keys(numstat); const may = expect.may_touch || {}; const mustNot = new Set(expect.must_not_touch || []); @@ -79,6 +80,15 @@ export function scoreRun(expect, numstat, addedByFile = {}) { for (const p of must) { if (!numstat[p]) violations.push(`missing required edit: ${p}`); } + // must_mention: { page: regex } — the page's added lines must state the + // new fact, so a cosmetic edit to a required page does not count. + for (const [p, pattern] of Object.entries(expect.must_mention || {})) { + if (!numstat[p]) continue; // already reported as missing above, if required + const text = (addedByFile[p] || []).join("\n"); + if (!new RegExp(pattern, "i").test(text)) { + violations.push(`edit does not state the change: ${p} (no added line matches /${pattern}/i)`); + } + } for (const p of touched) { if (mustNot.has(p)) { violations.push(`edited a page it should leave alone: ${p} (+${numstat[p].added} -${numstat[p].removed})`); @@ -86,8 +96,11 @@ export function scoreRun(expect, numstat, addedByFile = {}) { if (numstat[p].added > may[p]) { violations.push(`edit too large: ${p} adds ${numstat[p].added} lines (budget ${may[p]})`); } - } else if (!must.includes(p) && (expect.unlisted_pages || "fail") === "fail") { - violations.push(`edited an unlisted page: ${p} (+${numstat[p].added} -${numstat[p].removed})`); + } else if (!must.includes(p)) { + const mode = expect.unlisted_pages || "fail"; + const msg = `edited an unlisted page: ${p} (+${numstat[p].added} -${numstat[p].removed})`; + if (mode === "fail") violations.push(msg); + else if (mode === "warn") warnings.push(msg); } } @@ -103,7 +116,7 @@ export function scoreRun(expect, numstat, addedByFile = {}) { } for (const p of restated) violations.push(`restates the full rules outside their owner pages: ${p}`); } - return { pass: violations.length === 0, violations, restated }; + return { pass: violations.length === 0, violations, warnings, restated }; } // ------------------------------------------------------------------ running @@ -226,6 +239,7 @@ function printSummary(results) { console.log(`\n${r.pass ? "PASS" : "FAIL"} ${r.case} run ${r.run} — ${Object.keys(r.numstat).length} page(s)`); if (files) console.log(files); for (const v of r.violations) console.log(` ✗ ${v}`); + for (const w of r.warnings || []) console.log(` ! ${w}`); } const passed = results.filter((r) => r.pass).length; console.log(`\n${passed}/${results.length} run(s) passed`); diff --git a/scripts/sync-from-base-std/fixtures/code-change-1505323-token-self-recipient.json b/scripts/sync-from-base-std/fixtures/code-change-1505323-token-self-recipient.json new file mode 100644 index 000000000..2bbf5285f --- /dev/null +++ b/scripts/sync-from-base-std/fixtures/code-change-1505323-token-self-recipient.json @@ -0,0 +1,21 @@ +{ + "changed_paths": [ + "changelog/03_Denim_B20_token_receiver.md", + "changelog/README.md", + "docs/guides/seizeing-assets.md", + "docs/reference/errors.md", + "src/interfaces/IB20.sol", + "src/interfaces/IB20Asset.sol", + "test/lib/mocks/MockB20.sol", + "test/lib/mocks/MockB20Asset.sol", + "test/lib/mocks/MockB20Factory.sol" + ], + "diff": "diff --git a/changelog/03_Denim_B20_token_receiver.md b/changelog/03_Denim_B20_token_receiver.md\nnew file mode 100644\nindex 0000000..b0adfa4\n--- /dev/null\n+++ b/changelog/03_Denim_B20_token_receiver.md\n@@ -0,0 +1,122 @@\n+# Reject the Token Itself as a Credit Recipient\n+\n+- **Feature Name**: token_receiver\n+- **Start Date**: 2026-09-17\n+- **Authors**: Rayyan Alam\n+- **Title**: (Breaking) Reject the Token Itself as a Credit Recipient\n+\n+## Summary\n+\n+Denim rejects a send whose destination is this token's own address. That destination cannot spend the credited tokens, so the send would lock them.\n+\n+`transfer`, `transferFrom`, their memo variants, `mint`, `mintWithMemo`, `batchMint`, and `seizeWithMemo` revert `InvalidReceiver(to)` when `to` is the token. A holder sending to themselves (`from == to`) still succeeds. An issuer can still recover tokens already credited to the B20 token contract: `seizeWithMemo` from the token address succeeds.\n+\n+## Motivation\n+\n+It is common for users to mistakenly send tokens to the token address instead of their intended recipient's address. In the case of B20 tokens, this makes the funds inaccessible without intervention from the token admin.\n+\n+A B20 token is a precompile. It has no holder key and cannot call `transfer` on itself. After a transfer lands at the token address, the sender cannot recover those tokens. Only the issuer can, by calling `seizeWithMemo`.\n+\n+There is no valid use case for a B20 token address to hold its own tokens. Denim therefore reverts `InvalidReceiver(to)` on that destination so the accidental send fails instead of locking the funds.\n+\n+## Background\n+\n+B20 is a native token precompile. The token address has no holder key and cannot initiate calls. A credit to that address is not spender-recoverable by the sender.\n+\n+`InvalidReceiver(address receiver)` already fires for `address(0)` (ERC-6093). This change adds `address(this)` as a second trigger of the same error.\n+\n+## Specs\n+\n+### Interface Changes\n+\n+This change adds no new functions, events, errors, or selectors. `InvalidReceiver(address receiver)` already exists. Its documented triggers now include the token's own address.\n+\n+### Behavioural Changes\n+\n+A shared valid-receiver check runs at the same position as the existing zero-receiver guard:\n+\n+```solidity\n+if (to == address(0) || to == address(this)) revert InvalidReceiver(to);\n+```\n+\n+Canonical order is unchanged. `address(0)` and `address(this)` are two triggers of the same invalid-receiver step.\n+\n+\n+| Function | Check order |\n+| --------------------------------------- | -------------------------------------------------------------------------------------------------------------------------- |\n+| `transfer` / `transferWithMemo` | pause → **invalid-receiver** → zero-sender → executor policy → sender policy → receiver policy → balance |\n+| `transferFrom` / `transferFromWithMemo` | pause → **invalid-receiver** → zero-sender → allowance → executor policy → sender policy → receiver policy → balance |\n+| `mint` / `mintWithMemo` | pause → role → **invalid-receiver** → mint-receiver policy → supply cap |\n+| `batchMint` | pause → role → length / empty → per-element **invalid-receiver** → `_mint` body |\n+| `seizeWithMemo` | pause → role → **invalid-receiver** → zero-sender → self-seize (`from == to`) → seizable → seize-receiver policy → balance |\n+\n+\n+`from` may equal `address(this)`. A seize that drains the token into a treasury still succeeds.\n+\n+### Examples\n+\n+A holder transfer to this token reverts:\n+\n+```solidity\n+vm.prank(alice);\n+token.transfer({to: address(token), amount: uint256(amount)}); // reverts InvalidReceiver(address(token))\n+```\n+\n+Mint and seize to the token address revert the same way:\n+\n+```solidity\n+token.mint({to: address(token), amount: uint256(amount)}); // reverts InvalidReceiver(address(token))\n+token.seizeWithMemo({\n+ from: address(alice), to: address(token), amount: uint256(amount), memo: bytes32(memo)\n+}); // reverts InvalidReceiver(address(token))\n+```\n+\n+A holder sending to themselves still succeeds:\n+\n+```solidity\n+vm.prank(alice);\n+token.transfer({to: address(alice), amount: uint256(amount)}); // succeeds; balance and totalSupply unchanged\n+```\n+\n+Recovery of a pre-activation stuck balance still succeeds:\n+\n+```solidity\n+token.seizeWithMemo({\n+ from: address(token), to: address(treasury), amount: uint256(amount), memo: bytes32(memo)\n+}); // succeeds\n+```\n+\n+## Design Decisions & Alternatives Considered\n+\n+### Chosen: reuse `InvalidReceiver`, reject `to == address(this)` on every credit path\n+\n+The destination is invalid for the same reason `address(0)` is invalid: a holder cannot spend the credited units. Reusing `InvalidReceiver` avoids a new selector. Wallets that already treat that error as \"do not send here\" keep the same revert handling.\n+\n+The check compares against `address(this)` — one word, no external call, matches the reported paste-error footgun exactly. Mint, seize, and `batchMint` are included because they write the same `balances[to]` slot.\n+\n+The check lives next to the existing zero-receiver guard, not inside `_moveBalance`. `_moveBalance` is an unguarded mechanic. Callers already apply their own input checks.\n+\n+### Alternative — reject any B20-prefix address as recipient\n+\n+That would also revert when `to` is a different B20-prefix address (token A → token B). It was rejected. A prefix check cannot tell a B20 token from a user-controlled account in that address space, such as a multisig. Rejecting the whole prefix would revert valid transfers to those recipients.\n+\n+Denim therefore compares `to` against `address(this)` only. Sends to other addresses, including other B20 tokens, still succeed.\n+\n+### Alternative — call `isB20Initialized(to)`\n+\n+That would reject only live tokens. It was rejected because it adds a factory call on every credit path and does not fit the paste-error framing.\n+\n+### Alternative — new error such as `SelfSend(address)`\n+\n+A dedicated error would make the case obvious in traces. It was rejected because it adds ABI surface for a condition that is already \"this destination is invalid\".\n+\n+### Alternative — also reject `from == address(this)`\n+\n+Blocking spends from the token address would close the only recovery path for balances already sitting there. It was rejected.\n+\n+## Migration Steps\n+\n+1. Treat this token's own address as an invalid recipient, the same way you already treat `address(0)`. This applies to wallets, custodians, and indexers.\n+2. After Denim activation, expect `InvalidReceiver` from a transfer, mint, or seize to `address(token)` that succeeded before Denim.\n+3. If a balance is already credited to the token address from before activation, recover it with `seizeWithMemo(address(token), treasury, amount, memo)`. The token must be seizable under `SEIZE_EXEMPT_POLICY`. The caller must hold `SEIZE_ROLE`.\n+4. Do not change holder-to-holder self-transfers, approvals, burns, or sends to other B20 tokens. This change does not affect them.\ndiff --git a/changelog/README.md b/changelog/README.md\nindex 351a6e8..df96d26 100644\n--- a/changelog/README.md\n+++ b/changelog/README.md\n@@ -27,6 +27,7 @@ Grouped by hardfork, one collapsible section per hardfork, newest first.\n \n | Product(s) | Change | Affected interfaces | Entry |\n | --- | --- | --- | --- |\n+| B20 | Reject the token itself as a credit recipient | `src/interfaces/IB20.sol`, `src/interfaces/IB20Asset.sol` | [03_Denim_B20_token_receiver](03_Denim_B20_token_receiver.md) |\n | B20 | Transfer executor policy on every transfer path | `src/interfaces/IB20.sol` | [03_Denim_B20_transfer_executor_enforcement](03_Denim_B20_transfer_executor_enforcement.md) |\n | PolicyRegistry | NOT / invert policies | `src/interfaces/IPolicyRegistry.sol` | [03_Denim_PolicyRegistry_not_policy](03_Denim_PolicyRegistry_not_policy.md) |\n \ndiff --git a/docs/guides/seizeing-assets.md b/docs/guides/seizeing-assets.md\nindex 5e150fa..9aa2578 100644\n--- a/docs/guides/seizeing-assets.md\n+++ b/docs/guides/seizeing-assets.md\n@@ -22,7 +22,7 @@ You need all of the following:\n - A B20 token you administer.\n - `DEFAULT_ADMIN_ROLE` on that token, so you can grant roles and attach policies.\n - An account that will call `seizeWithMemo` (the seizer).\n-- A non-zero destination that is not the holder (typically a treasury).\n+- A non-zero destination that is not the holder and not this token's own address (typically a treasury).\n - `SEIZE` not paused. `pause([SEIZE])` blocks every seize until `unpause([SEIZE])`.\n \n Three independent controls then decide whether a seize can run. In this order, the steps later configure them in the same order.\n@@ -269,7 +269,7 @@ These errors follow the order `seizeWithMemo` checks them.\n | ------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------- |\n | `ContractPaused(SEIZE)` | `SEIZE` is paused. | Call `unpause` with `PausableFeature.SEIZE`. |\n | `AccessControlUnauthorizedAccount(caller, SEIZE_ROLE)` | The caller does not hold `SEIZE_ROLE`. | Grant `SEIZE_ROLE` to the seizer. |\n-| `InvalidReceiver(to)` | `to` is `address(0)`, or `from == to`. | Use a distinct, non-zero safekeeping address. |\n+| `InvalidReceiver(to)` | `to` is `address(0)`, this token's own address, or `from == to`. | Use a distinct, non-zero safekeeping address that is not this token itself. |\n | `InvalidSender(from)` | `from` is `address(0)`. | Pass the holder's address. |\n | `AccountNotSeizable(from)` | `from` is still authorized under `SEIZE_HOLDER_POLICY`. The slot is unset, or the holder is not on the attached blocklist. | Attach a blocklist and add `from`. |\n | `PolicyForbids(SEIZE_RECEIVER_POLICY, policyId)` | `to` is not authorized under `SEIZE_RECEIVER_POLICY`. | Add `to` to the receiver allowlist, or set the scope back to `0` (`ALWAYS_ALLOW`). |\n@@ -277,6 +277,8 @@ These errors follow the order `seizeWithMemo` checks them.\n | `PolicyNotFound(policyId)` | `updatePolicy` received an ID that is not a sentinel and does not exist in the registry. | Create the policy first, then attach the returned ID. |\n | `Unauthorized()` | A non-admin called `updateBlocklist` or `updateAllowlist`. | Call as the policy's `policyAdmin`. |\n \n+A balance already sitting at this token's address can still be recovered. `seizeWithMemo(address(token), treasury, amount, memo)` is allowed; seizing *to* `address(token)` is not.\n+\n \n ```mermaid\n flowchart TD\ndiff --git a/docs/reference/errors.md b/docs/reference/errors.md\nindex 08a2c79..f8f3d4d 100644\n--- a/docs/reference/errors.md\n+++ b/docs/reference/errors.md\n@@ -15,7 +15,7 @@\n | `InsufficientAllowance(address spender, uint256 allowance, uint256 needed)` | `0x192b9e4e` | `spender`'s allowance is less than `needed` for the requested `transferFrom`. |\n | `InsufficientBalance(address sender, uint256 balance, uint256 needed)` | `0xdb42144d` | `sender`'s balance is less than `needed` for the requested transfer or burn. |\n | `InvalidSender(address sender)` | `0x4c14f64c` | The transfer's source address is invalid (typically `address(0)`). |\n-| `InvalidReceiver(address receiver)` | `0x9cfea583` | The transfer's destination address is invalid (typically `address(0)`). |\n+| `InvalidReceiver(address receiver)` | `0x9cfea583` | The transfer's destination address is invalid (`address(0)` or the token's own address). |\n | `InvalidApprover(address approver)` | `0x8bc146c4` | The approval's `owner` address is invalid (typically `address(0)`). |\n | `InvalidSpender(address spender)` | `0x4e15efda` | The approval's `spender` address is invalid (typically `address(0)`). |\n | `InvalidAmount()` | `0x2c5211c6` | An amount argument was zero where a non-zero value is required. Not used for ERC-20 amount arguments. |\ndiff --git a/src/interfaces/IB20.sol b/src/interfaces/IB20.sol\nindex ca0ab6b..216bfcb 100644\n--- a/src/interfaces/IB20.sol\n+++ b/src/interfaces/IB20.sol\n@@ -65,7 +65,9 @@ interface IB20 {\n /// @notice The transfer's source address is invalid (typically `address(0)`).\n error InvalidSender(address sender);\n \n- /// @notice The transfer's destination address is invalid (typically `address(0)`).\n+ /// @notice The transfer's destination address is invalid. Fires for `address(0)` and for the\n+ /// token's own address (`to == address(this)`), which would otherwise lock the credited\n+ /// balance with no holder-side way to move it.\n error InvalidReceiver(address receiver);\n \n /// @notice The approval's `owner` address is invalid (typically `address(0)`).\n@@ -315,7 +317,7 @@ interface IB20 {\n /// @notice Transfers `amount` from `msg.sender` to `to`. Emits `Transfer`.\n ///\n /// @dev Reverts with `ContractPaused(TRANSFER)` when `TRANSFER` is paused.\n- /// @dev Reverts with `InvalidReceiver` when `to == address(0)`.\n+ /// @dev Reverts with `InvalidReceiver` when `to == address(0)` or `to == address(this)`.\n /// @dev Reverts with `InvalidSender` when `msg.sender == address(0)`.\n /// @dev Reverts with `PolicyForbids(TRANSFER_EXECUTOR_POLICY, ...)` when `msg.sender` is not authorized.\n /// @dev Reverts with `PolicyForbids(TRANSFER_SENDER_POLICY, ...)` when `msg.sender` is not authorized.\n@@ -333,7 +335,7 @@ interface IB20 {\n /// @notice Transfers `amount` from `from` to `to` using `msg.sender`'s allowance. Emits `Transfer`.\n ///\n /// @dev Reverts with `ContractPaused(TRANSFER)` when `TRANSFER` is paused.\n- /// @dev Reverts with `InvalidReceiver` when `to == address(0)`.\n+ /// @dev Reverts with `InvalidReceiver` when `to == address(0)` or `to == address(this)`.\n /// @dev Reverts with `InvalidSender` when `from == address(0)`.\n /// @dev Reverts with `InsufficientAllowance` when the caller's allowance from `from` is below `amount`.\n /// @dev Reverts with `PolicyForbids(TRANSFER_EXECUTOR_POLICY, ...)` when `msg.sender` is not authorized.\n@@ -412,7 +414,7 @@ interface IB20 {\n ///\n /// @dev Reverts with `ContractPaused(MINT)` when `MINT` is paused.\n /// @dev Reverts with `AccessControlUnauthorizedAccount` when the caller does not hold `MINT_ROLE`.\n- /// @dev Reverts with `InvalidReceiver` when `to == address(0)`.\n+ /// @dev Reverts with `InvalidReceiver` when `to == address(0)` or `to == address(this)`.\n /// @dev Reverts with `PolicyForbids(MINT_RECEIVER_POLICY, ...)` when `to` is not authorized.\n /// @dev Reverts with `SupplyCapExceeded` when `totalSupply + amount > supplyCap`.\n ///\n@@ -471,7 +473,9 @@ interface IB20 {\n /// unconfigured token may seize to any destination (a treasury need not be allowlisted).\n /// @dev Reverts with `ContractPaused(SEIZE)` when `SEIZE` is paused.\n /// @dev Reverts with `AccessControlUnauthorizedAccount` when the caller does not hold `SEIZE_ROLE`.\n- /// @dev Reverts with `InvalidReceiver` when `to == address(0)` or `from == to`.\n+ /// @dev Reverts with `InvalidReceiver` when `to == address(0)`, `to == address(this)`,\n+ /// or `from == to`. `from` may equal `address(this)` so a balance already stuck at\n+ /// the token can be recovered to a treasury.\n /// @dev Reverts with `InvalidSender` when `from == address(0)`.\n /// @dev Reverts with `AccountNotSeizable` when `from` is authorized under `SEIZE_EXEMPT_POLICY`.\n /// @dev Reverts with `PolicyForbids(SEIZE_RECEIVER_POLICY, ...)` when `to` is not authorized under `SEIZE_RECEIVER_POLICY`.\ndiff --git a/src/interfaces/IB20Asset.sol b/src/interfaces/IB20Asset.sol\nindex 6afd4bd..f1a4095 100644\n--- a/src/interfaces/IB20Asset.sol\n+++ b/src/interfaces/IB20Asset.sol\n@@ -254,7 +254,8 @@ interface IB20Asset is\n /// @dev Reverts with `AccessControlUnauthorizedAccount` when the caller does not hold `MINT_ROLE`.\n /// @dev Reverts with `LengthMismatch` when `recipients.length != amounts.length`.\n /// @dev Reverts with `EmptyBatch` when either array is empty.\n- /// @dev Reverts with `InvalidReceiver` when any `recipients[i] == address(0)`.\n+ /// @dev Reverts with `InvalidReceiver` when any `recipients[i]` is `address(0)` or equals\n+ /// `address(this)`.\n /// @dev Reverts with `PolicyForbids(MINT_RECEIVER_POLICY, ...)` when any recipient is not authorized.\n /// @dev Reverts with `SupplyCapExceeded` when the cumulative mint would exceed the cap.\n ///\ndiff --git a/test/lib/mocks/MockB20.sol b/test/lib/mocks/MockB20.sol\nindex 0d581cf..1844473 100644\n--- a/test/lib/mocks/MockB20.sol\n+++ b/test/lib/mocks/MockB20.sol\n@@ -185,7 +185,8 @@ abstract contract MockB20 is IB20 {\n // ============================================================\n \n function transfer(address to, uint256 amount) external whenNotPaused(PausableFeature.TRANSFER) returns (bool) {\n- _requireNonZeroActors(msg.sender, to);\n+ if (_isContractAddressOrZero(to)) revert InvalidReceiver(to);\n+ if (msg.sender == address(0)) revert InvalidSender(msg.sender);\n _transfer(msg.sender, to, amount);\n return true;\n }\n@@ -195,7 +196,8 @@ abstract contract MockB20 is IB20 {\n whenNotPaused(PausableFeature.TRANSFER)\n returns (bool)\n {\n- _requireNonZeroActors(from, to);\n+ if (_isContractAddressOrZero(to)) revert InvalidReceiver(to);\n+ if (from == address(0)) revert InvalidSender(from);\n // Allowance is consumed unconditionally — including during the factory\n // bootstrap window (`_isPrivileged()`). Matches the Rust precompile,\n // which carves no `privileged` exception for allowance accounting. An\n@@ -224,7 +226,8 @@ abstract contract MockB20 is IB20 {\n whenNotPaused(PausableFeature.TRANSFER)\n returns (bool)\n {\n- _requireNonZeroActors(msg.sender, to);\n+ if (_isContractAddressOrZero(to)) revert InvalidReceiver(to);\n+ if (msg.sender == address(0)) revert InvalidSender(msg.sender);\n _transfer(msg.sender, to, amount);\n emit Memo(msg.sender, memo);\n return true;\n@@ -235,7 +238,8 @@ abstract contract MockB20 is IB20 {\n whenNotPaused(PausableFeature.TRANSFER)\n returns (bool)\n {\n- _requireNonZeroActors(from, to);\n+ if (_isContractAddressOrZero(to)) revert InvalidReceiver(to);\n+ if (from == address(0)) revert InvalidSender(from);\n // Allowance is consumed unconditionally — including during the factory\n // bootstrap window — matching the Rust precompile. Infinite allowance\n // is still not decremented. The executor policy is enforced centrally\n@@ -266,7 +270,7 @@ abstract contract MockB20 is IB20 {\n // ============================================================\n \n function mint(address to, uint256 amount) external whenNotPaused(PausableFeature.MINT) onlyRole(MINT_ROLE) {\n- if (to == address(0)) revert InvalidReceiver(to);\n+ if (_isContractAddressOrZero(to)) revert InvalidReceiver(to);\n _mint(to, amount);\n }\n \n@@ -275,7 +279,7 @@ abstract contract MockB20 is IB20 {\n whenNotPaused(PausableFeature.MINT)\n onlyRole(MINT_ROLE)\n {\n- if (to == address(0)) revert InvalidReceiver(to);\n+ if (_isContractAddressOrZero(to)) revert InvalidReceiver(to);\n _mint(to, amount);\n emit Memo(msg.sender, memo);\n }\n@@ -315,10 +319,11 @@ abstract contract MockB20 is IB20 {\n \n /// @notice Seizes `amount` of `from`'s balance and reassigns it to `to` in a single admin operation,\n /// emitting `Transfer`, `Memo`, then `Seized` (in that order).\n- /// @dev Admin op: skips transfer policies and allowance. Reverts `InvalidReceiver` when `to == 0`\n- /// or `from == to`, and `InvalidSender` when `from == 0`. `from` must be unauthorized under\n- /// `SEIZE_EXEMPT_POLICY`; `to` must be authorized under `SEIZE_RECEIVER_POLICY` (mirrors\n- /// `MINT_RECEIVER_POLICY`: unset slot = always-allow).\n+ /// @dev Admin op: skips transfer policies and allowance. Reverts `InvalidReceiver` when `to == 0`,\n+ /// `to == address(this)`, or `from == to`, and `InvalidSender` when `from == 0`.\n+ /// `from` must be unauthorized under `SEIZE_EXEMPT_POLICY`; `to` must be authorized under\n+ /// `SEIZE_RECEIVER_POLICY` (mirrors `MINT_RECEIVER_POLICY`: unset slot = always-allow).\n+ /// `from` may equal `address(this)` so a balance already stuck at the token can be recovered.\n /// @param from Account whose balance is being seized.\n /// @param to Destination address for the seized balance.\n /// @param amount Amount to seize.\n@@ -328,7 +333,7 @@ abstract contract MockB20 is IB20 {\n whenNotPaused(PausableFeature.SEIZE)\n onlyRole(SEIZE_ROLE)\n {\n- if (to == address(0)) revert InvalidReceiver(to);\n+ if (_isContractAddressOrZero(to)) revert InvalidReceiver(to);\n if (from == address(0)) revert InvalidSender(from);\n if (from == to) revert InvalidReceiver(to);\n _requireSeizable(from);\n@@ -722,13 +727,15 @@ abstract contract MockB20 is IB20 {\n }\n }\n \n- /// @dev Receiver-then-sender zero-address check shared by every\n- /// transfer-family entrypoint. Reverts `InvalidReceiver(to)`\n- /// before `InvalidSender(from)` so the precedence between the\n- /// two matches the canonical order.\n- function _requireNonZeroActors(address from, address to) internal pure {\n- if (to == address(0)) revert InvalidReceiver(to);\n- if (from == address(0)) revert InvalidSender(from);\n+ /// @dev True iff `account` is `address(0)` or this token's own address.\n+ /// Crediting either would lock the balance: the precompile cannot\n+ /// call `transfer` on its own behalf, and `address(0)` is the\n+ /// ERC-6093 invalid-receiver sentinel. Callers revert\n+ /// `InvalidReceiver(to)` when this returns true. Seize *from*\n+ /// `address(this)` remains allowed so a balance already stuck at\n+ /// the token can be recovered.\n+ function _isContractAddressOrZero(address account) internal view returns (bool) {\n+ return account == address(0) || account == address(this);\n }\n \n /// @dev Pure mechanics: policy (with bootstrap bypass) + balance +\n@@ -809,7 +816,7 @@ abstract contract MockB20 is IB20 {\n }\n \n /// @dev Pure mechanics: policy + supply cap + effects. Pause, role,\n- /// and the zero-receiver check are enforced upstream by `mint`\n+ /// and the valid-receiver check are enforced upstream by `mint`\n /// / `mintWithMemo`. The asset variant's `batchMint` carries\n /// the same `whenNotPaused` + `onlyRole` modifiers ONCE for the\n /// whole batch and validates per-element receivers inline\ndiff --git a/test/lib/mocks/MockB20Asset.sol b/test/lib/mocks/MockB20Asset.sol\nindex 4902e09..3ddc488 100644\n--- a/test/lib/mocks/MockB20Asset.sol\n+++ b/test/lib/mocks/MockB20Asset.sol\n@@ -260,7 +260,7 @@ contract MockB20Asset is MockB20, IB20Asset {\n // ============================================================\n \n /// @dev Pause + role enforced ONCE for the entire batch via the\n- /// entrypoint modifiers. Per-element zero-receiver guard is\n+ /// entrypoint modifiers. Per-element valid-receiver guard is\n /// inlined in the loop since `_mint` no longer carries an\n /// input check.\n function batchMint(address[] calldata recipients, uint256[] calldata amounts)\n@@ -271,7 +271,7 @@ contract MockB20Asset is MockB20, IB20Asset {\n if (recipients.length != amounts.length) revert LengthMismatch(recipients.length, amounts.length);\n if (recipients.length == 0) revert EmptyBatch();\n for (uint256 i = 0; i < recipients.length; i++) {\n- if (recipients[i] == address(0)) revert InvalidReceiver(recipients[i]);\n+ if (_isContractAddressOrZero(recipients[i])) revert InvalidReceiver(recipients[i]);\n _mint(recipients[i], amounts[i]);\n }\n }\ndiff --git a/test/lib/mocks/MockB20Factory.sol b/test/lib/mocks/MockB20Factory.sol\nindex f8000a7..edbed18 100644\n--- a/test/lib/mocks/MockB20Factory.sol\n+++ b/test/lib/mocks/MockB20Factory.sol\n@@ -255,12 +255,12 @@ contract MockB20Factory is IB20Factory {\n \n /// @inheritdoc IB20Factory\n function isB20(address token) external pure returns (bool) {\n- return _isB20Prefix(token);\n+ return _hasB20Prefix(token);\n }\n \n /// @inheritdoc IB20Factory\n function isB20Initialized(address token) external view returns (bool) {\n- if (!_isB20Prefix(token)) return false;\n+ if (!_hasB20Prefix(token)) return false;\n // Same dedicated slot the factory flips at the end of createToken\n // (MockB20Storage.INITIALIZED_OFFSET). The slot holds nothing\n // else, so any non-zero word means initialized=true.\n@@ -300,7 +300,7 @@ contract MockB20Factory is IB20Factory {\n }\n \n /// @dev Returns true iff `token`'s first 10 bytes match the B-20 prefix.\n- function _isB20Prefix(address token) internal pure returns (bool) {\n+ function _hasB20Prefix(address token) internal pure returns (bool) {\n return (uint160(token) >> 80) == (uint160(0xB2) << 72);\n }\n \n", + "diff_truncated": false, + "kind": "code-change", + "pr_body": "## Summary\n- Reject crediting a B20 balance to **the token's own address** (`to == address(this)`) with the existing `InvalidReceiver(to)` on `transfer`, `transferFrom`, their memo variants, `mint`, `mintWithMemo`, `batchMint`, and `seizeWithMemo`. A B20 token is a precompile with no holder key, so a credit there is unrecoverable by the sender ([BOP-743](https://linear.app/coinbase/issue/BOP-743/disallow-b20-self-sends-in-base-std)).\n- Reuses the existing `InvalidReceiver` error (already fired for `address(0)`) — no new selector, function, or event. The check sits at the existing zero-receiver position, so canonical revert order is unchanged.\n- Sends to **other** B20 tokens stay legal; only `address(this)` is rejected. Holder self-sends (`from == to`) are unchanged.\n- `seizeWithMemo` **from** the token address stays allowed so a balance already stuck there can be recovered to a treasury.\n- Adds the Denim changelog entry, updates `IB20` / `IB20Asset` natspec and the errors reference, and adds unit, revert-order, and `batchMint` tests plus smoke probes that skip cleanly on pre-Denim chains.\n\n> Rebased on `main`. The extra `fix(b20): close mock bootstrap after initialization` commit restores the `MockB20Factory.createB20` line that writes `initialized = true` at the end of creation (an earlier commit had commented it out). Without it every mock token stayed in the factory bootstrap window, failing 10 unit tests plus the Forge Coverage and Interface Coverage jobs.\n\n## Test plan\n- [x] `forge test --match-test 'tokenRecipient|fromTokenAddress'`\n- [x] `forge test`\n- [x] `python3 script/check-coverage.py`\n- [ ] `make fork-tests` once the matching base/base change lands (cross-validate against Rust)\n- [ ] Smoke against a Denim-activated node: transfer/mint to `tok.address` → `InvalidReceiver`; `seizeWithMemo` from `tok.address` → succeeds", + "pr_number": "232", + "pr_title": "feat(b20): reject the token itself as a credit recipient", + "sha": "150532313c10a410fd81d74d5f1ca0df43865822", + "source_repo": "base/base-std" +} From 415bbad0ad386610c2d268d6506b0030b85163eb Mon Sep 17 00:00:00 2001 From: sohey Date: Mon, 5 Oct 2026 21:54:39 +0200 Subject: [PATCH 5/6] fix(docs-sync): find NatSpec-documented members; guard code samples and retry streams From the base-std@1505323 eval (0/2 before, 6/6 after across both cases): - natspecDocumentedSymbols: a NatSpec hunk usually stops above the declaration it documents, so mint.mdx was skipped whenever the model's manifest happened not to name mint. Walk the post-change source (already fetched for changelog entries) from each changed comment line to the next declaration; function-reference pages for those members are always called. - isCommentOnlyChange: reference mocks (test/lib/mocks/) count as code. Base Std is interface-only, so NatSpec plus a mock change is a behavior change, not a clarification. The prompt no longer lets the model infer "clarification" from a comment-only diff slice; only the flag decides. - restoreCodeSamples: on guide pages, when no signature changed upstream, restore fenced code blocks (not mermaid) the model altered. A run had rewritten send-a-payout's functionName to "simulateContract". - normalizeForNoop: table re-padding alone is a noop. - llm/client: retry mid-stream overloaded/api errors twice with backoff; the SDK's maxRetries only covers the initial response, and one such error failed a whole sync in the eval. Co-authored-by: Toshi --- .../__tests__/code-sample-guard.test.mjs | 51 ++++++++ .../__tests__/stream-retry.test.mjs | 12 ++ .../__tests__/symbol-route-gate.test.mjs | 60 +++++++++ scripts/sync-from-base-std/index.mjs | 34 ++++- scripts/sync-from-base-std/llm/client.mjs | 46 +++++-- scripts/sync-from-base-std/llm/prompts.mjs | 2 +- scripts/sync-from-base-std/release-utils.mjs | 119 ++++++++++++++++-- 7 files changed, 303 insertions(+), 21 deletions(-) create mode 100644 scripts/sync-from-base-std/__tests__/code-sample-guard.test.mjs create mode 100644 scripts/sync-from-base-std/__tests__/stream-retry.test.mjs diff --git a/scripts/sync-from-base-std/__tests__/code-sample-guard.test.mjs b/scripts/sync-from-base-std/__tests__/code-sample-guard.test.mjs new file mode 100644 index 000000000..76fb43636 --- /dev/null +++ b/scripts/sync-from-base-std/__tests__/code-sample-guard.test.mjs @@ -0,0 +1,51 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { manifestChangesSignatures, restoreCodeSamples } from "../release-utils.mjs"; + +const page = (fn, prose = "Send payouts.", diagram = "A-->B") => [ + "# Send a payout", + "", + prose, + "", + "```ts", + "const simulation = await publicClient.simulateContract({", + ` functionName: "${fn}",`, + "});", + "```", + "", + "```mermaid", + `flowchart TD`, + ` ${diagram}`, + "```", + "", +].join("\n"); + +test("restoreCodeSamples: reverts a changed sample, keeps prose and diagram edits", () => { + const current = page("sendPayouts"); + const updated = page("simulateContract", "Send payouts. Recipients cannot be the token itself.", "A-->C"); + const r = restoreCodeSamples(current, updated); + assert.equal(r.restored, 1); + assert.equal(r.content, page("sendPayouts", "Send payouts. Recipients cannot be the token itself.", "A-->C")); +}); + +test("restoreCodeSamples: block count mismatch leaves output alone", () => { + const current = page("sendPayouts"); + const updated = current + "\n```bash\nnpm i\n```\n"; + const r = restoreCodeSamples(current, updated); + assert.equal(r.mismatched, true); + assert.equal(r.content, updated); +}); + +test("manifestChangesSignatures", () => { + assert.equal(manifestChangesSignatures([{ kind: "revert_added" }]), false); + assert.equal(manifestChangesSignatures([{ kind: "signature_change" }]), true); +}); + +test("normalizeForNoop: table re-padding is a noop, a cell edit is not", async () => { + const { normalizeForNoop } = await import("../release-utils.mjs"); + const a = "| `NotSoleAdmin()` | renounce while others exist |\n"; + const b = "| `NotSoleAdmin()` | renounce while others exist |\n"; + const c = "| `NotSoleAdmin()` | renounce while other admins exist |\n"; + assert.equal(normalizeForNoop(a), normalizeForNoop(b)); + assert.notEqual(normalizeForNoop(a), normalizeForNoop(c)); +}); diff --git a/scripts/sync-from-base-std/__tests__/stream-retry.test.mjs b/scripts/sync-from-base-std/__tests__/stream-retry.test.mjs new file mode 100644 index 000000000..224deafbc --- /dev/null +++ b/scripts/sync-from-base-std/__tests__/stream-retry.test.mjs @@ -0,0 +1,12 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { isTransientStreamError } from "../llm/client.mjs"; + +test("isTransientStreamError: mid-stream overload is retried, real failures are not", () => { + assert.equal( + isTransientStreamError(new Error('{"type":"error","error":{"type":"overloaded_error","message":"The model stopped sending data. Retry this request."}}')), + true, + ); + assert.equal(isTransientStreamError(new Error("LLM gateway returned no text content")), false); + assert.equal(isTransientStreamError(new Error('{"type":"error","error":{"type":"invalid_request_error"}}')), false); +}); diff --git a/scripts/sync-from-base-std/__tests__/symbol-route-gate.test.mjs b/scripts/sync-from-base-std/__tests__/symbol-route-gate.test.mjs index 34f439eb2..7ae78d061 100644 --- a/scripts/sync-from-base-std/__tests__/symbol-route-gate.test.mjs +++ b/scripts/sync-from-base-std/__tests__/symbol-route-gate.test.mjs @@ -4,7 +4,9 @@ import fs from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; import { + decideCall, isCommentOnlyChange, + natspecDocumentedSymbols, newestChangelogFork, splitDiffByFile, symbolRouteGate, @@ -31,6 +33,17 @@ test("isCommentOnlyChange: a code line makes it a real change", () => { assert.equal(isCommentOnlyChange(splitDiffByFile(diff)), false); }); +test("isCommentOnlyChange: base-std#232 (NatSpec + mocks + changelog) is a real change", () => { + const payload = fixture("code-change-1505323-token-self-recipient.json"); + const byFile = splitDiffByFile(payload.diff); + assert.equal(isCommentOnlyChange(byFile), false); + // The interface edits alone are pure NatSpec; the mock change is what marks it. + const srcAndMocks = new Map([...byFile].filter(([f]) => f.startsWith("src/") || f.startsWith("test/lib/mocks/"))); + assert.equal(isCommentOnlyChange(srcAndMocks), false); + const srcOnly = new Map([...byFile].filter(([f]) => f.startsWith("src/"))); + assert.equal(isCommentOnlyChange(srcOnly), true); +}); + test("isCommentOnlyChange: block comments count, tests are ignored, non-Solidity is not", () => { const sol = [ "diff --git a/src/I.sol b/src/I.sol", @@ -86,3 +99,50 @@ test("symbolRouteGate: historical changelog entries and upgrade pages are not ro // No fork data → no fork gating. assert.equal(symbolRouteGate("docs/upgrades/beryl/b20.mdx", { role: "guide", newestFork: "" }), null); }); + +test("natspecDocumentedSymbols: a @dev hunk that stops above the declaration still names it", async () => { + const payload = fixture("code-change-1505323-token-self-recipient.json"); + const section = splitDiffByFile(payload.diff).get("src/interfaces/IB20.sol"); + // Minimal post-image: the mint hunk's lines, then @param lines, then the declaration. + const post = []; + const at = (n, text) => { while (post.length < n - 1) post.push(""); post.push(text); }; + at(414, " ///"); + at(415, " /// @dev Reverts with `ContractPaused(MINT)` when `MINT` is paused."); + at(416, " /// @dev Reverts with `AccessControlUnauthorizedAccount` when the caller does not hold `MINT_ROLE`."); + at(417, " /// @dev Reverts with `InvalidReceiver` when `to == address(0)` or `to == address(this)`."); + at(418, " /// @dev Reverts with `PolicyForbids(MINT_RECEIVER_POLICY, ...)` when `to` is not authorized."); + at(419, " /// @dev Reverts with `SupplyCapExceeded` when `totalSupply + amount > supplyCap`."); + at(420, " ///"); + at(421, " /// @param to Recipient."); + at(422, " /// @param amount Amount."); + at(423, " function mint(address to, uint256 amount) external;"); + const got = natspecDocumentedSymbols(section, post.join("\n")); + assert.ok(got.includes("mint"), `got ${got}`); +}); + +test("natspecDocumentedSymbols: errors, events, constants; code lines stop the walk", () => { + const section = [ + "@@ -1,6 +1,6 @@", + "- /// @notice old", + "+ /// @notice new", + " error InvalidReceiver(address receiver);", + " ", + "- /// old", + "+ /// new", + " bytes32 public constant SEIZE_ROLE = keccak256(\"SEIZE_ROLE\");", + ].join("\n"); + const post = [ + " /// @notice new", + " error InvalidReceiver(address receiver);", + "", + " /// new", + " bytes32 public constant SEIZE_ROLE = keccak256(\"SEIZE_ROLE\");", + ].join("\n"); + assert.deepEqual(natspecDocumentedSymbols(section, post).sort(), ["InvalidReceiver", "SEIZE_ROLE"]); +}); + +test("decideCall: function-reference is called when its member's NatSpec changed", () => { + const content = '---\ntitle: "IB20.mint"\n---\nbody'; + assert.match(decideCall({ role: "function-reference", content, diffSlice: "", symbols: [] }), /own symbol mint/); + assert.equal(decideCall({ role: "function-reference", content, diffSlice: "", symbols: [], documented: ["mint"] }), null); +}); diff --git a/scripts/sync-from-base-std/index.mjs b/scripts/sync-from-base-std/index.mjs index f2b6f3278..1389f7e9e 100644 --- a/scripts/sync-from-base-std/index.mjs +++ b/scripts/sync-from-base-std/index.mjs @@ -80,6 +80,10 @@ import { decideCall, changedLines, isCommentOnlyChange, + natspecDocumentedSymbols, + manifestChangesSignatures, + restoreCodeSamples, + normalizeForNoop, newestChangelogFork, symbolRouteGate, } from "./release-utils.mjs"; @@ -1562,7 +1566,18 @@ async function processPage(item, shared, useGroups) { console.error(`[reject] ${item.page}: ${reason}`); return { page: item.page, status: "rejected", reason }; } - const out = stripAuthorAttribution(completion.text); + let out = stripAuthorAttribution(completion.text); + + // Guides: code samples cannot need changing unless a signature did. + if (pageRole === "guide" && kind === "code-change" && !manifestChangesSignatures(shared.manifest)) { + const guard = restoreCodeSamples(current, out); + if (guard.restored > 0) { + console.warn(`[guard] ${item.page}: restored ${guard.restored} code sample(s) the model changed; no signature changed upstream`); + out = guard.content; + } else if (guard.mismatched) { + console.warn(`[guard] ${item.page}: model added or removed a code block; left for review`); + } + } const err = validateMdx(out, item.page, knownRoutes, { current, @@ -1588,9 +1603,8 @@ async function processPage(item, shared, useGroups) { // whitespace and any stale sync-source comment), AND // 2. No release version bump happened, AND // 3. The page on main HAS NO stale sync-source comment to clean up. - const trimTrailing = (s) => s.replace(/\s+$/, ""); const currentClean = stripProv(current); - const semanticEqual = trimTrailing(next) === trimTrailing(currentClean); + const semanticEqual = normalizeForNoop(next) === normalizeForNoop(currentClean); const currentHasStaleProvenance = current !== currentClean; if (semanticEqual && bumpCount === 0 && !currentHasStaleProvenance) { console.log( @@ -1855,6 +1869,18 @@ async function main() { if (!w.reasons) w.reasons = (w.sourceFiles || []).map((sf) => `path:${sf}`); } + // Members whose NatSpec the diff edits. The hunk often ends above the + // declaration, so read the post-change file to find it. Without a token + // this is empty and the old diff/manifest test applies unchanged. + const documented = []; + for (const [file, section] of diffByFile) { + if (!file.startsWith("src/") || !file.endsWith(".sol")) continue; + if (!changedLines(section).split("\n").some((l) => /^[+-]\s*(\/\/|\/\*|\*)/.test(l))) continue; + const full = await fetchSourceFile(sourceRepo(payload), sha, file); + if (full) documented.push(...natspecDocumentedSymbols(section, full)); + } + if (documented.length) console.log(`[symbols] NatSpec edited for: ${[...new Set(documented)].join(", ")}`); + // One decision per page, made here and logged with the work list: // does this page need a model call at all? for (const w of work) { @@ -1865,7 +1891,7 @@ async function main() { const diffSlice = changedLines( (w.sourceFiles || []).filter((sf) => !sf.startsWith("test/")).map((sf) => diffByFile.get(sf) || "").join("\n"), ); - w.skip = decideCall({ role: pageRoleFor(w.page, layout), content, diffSlice, manifest, symbols }); + w.skip = decideCall({ role: pageRoleFor(w.page, layout), content, diffSlice, manifest, symbols, documented }); } const modelBound = work.filter((w) => !w.skip); if (modelBound.length > CODE_CHANGE_MAX_PAGES) { diff --git a/scripts/sync-from-base-std/llm/client.mjs b/scripts/sync-from-base-std/llm/client.mjs index 50b25569c..5abfd84cd 100644 --- a/scripts/sync-from-base-std/llm/client.mjs +++ b/scripts/sync-from-base-std/llm/client.mjs @@ -111,6 +111,32 @@ function getClient() { * prompt where they're easier to tune per dispatch kind. * @returns {Promise} the model's text output */ +/** + * True for an error event that arrives inside an already-open stream + * (`overloaded_error`, `api_error`, a dropped stream). The SDK's + * `maxRetries` only covers the initial HTTP response, so these would + * otherwise fail the whole sync on one transient hiccup. + */ +export function isTransientStreamError(err) { + const msg = String(err?.message || err || ""); + return /overloaded_error|"type":"api_error"|stopped sending data|terminated|ECONNRESET|socket hang up/i.test(msg); +} + +const STREAM_RETRIES = 2; + +async function withStreamRetry(fn, page, { retries = STREAM_RETRIES, baseDelayMs = 5000 } = {}) { + for (let attempt = 0; ; attempt++) { + try { + return await fn(); + } catch (err) { + if (attempt >= retries || !isTransientStreamError(err)) throw err; + const delay = baseDelayMs * 2 ** attempt; + console.warn(`[llm] ${page || "call"}: transient stream error, retrying in ${delay / 1000}s (${attempt + 1}/${retries})`); + await new Promise((r) => setTimeout(r, delay)); + } + } +} + /** * Send one prompt through the Gateway and return the text plus the facts a * caller needs to judge it (stop reason, output tokens). Streams the @@ -129,14 +155,18 @@ export async function complete(prompt, page = "", opts = {}) { const maxTokens = opts.maxTokens || DEFAULT_MAX_TOKENS; const tStart = Date.now(); - const message = await client.messages - .stream({ - model, - max_tokens: maxTokens, - ...(opts.system ? { system: opts.system } : {}), - messages: [{ role: "user", content: prompt }], - }) - .finalMessage(); + const message = await withStreamRetry( + () => + client.messages + .stream({ + model, + max_tokens: maxTokens, + ...(opts.system ? { system: opts.system } : {}), + messages: [{ role: "user", content: prompt }], + }) + .finalMessage(), + page, + ); const text = (message.content || []) .filter((b) => b.type === "text") diff --git a/scripts/sync-from-base-std/llm/prompts.mjs b/scripts/sync-from-base-std/llm/prompts.mjs index f74d171dc..0df065953 100644 --- a/scripts/sync-from-base-std/llm/prompts.mjs +++ b/scripts/sync-from-base-std/llm/prompts.mjs @@ -115,7 +115,7 @@ const SHARED_RULES = `Hard requirements for your output: STEP 2 — INTERSECT. For each item from step 1, identify the changes in the source that touch it. • If a \`\` block is present below, it has already been extracted from the diff by a pre-pass. Every entry in it that names a surface from your step-1 inventory is an intersection — treat the manifest as the starting list. A manifest entry that matches only a mention is not an intersection. - • If the diff changes only comments or NatSpec and no code, it clarifies existing behavior rather than changing it. Treat a statement on the page as an intersection only if the clarification shows it is wrong or incomplete in a way that would mislead the reader. + • Only when the Context section says the change type is a comments/NatSpec-only clarification: treat a statement on the page as an intersection only if the clarification shows it is wrong or incomplete in a way that would mislead the reader. Do not infer this yourself from the diff — Base Std is interface-only, so a NatSpec edit in can record a real behavior change. • Always cross-check the manifest against \`\`: the manifest may miss something, especially newly-added fields buried in large diffs. If you find an additional intersection in the diff that isn't in the manifest, add it to your list and apply it in step 3. • If \`\` is absent or empty, fall back to scanning \`\` directly for: – Solidity parameter or return-type changes diff --git a/scripts/sync-from-base-std/release-utils.mjs b/scripts/sync-from-base-std/release-utils.mjs index b8e320f52..913a52530 100644 --- a/scripts/sync-from-base-std/release-utils.mjs +++ b/scripts/sync-from-base-std/release-utils.mjs @@ -585,8 +585,10 @@ const mentions = (text, sym) => * Returns null to call, or the reason to skip. The rules by page role: * * function-reference the page's own symbol (from its title, e.g. - * "IB20.seizeWithMemo") appears in the diff slice or - * among the routing symbols + * "IB20.seizeWithMemo") appears in the diff slice, + * among the routing symbols, or among `documented` + * (members whose NatSpec the diff edits; see + * natspecDocumentedSymbols) * interface-index the manifest adds/removes/renames a member of this * interface, or a routing symbol appears on the page * shared-reference, a routing symbol appears in the page's code spans; @@ -597,7 +599,7 @@ const mentions = (text, sym) => * Any page larger than MAX_REGENERABLE_CHARS is skipped regardless of role, * except the summary page, which never goes to the model. */ -export function decideCall({ role, content = "", diffSlice = "", manifest = [], symbols = [] }) { +export function decideCall({ role, content = "", diffSlice = "", manifest = [], symbols = [], documented = [] }) { if (role === "changelog-index") return null; if (content.length > MAX_REGENERABLE_CHARS) { return `page is ${content.length} chars; a full regeneration exceeds the ${MAX_REGENERABLE_CHARS}-char budget (edit mode pending)`; @@ -611,7 +613,7 @@ export function decideCall({ role, content = "", diffSlice = "", manifest = [], if (role === "function-reference") { const own = title.includes(".") ? title.split(".").pop() : title; if (!own) return null; - if (mentions(diffSlice, own) || symbols.includes(own) || symbols.includes(title)) return null; + if (mentions(diffSlice, own) || symbols.includes(own) || symbols.includes(title) || documented.includes(own)) return null; return `own symbol ${own} is not in the diff or the manifest`; } if (role === "interface-index") { @@ -639,21 +641,122 @@ export function changedLines(diff) { .join("\n"); } +const COMMENT_LINE_RE = /^\s*(\/\/|\/\*|\*)/; +const DECLARATION_RE = + /^\s*(?:function|error|event|modifier|struct|enum)\s+([A-Za-z_]\w*)|\bconstant\s+([A-Za-z_]\w*)/; + +/** + * Members whose NatSpec a Solidity diff edits. A NatSpec hunk often stops + * short of the declaration it documents (a `@dev Reverts ...` line sits + * several `@param` lines above `function mint(`), so neither the diff slice + * nor a model-written manifest reliably names the member. This walks the + * post-change source forward from each changed comment line to the next + * declaration. + * + * @param {string} section one file's unified diff section + * @param {string} postImage the whole file at the dispatched sha + * @returns {string[]} member names, deduplicated + */ +export function natspecDocumentedSymbols(section, postImage) { + const src = String(postImage || "").split("\n"); + const anchors = []; // 0-based post-image line indices of changed comment lines + let next = 0; // 0-based post-image index of the next line in the hunk + for (const line of String(section || "").split("\n")) { + const h = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(line); + if (h) { next = Number(h[1]) - 1; continue; } + if (line.startsWith("+++") || line.startsWith("---")) continue; + if (line.startsWith("+")) { + if (COMMENT_LINE_RE.test(line.slice(1))) anchors.push(next); + next++; + } else if (line.startsWith("-")) { + // A removed comment line documents whatever now follows it. + if (COMMENT_LINE_RE.test(line.slice(1))) anchors.push(next); + } else if (line.startsWith(" ")) { + next++; + } + } + const out = new Set(); + for (const start of anchors) { + for (let i = start; i < src.length; i++) { + const text = src[i]; + if (text.trim() === "" || COMMENT_LINE_RE.test(text)) continue; + const m = DECLARATION_RE.exec(text); + if (m) out.add(m[1] || m[2]); + break; // the first code line is the documented declaration, or nothing + } + } + return [...out]; +} + +/** + * A page for the noop test: trailing whitespace dropped and Markdown table + * rows with their cell padding collapsed. A regeneration that only realigns + * a table's columns changes nothing a reader sees (base-std#232 eval: + * roles-and-pause.mdx re-padded one row). + */ +export function normalizeForNoop(s) { + return String(s || "") + .split("\n") + .map((l) => (/^\s*\|/.test(l) ? l.replace(/[ \t]+/g, " ").trim() : l.replace(/[ \t]+$/, ""))) + .join("\n") + .replace(/\s+$/, ""); +} + +// ------------------------------------------------------- code-sample guard + +const FENCE_RE = /^([ \t]*)(`{3,}|~{3,})([^\n]*)\n[\s\S]*?\n\1\2[ \t]*$/gm; + +/** True when the manifest changes a signature or an interface's member set. */ +export function manifestChangesSignatures(manifest) { + return (manifest || []).some((e) => INVENTORY_KINDS.has(e.kind)); +} + +/** + * Put a guide's code samples back exactly as they were. A full-page + * regeneration sometimes "fixes" a working sample (base-std#232 eval: a + * viem call's functionName became "simulateContract"). When the source + * change alters no signature, no sample can need to change, so every fenced + * block except diagrams (mermaid) is restored from the current page, in + * order. If the model added or removed blocks the pairing is ambiguous; + * the output is returned untouched and the caller decides. + * + * @returns {{content: string, restored: number, mismatched: boolean}} + */ +export function restoreCodeSamples(current, updated) { + const before = [...String(current).matchAll(FENCE_RE)]; + const after = [...String(updated).matchAll(FENCE_RE)]; + if (before.length !== after.length) return { content: updated, restored: 0, mismatched: true }; + let restored = 0; + let i = 0; + const content = String(updated).replace(FENCE_RE, (block, _indent, _fence, info) => { + const original = before[i++][0]; + if (/^\s*mermaid\b/.test(info) || block === original) return block; + restored++; + return original; + }); + return { content, restored, mismatched: false }; +} + // ------------------------------------------------------- symbol-route gating /** * True when every changed line in the dispatch's source files is a comment * (Solidity `//`, `///`, or a block-comment line). Such a change clarifies - * documented behavior; it does not change it. Test files are ignored, the - * same as for routing. Any non-Solidity source file, or no changed lines at - * all, returns false so ordinary routing applies. + * documented behavior; it does not change it. + * + * Base Std is interface-only, so a real behavior change often appears in + * `src/` as NatSpec alone. What separates it from a clarification is that + * the reference implementation (`test/lib/mocks/`) or a changelog entry + * changes too. Mocks therefore count as code here; other test files are + * ignored. Any non-Solidity file (a changelog entry, a source doc), or no + * changed lines at all, returns false so ordinary routing applies. * * @param {Map} diffByFile path → unified diff section */ export function isCommentOnlyChange(diffByFile) { let sawChange = false; for (const [file, section] of diffByFile || []) { - if (file.startsWith("test/")) continue; + if (file.startsWith("test/") && !file.startsWith("test/lib/mocks/")) continue; if (!file.endsWith(".sol")) return false; for (const line of changedLines(section).split("\n")) { const body = line.slice(1).trim(); From a9229e9763e5833492f8a55cfcae07b04bb7c8f3 Mon Sep 17 00:00:00 2001 From: sohey Date: Mon, 5 Oct 2026 22:02:11 +0200 Subject: [PATCH 6/6] docs(docs-sync): shrink the evals README section to a pointer Usage lives in the run-eval.mjs header and each case documents itself; the README only needs to say when to run it. Co-authored-by: Toshi --- scripts/sync-from-base-std/README.md | 28 +++------------------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/scripts/sync-from-base-std/README.md b/scripts/sync-from-base-std/README.md index 9bdd10540..69a2b1491 100644 --- a/scripts/sync-from-base-std/README.md +++ b/scripts/sync-from-base-std/README.md @@ -101,31 +101,9 @@ Configuration knobs are optional positive numbers: - `CLAUDE_MAX_TOKENS` and `CLAUDE_MODEL` - The bounded release manifest/selection settings documented in `index.mjs` -## Evals - -`eval/run-eval.mjs` replays recorded dispatches against a pinned docs commit -in a throwaway worktree and scores which pages changed. Run it before and -after any change to prompts, routing, or the route table: - -```bash -LLM_GATEWAY_API_KEY=... node scripts/sync-from-base-std/eval/run-eval.mjs --runs 3 -# baseline: the same cases with the sync code from master -LLM_GATEWAY_API_KEY=... node scripts/sync-from-base-std/eval/run-eval.mjs --runs 3 --code-ref master -``` - -Each case in `eval/cases/` names a payload fixture, a `docs_base_sha`, and -expectations: `must_touch`, `must_mention` (a regex the added lines of a -required page must match), `may_touch` (page → added-line budget), -`must_not_touch`, `unlisted_pages` (`fail` or `warn`), and an optional -`restatement` check. Output goes to `.sync-eval/` (gitignored). - -| Case | What it guards | -| --- | --- | -| `3820cf0-isauthorized-natspec` | Precision: a NatSpec-only clarification edits only the owning reference page and its index row. | -| `1505323-token-self-recipient` | Recall: a real behavior change expressed as NatSpec plus mocks still reaches every function reference that documents it. | - -Model output varies, so compare several runs, and read the diffs: the -scores check scope, not wording. +Before changing prompts, routing, or the route table, run the evals with and +without your change (`eval/run-eval.mjs --runs 3`, then `--code-ref master`) +and compare. See the script header for options. ## Source PR attribution