feat(llms): AI integrator metadata for all public modules#330
Closed
bidzyyys wants to merge 25 commits into
Closed
feat(llms): AI integrator metadata for all public modules#330bidzyyys wants to merge 25 commits into
bidzyyys wants to merge 25 commits into
Conversation
Adds an llms.txt at the repo root following the llms.txt convention (https://llmstxt.org/), so AI integrator agents (Cursor, Claude Code, Cline, GitHub Copilot, Aider, Gemini CLI, and similar) can discover the authoritative entry points to this library when assisting downstream Sui Move projects that depend on it. The file is pointer-only — it references the maintained catalogs (contracts/README.md, math/README.md), the MVR @openzeppelin-move namespace for stable releases, and audits/ as the security source of truth. It does not enumerate individual modules or hard-code the current package layout, so it requires no maintenance as the library grows; new top-level groups become discoverable through their own catalog README without changes to llms.txt.
Adds machine-readable YAML metadata for the three modules in the openzeppelin_access package (access_control, two_step_transfer, delayed_transfer) under contracts/access/llms/. The folder mirrors sources/: top-level modules at contracts/access/llms/, modules grouped under ownership_transfer/ at contracts/access/llms/ownership_transfer/. Each YAML describes the module at the schema level — APIs, error codes, preconditions, integration patterns, anti-patterns, decision points — so AI integrator agents can ground code generation against this library without re-deriving structure from Move source. Pinned to commit 55a7971. Verified: scaffolded Move package with each YAML's install snippet + use_statement builds clean under sui move build --build-env testnet. Also updates llms.txt to document the per-package llms/ convention so agents following llms.txt discovery know where to look.
Adds machine-readable YAML metadata for all six modules in the openzeppelin_fp_math package under math/fixed_point/llms/, mirroring sources/ layout: ud30x9/ and sd29x9/ subfolders, each containing the type module, the base operations module, and the conversions module. Each YAML describes its module at the schema level — APIs, error codes, preconditions, integration patterns, anti-patterns (cast-vs-convert, asymmetric signed boundary, raw two's-complement bit comparison, bitwise-as-decimal, rem-vs-mod semantics, log undefined range), and decision points. Pinned to commit 55a7971. Verified: scaffolded Move package with each YAML's install snippet + use_statement builds clean under sui move build --build-env testnet.
Adds machine-readable YAML metadata for all ten modules in the openzeppelin_math package under math/core/llms/, flat to match sources/ layout: the seven per-width primitive integer modules (u8, u16, u32, u64, u128, u256, u512), the rounding-mode enum module, decimal_scaling utilities, and the vector helper macros. Each YAML describes its module at the schema level — APIs, error codes (cross-module errors from internal::macros documented inline), preconditions, integration patterns, anti-patterns (per-width selection, rounding-direction-vs-name confusion, hand-rolled checked shift, std::vector naming collision, u512 software emulation cost), and decision points. Pinned to commit 55a7971. Verified: scaffolded Move package with each YAML's install snippet + use_statement builds clean under sui move build --build-env testnet. Completes per-package llms/ coverage for both contracts-sui packages: openzeppelin_access (3 modules), openzeppelin_fp_math (6 modules), and openzeppelin_math (10 modules) — 19 total module YAMLs.
…ADMEs Three README polish-passes: 1. Root README: the "Docs" section pointed only at local `sui move build --doc`, hiding the comprehensive package guides and API reference at docs.openzeppelin.com/contracts-sui. Replace with a primary pointer to the docs site, mention llms.txt + per-package llms/ for AI integrator agents, and keep the local-doc-generation snippet as a secondary. 2. math/core/README: missing the `## Install` section every other package README ships (without it, integrators can't depend without guessing the MVR slug). Add MVR snippet pinned to @openzeppelin-move/integer-math. Add `## Learn More` with docs links. Switch `rust` code blocks to `move` (correct language hint). 3. math/fixed_point/README: missing `## Install` section. Add MVR snippet pinned to @openzeppelin-move/fixed-point-math. Add `## Learn More` with docs links.
Adds an `index.yaml` to each package's `llms/` directory enumerating every module YAML in the package with a one-line summary, the package's MVR slug, and the commit SHA the YAMLs were extracted from. Before this commit, an AI integrator agent following `llms.txt` to `<pkg>/llms/<module>.yaml` could not learn a package's full module set without either crawling the GitHub directory listing API or already knowing the module names from the per-package README. With `index.yaml`, discovery becomes a single fetch: raw.githubusercontent.com/.../contracts/access/llms/index.yaml raw.githubusercontent.com/.../math/core/llms/index.yaml raw.githubusercontent.com/.../math/fixed_point/llms/index.yaml llms.txt is updated to document the convention. The "coverage is incremental" disclaimer is removed now that every public module in the three packages ships a YAML — only private helpers under each package's sources/internal/ directory are intentionally omitted.
WalkthroughThis PR introduces a comprehensive machine-readable metadata system for OpenZeppelin's Sui Move libraries, enabling AI integrators to discover, validate, and consume standardized specifications for smart contracts via YAML. The implementation includes v1.0 JSON schemas, deterministic Move source extraction scripts, three-tier validation (schema/semantic/completeness), GitHub Actions CI integration, and golden metadata specs covering access control and all math libraries. ChangesAI integrator metadata system
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #330 +/- ##
=======================================
Coverage 95.88% 95.88%
=======================================
Files 22 22
Lines 2186 2186
Branches 595 592 -3
=======================================
Hits 2096 2096
Misses 65 65
Partials 25 25
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ia strict validation
The published v1.0 schema (schemas/llms-metadata-v1.json) was written from
the design spec, not from a read of the published YAMLs. Three fields
diverged structurally, and strict validation surfaced four real data bugs
that compile-check missed.
Schema fixes (drift):
- preconditions[].items: required `error_code + condition + affects`;
YAMLs ship `must + fails_with + affects`. Schema rewritten to match.
- _audit_grounding.precondition_proofs[].items: required `error_code +
test_functions`; YAMLs ship `precondition_fails_with + test_file +
test_functions`. Schema rewritten.
- _audit_grounding.canonical_test: required string|null; YAMLs ship
`{file, function}` object or null. Schema rewritten via oneOf.
Schema relaxations and tightening:
- root `additionalProperties: false` catches typos like `summay:` instead
of `summary:`.
- `domain` relaxed from a fixed enum to a kebab-case pattern so new
categories don't require a schema bump.
- `install.release` now requires a semver-shaped tag when non-null.
- `decisions[].options.minItems: 1` (was 2) to allow info-style single
recommendations where the answer is genuinely unique.
- `_audit_grounding.tests_count` accepts integer or null (sharded suites
legitimately can't carry a count).
- Adds schemas/llms-package-index-v1.json as a separate schema for
per-package index.yaml files.
YAML data fixes (caught by schema validation):
- math/fixed_point/llms/ud30x9/ud30x9_convert.yaml had
`mvr: @openzeppelin-move/fp_math` — the correct MVR slug is
`@openzeppelin-move/fixed-point-math`. Integrators copying the MVR
snippet would have hit a 404 on resolve. compile_check_yamls.sh missed
this because it uses install.github_alternative (git path), not MVR.
- math/fixed_point/llms/sd29x9/sd29x9.yaml was missing the `heuristic`
field on two `do_not_detection` entries.
- math/fixed_point/llms/sd29x9/sd29x9_base.yaml had a partial-null
canonical_test (`{file: ..., function: null}`) which conflated "file
exists" with "no canonical function". Now null'd entirely with the file
context preserved in a comment.
- Three do_not IDs renamed from `from_u128-on-untrusted-input` to
`from-u128-on-untrusted-input` for kebab-case consistency with all
other do_not IDs in the library.
19/19 module YAMLs + 3/3 index YAMLs validate clean against the schemas.
Brings 16 remaining YAMLs (and llms.txt) up to v1.0 — schema_version
bumped from `2` (integer, internal) to `"1.0"` (string, public), one_liner
reshaped to single-line ≤240 chars, new `summary` field added for
multi-paragraph elaboration, and `release: null` comment clarified.
Coverage gap before this commit:
- math/fixed_point/* (7 files) + 3 index.yaml + 2 schemas → already v1.0
- math/core/* (11 files) + contracts/access/{access_control, ownership_transfer/*}
+ llms.txt → still on internal v2
After this commit: all 22 module YAMLs validate clean against
schemas/llms-metadata-v1.json (jsonschema 22/22 PASS), all 3 index.yamls
validate against schemas/llms-package-index-v1.json, and all 19
install snippets compile-check clean.
No behavioral changes — pure schema-version + cosmetic field reshape.
Module APIs, do_not lists, preconditions, audit grounding all unchanged.
Adds a path-scoped CI workflow that re-validates llms metadata YAMLs on every PR — but only the YAMLs the PR actually changed. Schema validation catches data bugs the compile-check misses: wrong MVR slugs, kebab-case violations on do_not[].id, one_liner length-cap overruns, schema_version typed as integer instead of string, required-field omissions, and any addition of unknown fields (root has additionalProperties: false). Scope detection: - Schemas changed (schemas/llms-metadata-v1.json or schemas/llms-package-index-v1.json) → re-validate every YAML under any */llms/. A stricter schema can retroactively invalidate previously-passing YAMLs; we must surface that on the PR that introduces the change. - Otherwise → only YAMLs that appear in the PR diff (status=ACMR). Path filter on the workflow itself keeps it dormant for PRs that don't touch metadata, schemas, the validator, or the workflow file. Validator (scripts/validate-llms-schema.py) is callable both from CI and locally (`scripts/validate-llms-schema.py --all` for a full sweep, or pass individual paths). Routes index.yaml files to the package-index schema and everything else to the per-module schema. Non-existent paths are skipped, which makes feeding `git diff --name-only` output safe even when it includes deleted files. Tested locally against all 22 YAMLs currently shipped on this branch (22/22 PASS reproduced from the validation we ran before publishing v1.0).
…hanges Previously the scope detection only escalated to "validate every YAML" when the JSON Schemas themselves changed. That left a UX gap: the PR that introduces the workflow file (or any subsequent edit to the validator script / workflow) triggers the run but finds zero YAMLs in the diff and reports "no YAMLs to validate" — green, but unproven. The intent of the escalation rule is "anything that could alter validation behavior triggers a full sweep". The validator script and the workflow's own scope-detection logic both meet that bar, so include them alongside the schemas. Side effect (intentional): this commit will, on its own push, re-run the workflow with scope=ALL and validate all 22 YAMLs end to end — giving us first proof in CI that the validator agrees with the locally-verified 22/22 pass.
JSON Schema validates shape but not the relationships between fields.
This adds a second validation layer that checks six referential
integrity invariants within each module YAML:
1. api[].aborts ⊆ errors.keys()
2. preconditions[].fails_with ⊆ errors.keys()
3. preconditions[E].affects = {api.name where E in api.aborts}
4. _audit_grounding.precondition_proofs[].precondition_fails_with
⊆ preconditions[].fails_with
5. _audit_grounding.do_not_demonstrations[].do_not_id
⊆ do_not[].id
6. _audit_grounding.do_not_detection[].do_not_id
⊆ do_not[].id
Check #3 is strict set equality — catches both omissions (api aborts
with E but missing from affects, the exact class of bug we already had
on this branch) and invented entries (affects names an api that does
not actually abort with E).
Workflow integration: a single "Validate (schema + semantics)" step
runs both validators sequentially under one affected-set compute. Both
always run regardless of which fails first, so authors see all issues
in one CI cycle instead of fix-rerun-fix.
Tested locally against all 19 module YAMLs: 18/19 pass; the one failure
caught a real referential omission in access_control.yaml that JSON
Schema validation could not surface (see next commit for the fix).
Acknowledgment: this check was prompted by an independent review that
identified the missing affects entry — exactly the kind of drift that
schema validation alone cannot prevent.
…by semantic check) access_control.yaml declared `assert_has_role` aborts with EUnauthorized in its api[] block, but the EUnauthorized precondition's affects[] list covered only the 10 admin-frequency mutators (grant_role, revoke_role, etc.) and missed assert_has_role. The matching precondition_proofs entry under _audit_grounding likewise omitted the corresponding test function. This was caught by the cross-field semantic validator added in the previous commit (check #3: preconditions[E].affects must equal the union of api.name that abort with E — strict set equality). Before this fix: FAIL contracts/access/llms/access_control.yaml preconditions[6] (fails_with=EUnauthorized).affects mismatch — missing ['assert_has_role'] Changes: - preconditions[EUnauthorized].affects: add assert_has_role; the `must:` narrative extended to cover the membership-query case (assert_has_role checks an arbitrary address rather than the caller). - _audit_grounding.precondition_proofs[EUnauthorized].test_functions: add test_assert_has_role_aborts_for_non_member (already exists in contracts/access/tests/access_control_tests.move:657). After this fix: 19/19 module YAMLs pass semantic validation.
`_audit_grounding.tests_count` aggregated a raw test-line count per module, but the value was either trivially derivable, structurally misleading, or meaningless to integrator agents: - sd29x9.yaml.tests_count was 220, of which 207 covered SIBLING modules via re-exports (the file's own comment admitted this) — pure provenance theater. - ud30x9.yaml.tests_count was 0 because the module has no in-tree tests; agents already know that from the source. - Most other YAMLs duplicated a number the integrator could see by running `grep ^fun.*test tests/<module>_tests.move | wc -l`. The signal that matters — which test exercises a given precondition or demonstrates an anti-pattern — is already carried by canonical_test and precondition_proofs / do_not_demonstrations. Those stay. Changes: - Remove `tests_count` from schemas/llms-metadata-v1.json (the `_audit_grounding` object enforces additionalProperties: false so YAMLs without the field still validate; YAMLs WITH the field would fail going forward). - Remove `tests_count:` lines (and their multi-line continuation comments) from all 19 module YAMLs. - Remove tests_count references from the generator skill: - SKILL.md `_audit_grounding:` synthesis rules - format-spec.md schema sample - golden-rbac.yaml (verbatim copy of access_control.yaml) After this commit: 22 YAMLs validate clean against the updated schema (strict additionalProperties); 19/19 module YAMLs still pass the cross-field semantic validator. Acknowledgment: noted by independent reviewers as "provenance theater" that aggregates across sibling modules and tells the integrator nothing.
…ibility Sui Move's `public use fun OTHER::name as Self.name;` syntax lets a module expose sibling-module APIs as method-style calls on its own type. The reachable surface of such a module is its own functions PLUS the re-exported set — but the YAML structure of the metadata layer splits modules one-per-file, so an integrator agent fetching only `sd29x9.yaml` would miss the ~80 method-style calls re-exported from `sd29x9_base` and `sd29x9_convert`. Adds an optional `module.re_exports_from: [module_name, ...]` field naming the sibling modules whose APIs are re-exported. Two YAMLs in this repo qualify: sd29x9.yaml → re_exports_from: [sd29x9_base, sd29x9_convert] ud30x9.yaml → re_exports_from: [ud30x9_base, ud30x9_convert] Schema additive (non-breaking) — `minItems: 1, uniqueItems: true, items match ^[a-z][a-z0-9_]*$`. Field omitted entirely when no re-exports exist (the common case). Skill (SKILL.md + format-spec.md) updated to describe when to emit the field and how to discover re-exports mechanically (`grep "^public use fun" <module>.move` for entries pointing at sibling modules in the same package). After this commit: 22 YAMLs pass schema validation; 19/19 module YAMLs pass semantic validation. No existing checks affected; future surface-match validator (CI job #4) will be able to verify re_exports_from matches the actual `public use fun` set in source. Acknowledgment: this gap was flagged by an independent reviewer referencing the sd29x9.move re-export pattern.
Module YAMLs no longer carry `mvr`, `repo_ref`, `release`,
`move_toml_snippet`, or `github_alternative` — these are emitted once
per package in the per-package `index.yaml` under its own `install:`
block. Per-module `install:` shrinks to `use_statement` only.
Eliminates ~250 lines of duplication across 19 module YAMLs and the
cross-file drift surface where a module's `repo_ref` could diverge
from the index's. `github_alternative` is now canonical and
package-agnostic: TOML table-heading form rationale + all three
pinning strategies (latest tag / SHA / branch) spelled out, no
concrete version tag baked into the comment.
Schemas: both `llms-metadata-v1.json` and `llms-package-index-v1.json`
remain at `schema_version: "1.0"`. The module schema shrinks
`install` to `{ use_statement }`; the index schema gains the new
`install` object and removes the previous top-level `mvr`/`repo_ref`/
`release` fields (moved inside).
Skill updates: `golden-rbac.yaml` synced verbatim to the migrated
`access_control.yaml` (also picks up the `assert_has_role` precondition
fix from f74d50e that the golden missed). `SKILL.md` Stage 2 section
split into package-level (index) vs module-level rules. `format-spec.md`
gains a dedicated "Index schema" section showing the new index shape.
Validators (22/22 schema PASS, 19/19 semantics PASS) confirm every
existing YAML conforms to the new schema shape.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ustering + canonical_test check Improvements found by regenerating access + fixed-point modules from source via fresh sub-agents and converging do_not[] to the canonical set. extract_move_structure.sh: - New DOC_GUIDANCE| pass — extracts narrative `/// #### Security* / Misuse* / *Warning / *Guidance / Invariant / Tradeoff` doc blocks (module- and function-level), filtering out mechanical Parameters/Returns/Aborts. Makes do_not[] coverage deterministic from source even without Notion. SKILL.md do_not[] guidance: - MANDATORY: every hazard DOC_GUIDANCE block must be covered by do_not[]. - Type-capability inference: key-without-store wrapper -> assume-wrapper-publicly-transferable; no-ability guard -> drop-<guard>-hot-potato; drop-only proof -> store-ephemeral-proof. - Operation-aware clustering: cluster signals about the SAME operation into one entry (two_step Misuse Paths + Security Warning + Security Note -> one shared-object-executor-init), keep DISTINCT operations separate (ud30x9 wrap vs unwrap vs bitwise -> three entries). - Stable kebab ids for recurring patterns so regenerations converge. validate-llms-semantics.py: - New check #7: _audit_grounding.canonical_test (when non-null) must point at a real {file, function} — reads the Move test file and confirms the named `fun` is declared. Closes the silent-pass gap for fabricated test names. Convergence verified: access_control 4/4 do_not (Notion), two_step 3/3 (DOC_GUIDANCE clustering), ud30x9 3/3 (operation clustering). 19/19 schema + 19/19 semantics pass on the canonical corpus; extractor exit 0 on all 19. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Re-generated via the generate-sui-metadata skill with the full feed (source + Notion Access Control R&D page + docs PR #153). Structurally identical to the prior canonical (same 22 api, 13 errors, 2 types, 7 preconditions, 4 do_not ids, 7 precondition_proofs) with Notion-sourced enrichment: - summary now states the two structural invariants (singleton OTW-keyed registry + home-module role check) that make every Auth<Role> self-validating — the core design rationale from the R&D page. - does_not_solve sharpened (enumerable "storage supports it, deferred"; added package-upgrade safety of the registry itself). - api reordered to actions-first / queries-last per the format spec. Schema + semantics pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Single-fetch entry point for MCP servers / AI agents to discover every package in one request, before drilling into per-package index.yaml and per-module YAMLs. The narrative llms.txt stays the human-facing convention entry; this is its structured sibling. - llms/index.yaml: root catalog enumerating the 3 packages (name, group, mvr slug, path to each package's index.yaml, one-line summary). - schemas/llms-package-index-v1.json: extended to a oneOf over two forms — root catalog (packages[]) and per-package index (modules[]) — distinguished by disjoint required keys (repo+packages vs package+install+modules). One schema, no new file; dispatch stays by the shared `index.yaml` name. - llms.txt: points at the root catalog as the structured discovery entry. - CI: scope=DIFF grep widened to `(^|/)llms/` so the root llms/index.yaml (whose path has no leading slash) is picked up on change. Deliberately minimal: no dedicated catalog schema or filename — an earlier iteration that added both was reverted as overkill. The root catalog reuses the index.yaml name + index schema; glob `**/llms/index.yaml` remains a fallback for consumers with repo access. Validation: root catalog + 3 package indexes + 19 module YAMLs all pass (23/23 schema, 19/19 semantics). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Integrator code-quality fix found by running /move-quality on a contract that a fresh-context agent built from the metadata alone: the agent copied the bare `const EName: u64 = N;` error-const form because the math snippets modeled it (26 occurrences across 9 math/core YAMLs). Integrator agents copy setup_snippet / quick_start / do_not example code verbatim, so the non-idiomatic form propagated into generated downstream code. Converted all 26 to the Move 2024 idiom the library itself uses and the move-quality checklist requires: #[error(code = 0)] const EMathOverflow: vector<u8> = "arithmetic overflow"; Only declaration lines were rewritten — references to the library's own EOverflow error (e.g. in fixed_point YAMLs) were untouched. Verified the #[error] vector<u8> const aborts correctly inside macro-expanded `.destroy_or!(abort EName)` contexts via a throwaway test build. SKILL.md: added a rule so regeneration keeps the #[error] form even when a source doc block shows the bare u64 form — prevents the fix from regressing. Validators: 23/23 schema, 19/19 semantics. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Third validation layer above schema + semantics: asserts the metadata set stays in sync with the Move source tree, catching the "added a module, forgot its YAML" class (and the reverse). scripts/validate-llms-completeness.py — per package: - every non-internal `<pkg>/sources/**/*.move`, by its DECLARED module name (not the file basename — `conversions.move` declares `sd29x9_convert`), must have an `index.yaml` modules[] entry AND a per-module YAML whose module.name matches the fully-qualified declaration; - orphan check: every per-module YAML must map to a real non-internal source module; - every index modules[].path must resolve on disk. Root catalog (`llms/index.yaml`): every listed package index must exist, and every on-disk package index must be catalogued. Workflow: completeness runs whole-tree and ALWAYS (even when the YAML diff is empty) — a sources-only change can add a public module with no YAML, which produces zero YAML diff but is exactly the gap to catch. Path filter gains `**/sources/**/*.move`. Negative-tested: orphan YAML and missing index path both flagged; current tree passes (3 packages complete).
…process CI validates llms metadata structure (schema), cross-field integrity (semantics), and completeness (module ↔ YAML ↔ index), but does NOT check content drift — a YAML whose api/errors/types have gone stale relative to its source still passes. Close that gap with two process controls: - PR template: a checklist item to regenerate a module's YAML when its public surface changes. - CONTRIBUTING.md: a new "AI integrator metadata" section documenting the layers, the content-drift gap, and the release-process requirement to re-run generate-sui-metadata for every module before tagging — the safety net that guarantees a release's metadata matches its released code.
The skill's `output_path` defaulted to `/workspace/generated/sui-metadata/ <module>.yaml` — a path from the catalina-workspace devcontainer where the skill was originally authored. Now that the skill lives in contracts-sui and runs from inside this checkout, that default wrote the generated YAML to a nonexistent/irrelevant location instead of the module's committed home. stage_sources.sh now computes and emits `METADATA_PATH_IN_REPO` — the canonical destination mirroring the source layout: <pkg>/sources/<subdir>/<file>.move → <pkg>/llms/<subdir>/<module-decl>.yaml The basename is the MODULE DECLARATION name, not the file basename, so `two_step.move` (declaring `two_step_transfer`) correctly resolves to `contracts/access/llms/ownership_transfer/two_step_transfer.yaml`. Verified against all four layout shapes in the repo (flat, nested, access, math). SKILL.md step 5 + the output_path input prompt now reference METADATA_PATH_IN_REPO as the default; an explicit caller-supplied output_path still overrides. No more /workspace references anywhere in the skill. Catalina skill verifier still passes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
38-38:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse
<package>instead of<module>in the doc command.
--pathtakes a package directory, so this placeholder can mislead copy/paste usage.Suggested fix
-sui move build --doc --path <module> +sui move build --doc --path <package>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 38, Update the README command example "sui move build --doc --path <module>" to use the correct placeholder "<package>" since --path expects a package directory; locate the string "sui move build --doc --path <module>" and replace "<module>" with "<package>" and optionally adjust surrounding text to clarify that a package directory path is required.
🧹 Nitpick comments (1)
schemas/llms-metadata-v1.json (1)
223-225: ⚡ Quick winEnforce at least two options per decision.
decisions[]models a branch, butminItems: 1permits non-decisions. Tightening this to2will prevent low-signal entries.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@schemas/llms-metadata-v1.json` around lines 223 - 225, The schema currently allows a decisions array with "minItems": 1 which permits non-decision branches; update the JSON schema for the decisions array (the "decisions" property where "type": "array" and "minItems": 1 appears) to use "minItems": 2 so every decisions[] entry requires at least two options, preserving the existing "items" definition and any other constraints.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/generate-sui-metadata/scripts/extract_move_structure.sh:
- Around line 220-229: The multiline field-extraction loop in
extract_move_structure.sh incorrectly stops only for a `}` at column 0 (the
condition `next_line ~ /^\}/`), so indented closing braces are missed; update
the loop’s termination check to match optional leading whitespace (e.g. use
`next_line ~ /^[[:space:]]*\}/` or equivalent) so the `while ((getline
next_line) > 0)` loop breaks on indented `}` and prevents emitting corrupted
EVENT fields (adjust the same pattern if there are other `}` checks near the
`fields`, `field_name`, and `gsub` logic).
In @.claude/skills/generate-sui-metadata/scripts/stage_sources.sh:
- Around line 114-115: The script currently assigns PR_BASE_REF from PR_JSON
using .baseRefName which is mutable; replace those assignments to capture the PR
base commit SHA instead (e.g., use .baseRefOid or the JSON key that holds the
base commit SHA) and store it in a variable (rename or add PR_BASE_SHA or
overwrite PR_BASE_REF with the SHA) so checkout/fetch operations are
deterministic; update both occurrences (the block around PR_BASE_REF="$(jq -r
'.baseRefName' <<<"$PR_JSON")" and the similar assignments at lines referenced
193-194) and ensure any later git commands use the SHA variable rather than the
branch name.
- Around line 117-125: The candidate file filter only matches paths under
contracts/<pkg>/sources and thus misses other top-level package directories
(e.g., math/<pkg>/sources); update the grep regex used when populating
CANDIDATES in the mapfile pipeline to accept any top-level package name by
replacing the '^contracts/[^/]+/sources/.+\.move$' pattern with a generalized
pattern such as '^[^/]+/[^/]+/sources/.+\.move$' so that the CANDIDATES array
(populated in the mapfile -t CANDIDATES < <(...) block) includes sources from
non-`contracts` packages as well.
In `@contracts/access/llms/access_control.yaml`:
- Around line 380-383: The EUnauthorized coverage list is incomplete—update the
audit grounding entries that enumerate which APIs are affected so they exactly
match the "fails_with: EUnauthorized" declaration; add all referenced symbols
(grant_role, revoke_role, set_role_admin, new_auth, assert_has_role,
begin_default_admin_transfer, begin_default_admin_renounce,
cancel_default_admin_transfer, accept_default_admin_renounce,
begin_default_admin_delay_change, cancel_default_admin_delay_change and any
other root-flow APIs mentioned) to the coverage block (the list around lines
510–519) so the metadata fully mirrors the fails_with list and maintains
one-to-one traceability with EUnauthorized.
In `@math/core/llms/u32.yaml`:
- Around line 349-354: The "fix" guidance for the rule id
default-rounding-without-thinking is self-contradictory; update the fix text so
it consistently prescribes one clear rounding policy (e.g., "Round AGAINST the
user on both deposits-into-vault and withdrawals-from-vault so the protocol
always keeps the remainder") and ensure the wording explicitly states the
direction for each flow and asks reviewers to make the choice visible in code
review.
In `@math/core/llms/u8.yaml`:
- Around line 54-70: The quick_start snippet is missing imports and the error
constant; make it self-contained by adding the appropriate imports for rounding
and u8 (e.g., use openzeppelin_math::{u8, rounding};) and include or import the
error constant used (EMathOverflow) so the example compiles; update the snippet
that exercises u8::mul_div, u8::sqrt, and u8::checked_shl to either declare
#[error] const EMathOverflow or import it from its module so abort EMathOverflow
and assert!(..., EMathOverflow) resolve.
In `@math/fixed_point/llms/ud30x9/ud30x9_base.yaml`:
- Around line 307-311: Update the documentation text around the lshift/rshift
guidance to say: lshift/rshift operate on the raw integer representation (they
perform bit shifts on the stored fixed-point integer and will abort with
EInvalidShiftSize or EOverflow as described) — they are not decimal
multiply/divide operations, but because this implementation uses a fixed 10^9
scale, a bit shift on the raw bits will correspond to multiplying/dividing the
decoded value by powers of two (subject to overflow/truncation); for pure
decimal scaling by 2 use x.add(x) or proper decimal multiply routines instead of
lshift(1). Replace the ambiguous sentences in the blocks mentioning
lshift/rshift (the passages around the current lshift/rshift guidance at lines
~307, ~407-412, and the contradictory note near ~474) with this clarified
wording and keep references to errors EInvalidShiftSize and EOverflow.
In `@schemas/llms-metadata-v1.json`:
- Around line 323-329: The "composes_with" property currently defines items as
"type": "string" but must be an array of structured objects per the documented
contract; update the "composes_with" schema in schemas/llms-metadata-v1.json so
each item is an object with properties "module" (string), "when" (string,
optional/nullable), "source" (object with at least "type"/"uri" or simple string
— choose the project's citation shape), and "example" (string, optional), and
mark "module" as required; also provide appropriate types/formats and a
description for each sub-property to enforce source-citation and composition
metadata.
- Around line 192-201: The errors.code property is currently required and
integer-only which will reject legacy metadata; update the schema so "code" is
not mandatory under the errors object (remove "code" from the "required" array)
and relax its type to accept null as well as integer (e.g., "type":
["integer","null"]) so missing/unknown codes are allowed while preserving
integer validation for present values; locate the errors object and the "code"
property in the schema (errors.code) to make these changes.
---
Outside diff comments:
In `@README.md`:
- Line 38: Update the README command example "sui move build --doc --path
<module>" to use the correct placeholder "<package>" since --path expects a
package directory; locate the string "sui move build --doc --path <module>" and
replace "<module>" with "<package>" and optionally adjust surrounding text to
clarify that a package directory path is required.
---
Nitpick comments:
In `@schemas/llms-metadata-v1.json`:
- Around line 223-225: The schema currently allows a decisions array with
"minItems": 1 which permits non-decision branches; update the JSON schema for
the decisions array (the "decisions" property where "type": "array" and
"minItems": 1 appears) to use "minItems": 2 so every decisions[] entry requires
at least two options, preserving the existing "items" definition and any other
constraints.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a17f0365-cd9b-4ede-aeb4-1269a104ccdf
📒 Files selected for processing (45)
.claude/skills/generate-sui-metadata/SKILL.md.claude/skills/generate-sui-metadata/references/format-spec.md.claude/skills/generate-sui-metadata/references/golden-rbac.yaml.claude/skills/generate-sui-metadata/scripts/extract_move_structure.sh.claude/skills/generate-sui-metadata/scripts/extract_tests.sh.claude/skills/generate-sui-metadata/scripts/stage_docs_pr.sh.claude/skills/generate-sui-metadata/scripts/stage_sources.sh.claude/skills/generate-sui-metadata/scripts/validate_metadata.sh.github/pull_request_template.md.github/workflows/llms-schema-validation.yml.gitignoreCONTRIBUTING.mdREADME.mdcontracts/README.mdcontracts/access/llms/access_control.yamlcontracts/access/llms/index.yamlcontracts/access/llms/ownership_transfer/delayed_transfer.yamlcontracts/access/llms/ownership_transfer/two_step_transfer.yamlllms.txtllms/index.yamlmath/core/README.mdmath/core/llms/decimal_scaling.yamlmath/core/llms/index.yamlmath/core/llms/rounding.yamlmath/core/llms/u128.yamlmath/core/llms/u16.yamlmath/core/llms/u256.yamlmath/core/llms/u32.yamlmath/core/llms/u512.yamlmath/core/llms/u64.yamlmath/core/llms/u8.yamlmath/core/llms/vector.yamlmath/fixed_point/README.mdmath/fixed_point/llms/index.yamlmath/fixed_point/llms/sd29x9/sd29x9.yamlmath/fixed_point/llms/sd29x9/sd29x9_base.yamlmath/fixed_point/llms/sd29x9/sd29x9_convert.yamlmath/fixed_point/llms/ud30x9/ud30x9.yamlmath/fixed_point/llms/ud30x9/ud30x9_base.yamlmath/fixed_point/llms/ud30x9/ud30x9_convert.yamlschemas/llms-metadata-v1.jsonschemas/llms-package-index-v1.jsonscripts/validate-llms-completeness.pyscripts/validate-llms-schema.pyscripts/validate-llms-semantics.py
Comment on lines
+220
to
+229
| if ($0 ~ /\{[[:space:]]*$/) { | ||
| fields = "" | ||
| while ((getline next_line) > 0) { | ||
| if (next_line ~ /^\}/) break | ||
| field_name = next_line | ||
| gsub(/^[[:space:]]+/, "", field_name) | ||
| gsub(/:.*$/, "", field_name) | ||
| if (length(field_name) > 0 && field_name !~ /^\/\//) { | ||
| fields = fields (length(fields) > 0 ? "," : "") field_name | ||
| } |
There was a problem hiding this comment.
Fix multiline event parsing termination for indented }.
Line 223 only matches } at column 0. In normal Move formatting, struct closing braces are indented, so the loop may continue past the struct and emit corrupted EVENT fields.
Suggested patch
- if (next_line ~ /^\}/) break
+ if (next_line ~ /^[[:space:]]*\}/) break
field_name = next_line
gsub(/^[[:space:]]+/, "", field_name)
- gsub(/:.*$/, "", field_name)
+ gsub(/:.*$/, "", field_name)
+ gsub(/,[[:space:]]*$/, "", field_name)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if ($0 ~ /\{[[:space:]]*$/) { | |
| fields = "" | |
| while ((getline next_line) > 0) { | |
| if (next_line ~ /^\}/) break | |
| field_name = next_line | |
| gsub(/^[[:space:]]+/, "", field_name) | |
| gsub(/:.*$/, "", field_name) | |
| if (length(field_name) > 0 && field_name !~ /^\/\//) { | |
| fields = fields (length(fields) > 0 ? "," : "") field_name | |
| } | |
| if ($0 ~ /\{[[:space:]]*$/) { | |
| fields = "" | |
| while ((getline next_line) > 0) { | |
| if (next_line ~ /^[[:space:]]*\}/) break | |
| field_name = next_line | |
| gsub(/^[[:space:]]+/, "", field_name) | |
| gsub(/:.*$/, "", field_name) | |
| gsub(/,[[:space:]]*$/, "", field_name) | |
| if (length(field_name) > 0 && field_name !~ /^\/\//) { | |
| fields = fields (length(fields) > 0 ? "," : "") field_name | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/generate-sui-metadata/scripts/extract_move_structure.sh
around lines 220 - 229, The multiline field-extraction loop in
extract_move_structure.sh incorrectly stops only for a `}` at column 0 (the
condition `next_line ~ /^\}/`), so indented closing braces are missed; update
the loop’s termination check to match optional leading whitespace (e.g. use
`next_line ~ /^[[:space:]]*\}/` or equivalent) so the `while ((getline
next_line) > 0)` loop breaks on indented `}` and prevents emitting corrupted
EVENT fields (adjust the same pattern if there are other `}` checks near the
`fields`, `field_name`, and `gsub` logic).
Comment on lines
+114
to
+115
| PR_BASE_REF="$(jq -r '.baseRefName' <<<"$PR_JSON")" | ||
| PR_TITLE="$(jq -r '.title' <<<"$PR_JSON")" |
There was a problem hiding this comment.
Base-branch fallback is non-reproducible due to moving ref.
Using baseRefName can fetch different files over time as the branch advances. For deterministic staging, use the PR base commit SHA instead.
Proposed fix
- PR_JSON="$(gh pr view "$SOURCE_URL" \
- --json files,headRefOid,baseRefName,title \
+ PR_JSON="$(gh pr view "$SOURCE_URL" \
+ --json files,headRefOid,baseRefOid,baseRefName,title \
2>/dev/null)" || {
@@
- PR_BASE_REF="$(jq -r '.baseRefName' <<<"$PR_JSON")"
+ PR_BASE_REF="$(jq -r '.baseRefOid' <<<"$PR_JSON")"Also applies to: 193-194
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/generate-sui-metadata/scripts/stage_sources.sh around lines
114 - 115, The script currently assigns PR_BASE_REF from PR_JSON using
.baseRefName which is mutable; replace those assignments to capture the PR base
commit SHA instead (e.g., use .baseRefOid or the JSON key that holds the base
commit SHA) and store it in a variable (rename or add PR_BASE_SHA or overwrite
PR_BASE_REF with the SHA) so checkout/fetch operations are deterministic; update
both occurrences (the block around PR_BASE_REF="$(jq -r '.baseRefName'
<<<"$PR_JSON")" and the similar assignments at lines referenced 193-194) and
ensure any later git commands use the SHA variable rather than the branch name.
Comment on lines
+117
to
+125
| mapfile -t CANDIDATES < <( | ||
| jq -r '.files[].path' <<<"$PR_JSON" \ | ||
| | grep -E '^contracts/[^/]+/sources/.+\.move$' \ | ||
| || true | ||
| ) | ||
|
|
||
| if [[ "${#CANDIDATES[@]}" -eq 0 ]]; then | ||
| echo "ERR|PR $SOURCE_URL does not touch any contracts/<pkg>/sources/<name>.move file" >&2 | ||
| exit 4 |
There was a problem hiding this comment.
PR candidate path filter excludes non-contracts/ packages.
This regex only accepts contracts/<pkg>/sources/..., so PRs touching math/.../sources/... are incorrectly reported as having no modules.
Proposed fix
mapfile -t CANDIDATES < <(
jq -r '.files[].path' <<<"$PR_JSON" \
- | grep -E '^contracts/[^/]+/sources/.+\.move$' \
+ | grep -E '^[^/]+(/[^/]+)?/sources/.+\.move$' \
|| true
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| mapfile -t CANDIDATES < <( | |
| jq -r '.files[].path' <<<"$PR_JSON" \ | |
| | grep -E '^contracts/[^/]+/sources/.+\.move$' \ | |
| || true | |
| ) | |
| if [[ "${#CANDIDATES[@]}" -eq 0 ]]; then | |
| echo "ERR|PR $SOURCE_URL does not touch any contracts/<pkg>/sources/<name>.move file" >&2 | |
| exit 4 | |
| mapfile -t CANDIDATES < <( | |
| jq -r '.files[].path' <<<"$PR_JSON" \ | |
| | grep -E '^[^/]+(/[^/]+)?/sources/.+\.move$' \ | |
| || true | |
| ) | |
| if [[ "${`#CANDIDATES`[@]}" -eq 0 ]]; then | |
| echo "ERR|PR $SOURCE_URL does not touch any contracts/<pkg>/sources/<name>.move file" >&2 | |
| exit 4 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/generate-sui-metadata/scripts/stage_sources.sh around lines
117 - 125, The candidate file filter only matches paths under
contracts/<pkg>/sources and thus misses other top-level package directories
(e.g., math/<pkg>/sources); update the grep regex used when populating
CANDIDATES in the mapfile pipeline to accept any top-level package name by
replacing the '^contracts/[^/]+/sources/.+\.move$' pattern with a generalized
pattern such as '^[^/]+/[^/]+/sources/.+\.move$' so that the CANDIDATES array
(populated in the mapfile -t CANDIDATES < <(...) block) includes sources from
non-`contracts` packages as well.
Comment on lines
+380
to
+383
| - must: "To call grant_role / revoke_role / set_role_admin / new_auth on Role R, the caller must currently hold the admin role of R (defaults to RootRole). Root-role flows require the current root holder. assert_has_role checks membership of an arbitrary address and aborts EUnauthorized when that address does not hold the queried role." | ||
| fails_with: EUnauthorized | ||
| affects: [grant_role, revoke_role, set_role_admin, new_auth, assert_has_role, begin_default_admin_transfer, begin_default_admin_renounce, cancel_default_admin_transfer, accept_default_admin_renounce, begin_default_admin_delay_change, cancel_default_admin_delay_change] | ||
|
|
There was a problem hiding this comment.
Complete EUnauthorized audit grounding coverage list.
Line 380 states EUnauthorized affects multiple root-flow APIs, but Lines 510–519 only enumerate a subset of corresponding tests. This weakens audit traceability in this metadata file.
Suggested patch
- precondition_fails_with: EUnauthorized
test_file: contracts/access/tests/access_control_tests.move
test_functions:
- test_grant_role_rejects_non_admin
- test_revoke_role_rejects_non_admin
- test_set_role_admin_rejects_non_admin
- test_new_auth_aborts_for_non_member
- test_assert_has_role_aborts_for_non_member
- test_begin_admin_transfer_rejects_non_root
+ - test_begin_admin_renounce_rejects_non_root
+ - test_cancel_admin_transfer_rejects_non_root
+ - test_accept_admin_renounce_rejects_non_root
+ - test_begin_delay_change_rejects_non_root
+ - test_cancel_delay_change_rejects_non_rootAlso applies to: 510-519
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@contracts/access/llms/access_control.yaml` around lines 380 - 383, The
EUnauthorized coverage list is incomplete—update the audit grounding entries
that enumerate which APIs are affected so they exactly match the "fails_with:
EUnauthorized" declaration; add all referenced symbols (grant_role, revoke_role,
set_role_admin, new_auth, assert_has_role, begin_default_admin_transfer,
begin_default_admin_renounce, cancel_default_admin_transfer,
accept_default_admin_renounce, begin_default_admin_delay_change,
cancel_default_admin_delay_change and any other root-flow APIs mentioned) to the
coverage block (the list around lines 510–519) so the metadata fully mirrors the
fails_with list and maintains one-to-one traceability with EUnauthorized.
Comment on lines
+349
to
+354
| - id: default-rounding-without-thinking | ||
| severity: medium | ||
| description: "Pass `rounding::nearest()` (or any single mode) to every `mul_div` / `sqrt` / `log*` call without considering who absorbs the remainder." | ||
| why_bad: "Rounding direction determines which side of the transaction wins on fractional remainders. A protocol that rounds up on deposits and up on withdrawals leaks value on every round-trip — exploitable by repeated small operations." | ||
| fix: "Pick rounding per call. Round AGAINST the user on deposits-into-vault and FOR the user on withdrawals-from-vault, so the protocol always keeps the remainder. Make the choice visible in code review." | ||
| example_bad: | |
There was a problem hiding this comment.
Fix contradictory rounding-direction guidance in anti-pattern remediation.
The fix text says to round for the user on withdrawals, but the same section (and example below) says the protocol should keep remainder (round down both directions in vault flows). This contradiction can drive unsafe implementation choices.
Proposed wording fix
- fix: "Pick rounding per call. Round AGAINST the user on deposits-into-vault and FOR the user on withdrawals-from-vault, so the protocol always keeps the remainder. Make the choice visible in code review."
+ fix: "Pick rounding per call. In vault/share accounting, round AGAINST the user on both deposit and withdrawal paths (typically `rounding::down()`) so the protocol keeps the remainder. Make the policy explicit in code review."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - id: default-rounding-without-thinking | |
| severity: medium | |
| description: "Pass `rounding::nearest()` (or any single mode) to every `mul_div` / `sqrt` / `log*` call without considering who absorbs the remainder." | |
| why_bad: "Rounding direction determines which side of the transaction wins on fractional remainders. A protocol that rounds up on deposits and up on withdrawals leaks value on every round-trip — exploitable by repeated small operations." | |
| fix: "Pick rounding per call. Round AGAINST the user on deposits-into-vault and FOR the user on withdrawals-from-vault, so the protocol always keeps the remainder. Make the choice visible in code review." | |
| example_bad: | | |
| - id: default-rounding-without-thinking | |
| severity: medium | |
| description: "Pass `rounding::nearest()` (or any single mode) to every `mul_div` / `sqrt` / `log*` call without considering who absorbs the remainder." | |
| why_bad: "Rounding direction determines which side of the transaction wins on fractional remainders. A protocol that rounds up on deposits and up on withdrawals leaks value on every round-trip — exploitable by repeated small operations." | |
| fix: "Pick rounding per call. In vault/share accounting, round AGAINST the user on both deposit and withdrawal paths (typically `rounding::down()`) so the protocol keeps the remainder. Make the policy explicit in code review." | |
| example_bad: | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@math/core/llms/u32.yaml` around lines 349 - 354, The "fix" guidance for the
rule id default-rounding-without-thinking is self-contradictory; update the fix
text so it consistently prescribes one clear rounding policy (e.g., "Round
AGAINST the user on both deposits-into-vault and withdrawals-from-vault so the
protocol always keeps the remainder") and ensure the wording explicitly states
the direction for each flow and asks reviewers to make the choice visible in
code review.
Comment on lines
+54
to
+70
| quick_start: | | ||
| // u8 end-to-end — overflow-safe arithmetic in the typical 3-call shape. | ||
| // Choose a rounding mode, call the operation, and handle `Option<u8>` | ||
| // for paths that can overflow. | ||
|
|
||
| // 1. Multiply-then-divide with explicit rounding. Returns Option<u8>; | ||
| // aborts EDivideByZero only if `denominator == 0`. | ||
| let fee_quote = u8::mul_div(amount, 5u8, 100u8, rounding::nearest()); | ||
| let value = fee_quote.destroy_or!(abort EMathOverflow); | ||
|
|
||
| // 2. Square root rounded down (cannot overflow; always returns u8). | ||
| let root = u8::sqrt(value, rounding::down()); | ||
|
|
||
| // 3. Lossless left shift; returns None if non-zero bits would be lost. | ||
| let scaled = u8::checked_shl(root, 2u8); | ||
| assert!(scaled.is_some(), EMathOverflow); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
from pathlib import Path
text = Path("math/core/llms/u8.yaml").read_text()
start = text.index("quick_start: |")
end = text.index("# ============================================================", start)
qs = text[start:end]
print("uses u8 module:", "u8::" in qs)
print("uses rounding module:", "rounding::" in qs)
print("uses EMathOverflow:", "EMathOverflow" in qs)
print("declares import:", "use openzeppelin_math::{u8, rounding};" in qs)
print("declares EMathOverflow const:", "const EMathOverflow" in qs)
PYRepository: OpenZeppelin/contracts-sui
Length of output: 202
Make math/core/llms/u8.yaml quick_start self-contained (lines 54-70): the snippet uses rounding::... and EMathOverflow but doesn’t declare use openzeppelin_math::{u8, rounding}; or the #[error] const EMathOverflow inside the block, so copying just quick_start will fail symbol resolution.
Proposed fix
quick_start: |
// u8 end-to-end — overflow-safe arithmetic in the typical 3-call shape.
// Choose a rounding mode, call the operation, and handle `Option<u8>`
// for paths that can overflow.
+
+ use openzeppelin_math::{u8, rounding};
+
+ #[error(code = 0)]
+ const EMathOverflow: vector<u8> = "arithmetic overflow";
// 1. Multiply-then-divide with explicit rounding. Returns Option<u8>;
// aborts EDivideByZero only if `denominator == 0`.
let fee_quote = u8::mul_div(amount, 5u8, 100u8, rounding::nearest());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| quick_start: | | |
| // u8 end-to-end — overflow-safe arithmetic in the typical 3-call shape. | |
| // Choose a rounding mode, call the operation, and handle `Option<u8>` | |
| // for paths that can overflow. | |
| // 1. Multiply-then-divide with explicit rounding. Returns Option<u8>; | |
| // aborts EDivideByZero only if `denominator == 0`. | |
| let fee_quote = u8::mul_div(amount, 5u8, 100u8, rounding::nearest()); | |
| let value = fee_quote.destroy_or!(abort EMathOverflow); | |
| // 2. Square root rounded down (cannot overflow; always returns u8). | |
| let root = u8::sqrt(value, rounding::down()); | |
| // 3. Lossless left shift; returns None if non-zero bits would be lost. | |
| let scaled = u8::checked_shl(root, 2u8); | |
| assert!(scaled.is_some(), EMathOverflow); | |
| quick_start: | | |
| // u8 end-to-end — overflow-safe arithmetic in the typical 3-call shape. | |
| // Choose a rounding mode, call the operation, and handle `Option<u8>` | |
| // for paths that can overflow. | |
| use openzeppelin_math::{u8, rounding}; | |
| #[error(code = 0)] | |
| const EMathOverflow: vector<u8> = "arithmetic overflow"; | |
| // 1. Multiply-then-divide with explicit rounding. Returns Option<u8>; | |
| // aborts EDivideByZero only if `denominator == 0`. | |
| let fee_quote = u8::mul_div(amount, 5u8, 100u8, rounding::nearest()); | |
| let value = fee_quote.destroy_or!(abort EMathOverflow); | |
| // 2. Square root rounded down (cannot overflow; always returns u8). | |
| let root = u8::sqrt(value, rounding::down()); | |
| // 3. Lossless left shift; returns None if non-zero bits would be lost. | |
| let scaled = u8::checked_shl(root, 2u8); | |
| assert!(scaled.is_some(), EMathOverflow); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@math/core/llms/u8.yaml` around lines 54 - 70, The quick_start snippet is
missing imports and the error constant; make it self-contained by adding the
appropriate imports for rounding and u8 (e.g., use openzeppelin_math::{u8,
rounding};) and include or import the error constant used (EMathOverflow) so the
example compiles; update the snippet that exercises u8::mul_div, u8::sqrt, and
u8::checked_shl to either declare #[error] const EMathOverflow or import it from
its module so abort EMathOverflow and assert!(..., EMathOverflow) resolve.
Comment on lines
+307
to
+311
| Bit shift on raw bits, NOT a decimal multiply by `2^bits`. Aborts | ||
| `EInvalidShiftSize` if `bits >= 128`; aborts `EOverflow` if the | ||
| shift would consume non-zero high bits. For decimal `x * 2` use | ||
| `x.add(x)`, not `x.lshift(1)`. | ||
|
|
There was a problem hiding this comment.
Correct the shift semantics guidance for powers-of-two scaling.
Line 307 and Line 411 state lshift/rshift are not decimal * 2^k / / 2^k, but with fixed 10^9 scale they do scale decoded values by powers of two (subject to overflow/truncation). This conflicts with Line 474 and can mislead agents/integrators.
Suggested wording fix
- Bit shift on raw bits, NOT a decimal multiply by `2^bits`. Aborts
+ Bit shift on raw bits. In UD30x9, this corresponds to scaling the
+ decoded numeric value by `2^bits` (subject to overflow / truncation),
+ but it does NOT apply or remove the fixed-point `10^9` scale. Aborts
`EInvalidShiftSize` if `bits >= 128`; aborts `EOverflow` if the
shift would consume non-zero high bits. For decimal `x * 2` use
`x.add(x)`, not `x.lshift(1)`.- why: "`lshift` / `rshift` operate on RAW BITS — they multiply / divide the underlying `u128`, not the decimal value. `x.lshift(1)` is NOT `x * 2.0` in decimal terms."
+ why: "`lshift` / `rshift` operate on RAW BITS. In UD30x9 they correspond to `* 2^k` / floor(`/ 2^k`) on the decoded value, but they are not decimal-scale conversions (`* 10^k` / `/ 10^k`)."Also applies to: 407-412
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@math/fixed_point/llms/ud30x9/ud30x9_base.yaml` around lines 307 - 311, Update
the documentation text around the lshift/rshift guidance to say: lshift/rshift
operate on the raw integer representation (they perform bit shifts on the stored
fixed-point integer and will abort with EInvalidShiftSize or EOverflow as
described) — they are not decimal multiply/divide operations, but because this
implementation uses a fixed 10^9 scale, a bit shift on the raw bits will
correspond to multiplying/dividing the decoded value by powers of two (subject
to overflow/truncation); for pure decimal scaling by 2 use x.add(x) or proper
decimal multiply routines instead of lshift(1). Replace the ambiguous sentences
in the blocks mentioning lshift/rshift (the passages around the current
lshift/rshift guidance at lines ~307, ~407-412, and the contradictory note near
~474) with this clarified wording and keep references to errors
EInvalidShiftSize and EOverflow.
Comment on lines
+192
to
+201
| "required": [ | ||
| "code", | ||
| "message" | ||
| ], | ||
| "additionalProperties": false, | ||
| "properties": { | ||
| "code": { | ||
| "type": "integer", | ||
| "minimum": 0 | ||
| }, |
There was a problem hiding this comment.
errors.code is over-constrained and will reject valid legacy metadata.
code is required and integer-only, but your own generator/spec path allows legacy entries with missing/unknown codes. This schema will fail those files.
Proposed schema fix
"additionalProperties": {
"type": "object",
- "required": [
- "code",
- "message"
- ],
+ "required": ["message"],
"additionalProperties": false,
"properties": {
"code": {
- "type": "integer",
- "minimum": 0
+ "oneOf": [
+ { "type": "integer", "minimum": 0 },
+ { "type": "null" }
+ ]
},
"message": {
"type": "string"
}
}
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "required": [ | |
| "code", | |
| "message" | |
| ], | |
| "additionalProperties": false, | |
| "properties": { | |
| "code": { | |
| "type": "integer", | |
| "minimum": 0 | |
| }, | |
| "required": ["message"], | |
| "additionalProperties": false, | |
| "properties": { | |
| "code": { | |
| "oneOf": [ | |
| { "type": "integer", "minimum": 0 }, | |
| { "type": "null" } | |
| ] | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@schemas/llms-metadata-v1.json` around lines 192 - 201, The errors.code
property is currently required and integer-only which will reject legacy
metadata; update the schema so "code" is not mandatory under the errors object
(remove "code" from the "required" array) and relax its type to accept null as
well as integer (e.g., "type": ["integer","null"]) so missing/unknown codes are
allowed while preserving integer validation for present values; locate the
errors object and the "code" property in the schema (errors.code) to make these
changes.
Comment on lines
+323
to
+329
| "composes_with": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| }, | ||
| "description": "Modules this one composes with in real protocols. Empty array is a valid value; populate as composition patterns are validated. Cross-module composability queries are the natural fit for an MCP server \u2014 see the project's MCP roadmap." | ||
| }, |
There was a problem hiding this comment.
composes_with schema shape does not match the documented object contract.
It is currently string[], but the format spec requires structured entries (module, when, source, example). This blocks valid metadata and weakens source-citation guarantees.
Proposed schema fix
"composes_with": {
"type": "array",
"items": {
- "type": "string"
+ "type": "object",
+ "required": ["module", "when", "source", "example"],
+ "additionalProperties": false,
+ "properties": {
+ "module": { "type": "string" },
+ "when": { "type": "string" },
+ "source": { "type": "string" },
+ "example": { "type": "string" }
+ }
},
"description": "Modules this one composes with in real protocols. Empty array is a valid value; populate as composition patterns are validated. Cross-module composability queries are the natural fit for an MCP server — see the project's MCP roadmap."
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "composes_with": { | |
| "type": "array", | |
| "items": { | |
| "type": "string" | |
| }, | |
| "description": "Modules this one composes with in real protocols. Empty array is a valid value; populate as composition patterns are validated. Cross-module composability queries are the natural fit for an MCP server \u2014 see the project's MCP roadmap." | |
| }, | |
| "composes_with": { | |
| "type": "array", | |
| "items": { | |
| "type": "object", | |
| "required": ["module", "when", "source", "example"], | |
| "additionalProperties": false, | |
| "properties": { | |
| "module": { "type": "string" }, | |
| "when": { "type": "string" }, | |
| "source": { "type": "string" }, | |
| "example": { "type": "string" } | |
| } | |
| }, | |
| "description": "Modules this one composes with in real protocols. Empty array is a valid value; populate as composition patterns are validated. Cross-module composability queries are the natural fit for an MCP server — see the project's MCP roadmap." | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@schemas/llms-metadata-v1.json` around lines 323 - 329, The "composes_with"
property currently defines items as "type": "string" but must be an array of
structured objects per the documented contract; update the "composes_with"
schema in schemas/llms-metadata-v1.json so each item is an object with
properties "module" (string), "when" (string, optional/nullable), "source"
(object with at least "type"/"uri" or simple string — choose the project's
citation shape), and "example" (string, optional), and mark "module" as
required; also provide appropriate types/formats and a description for each
sub-property to enforce source-citation and composition metadata.
0xNeshi
reviewed
May 28, 2026
| | [`access/`](access/) | Transfer policies that wrap privileged capabilities and guard ownership handoffs (two-step approvals and time-locked transfers). | | ||
| | Name | Path | Highlights | | ||
| |------|------|------------| | ||
| | `openzeppelin_access` | [`access/`](access/) | Role-based access control plus controlled ownership-transfer policies (two-step approvals, time-locked transfers). | |
Collaborator
There was a problem hiding this comment.
Update branch and add openzeppelin_utils and related AI metadata
This was referenced Jun 15, 2026
Collaborator
Author
|
Closing without merge — superseded by the V2 approach to AI-integrator metadata. V1 here built a parallel per-module metadata layer (llms/*.yaml + JSON schema + 3 validators + generator). It works, but it is a second copy of what already lives in source, docs, and examples, so it drifts (CI catches structural, not content, drift). V2 removes the parallel layer: metadata lives in its natural single sources of truth — compilable Tracked as epic #375 — delivered/in-review: #380 (PR #390), #381 (PR #391), #385 (PR #392, completed); remaining: #386 examples. Rationale: see the V2 decision note. The V1 empirical result (an agent built a working contract from metadata alone) stands as evidence the direction works — the heavy implementation is simply not the path. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a complete AI-integrator metadata layer for OpenZeppelin Contracts for Sui: discovery files, schema-validated per-module metadata, a generator skill, three validation layers, and CI. The goal is to let AI coding agents (Cursor, Claude Code, Cline, Copilot, Aider, Gemini CLI) that consume this library ground their code generation at the schema level — instead of re-deriving structure from Move source on every call.
This is integrator-facing metadata (downstream consumers), distinct from contributor docs.
Discovery
llms.txt(root) — narrative entry point (llmstxt.org convention) pointing at the catalog, per-package indexes, MVR, audits, and schemas.llms/index.yaml(root catalog) — machine-readable enumeration of every package (name, group, MVR slug, path to each package index). Single-fetchlist_packagesentry for MCP servers / agents.<pkg>/llms/index.yaml(per package) — enumerates the package's module YAMLs (name, path, one-line summary).<pkg>/llms/<...>/<module>.yaml(per module) — full schema-level metadata: API signatures, errors, types, preconditions, decisions (which function to pick), anti-patterns (do_not[]withexample_bad/example_good+ detection heuristics), and an opt-in_audit_groundingblock (canonical test, precondition proofs).Coverage
100% of public modules — 19 module YAMLs across 3 packages (
openzeppelin_access×3,openzeppelin_math×10,openzeppelin_fp_math×6) + 3 package indexes + 1 root catalog. Private helpers undersources/internal/are intentionally excluded (not integrator-facing).Schemas (public, v1.0)
schemas/llms-metadata-v1.json— per-module YAML (JSON Schema Draft 2020-12, strictadditionalProperties: false).schemas/llms-package-index-v1.json— aoneOfcovering both the root catalog (packages[]) and the per-package index (modules[]) forms.Package-level install fields (
mvr,repo_ref,release, Move.toml snippets) are hoisted into each packageindex.yaml; per-moduleinstall:carries onlyuse_statement— eliminates ~250 lines of duplication and the cross-filerepo_refdrift surface.Validation & CI — three layers
.github/workflows/llms-schema-validation.ymlruns on PRs touching metadata, schemas, validators, orsources/**:validate-llms-schema.py) — JSON Schema conformance; dispatches catalog/index/module by filename.validate-llms-semantics.py) — 7 cross-field referential-integrity checks:api[].aborts⊆errors,preconditions[].fails_with⊆errors,affects= exhaustive union, audit-grounding refs resolve,do_not_detection/do_not_demonstrationsids ⊆do_not[].id, andcanonical_testpoints at a realfunin the named test file.validate-llms-completeness.py) — whole-tree source↔metadata sync: every non-internalsources/**module (by declared module name, not file basename) has a YAML + index entry; no orphan YAMLs; all index paths resolve; the root catalog lists every package index. Always runs (even on a sources-only diff) so adding a module without its YAML is caught.Layers 1–2 are diff-scoped (full re-validation when a schema/validator/workflow changes). Current: 23/23 schema + 19/19 semantics + 3/3 packages complete.
Generator skill
.claude/skills/generate-sui-metadata/regenerates a module's YAML from its GitHub source + (optional) Notion R&D + docs PR. Extractors surface API/errors/types/events andDOC_GUIDANCEhazard doc-blocks (#### Security/#### Misuse/...) that deterministically drivedo_not[]. Includes operation-aware anti-pattern clustering and type-capability inference fordo_not[]id stability.Empirical validation
A fresh-context agent built a working contract using RBAC + fixed-point math + decimal_scaling from the metadata alone (no library source) —
sui move build --lintclean on the first try. Thedecisions[]/do_not[]fields demonstrably steered correct choices (e.g.from_u64notwrap,mul_awayfor fees, no redundant registry-ID comparison). A/move-qualitypass on the generated code surfaced a single idiom gap (error-const form), now fixed across the snippets.Follow-ups (out of scope for this PR)
/contracts-sui/llms.txton docs.openzeppelin.com, per-package MVRdocumentation_url, llmstxt.site.Summary by CodeRabbit
New Features
llms/directories, including API signatures, error codes, and security patterns.llms.txtandllms/index.yamldiscovery system for locating and understanding available packages and modules.Documentation