Semantic diff: commit records rendered as a page linked on each PR by cristianoc · Pull Request #8870 · rescript-lang/rescript

cristianoc · 2026-10-16T00:57:33Z

Review threads in this repository keep asking the same questions:

Is there a changelog entry, in the right category, with the right link? (Fix function assignment formatting and unify formatter paths #8611, Fix lost and unstable comments around callbacks #8627, Fix constant folding of overlapping unboxed variant matches #8631, Separate parser token inspection from consumption #8633, Represent regexp literals explicitly and remove %re syntax #8634, Skip rescript-schema-ppx and sury-ppx for unrelated sources #8662, Stabilize rewatch scheduling and integration tests #8667, Make inclusion error output independent of the source path #8691–Resolve RescriptTools.binaryPath in user projects #8694)
What is the generated JavaScript before and after? (Normalize string literal representation across the compiler #8606, Declare exports left unbound by a toplevel throw #8692, Small compiler and rewatch performance fixes #8769, Find live toplevel bindings in Js_shake with a worklist #8773, Bound how deep inlining nests #8801)
Does rescript format rewrite existing code, and does it round-trip? (Fix function assignment formatting and unify formatter paths #8611, Print coercions with context-aware parentheses #8614, Fix lost and unstable comments around callbacks #8627, Represent ternary expressions with a dedicated parsetree node #8674, Format switch case blocks without redundant braces #8677)
Does a marshaled format change without a magic-number bump? (Normalize string literal representation across the compiler #8606, Remove unused Ppat_open from the current AST #8682, Represent dict literals with Pexp_dict and Texp_dict nodes #8746)
Do the frozen v0 PPX wire and editor analysis still work? (Normalize string literal representation across the compiler #8606, Make @as part of the AST #8619, Separate parser token inspection from consumption #8633, Represent dict literals with Pexp_dict and Texp_dict nodes #8746)
Which tests pin the change, and which suites ran? (Stabilize rewatch scheduling and integration tests #8667, Declare exports left unbound by a toplevel throw #8692, Stop measuring lambda size once the inlining limit is reached #8771, Find live toplevel bindings in Js_shake with a worklist #8773)

This PR answers them in the commit itself, and shows the answers as a page linked from every PR.

Commit records. .claude/skills/records/SKILL.md defines a commit message format with prose sections (Example (rescript), Before (js) / After (js), Source, Breaking, Changelog entry, Review) and git trailers (Area, Changelog, Output, Formatting, Formats, PPX, Editor, Errors, Tests, Measured). For each field it states the concrete check, so an agent needs nothing beyond the skill. Tests lists the tests added or changed, or says none, because …. Coding agents load it as a skill; for people it is a plain document, and AGENTS.md points to it.
Semantic diff page. scripts/commitRecords.js renders a range of commits as one page with a card per commit: the ReScript example beside the JavaScript before and after, the changelog category and entry, the effects, and tests by suite. Each card is checked against its own diff: the changelog fragment, modified generated JavaScript, magic-number bumps, v0 PPX bridge fixtures, and ERROR_VARIANTS.md. A range adds a summary, a changelog draft, and the files that more than one commit changes. The script reads only git and has no dependencies. Locally, make semantic-diff renders master..HEAD and opens the page.
A link on every PR. .github/workflows/semantic_diff.yml renders a PR's commits when the PR is opened or pushed, independently of CI, uploads the page to cdn.rescript-lang.org/semantic-diff/pr-<N>/, and posts the link as a comment that replaces the previous one, so the newest link is the last comment. Closing the PR deletes the page. A PR from a branch of this repository runs on pull_request with its own workflow, from its first push. A PR from a fork runs on pull_request_target, which runs only master's code: the fork's commits are fetched as git objects and never checked out.

Demo: #8871, stacked on this PR, is a small codegen change written by an agent with a record as its commit message. Both PRs get their semantic diff link as a comment from the workflow added here.

Everything is opt-in. Nothing enforces the format, and a commit without a record still renders, with its card flagging the missing Changelog trailer. The main question is which fields are worth keeping.

Test plan

node --test scripts/commitRecords.test.js: 3 passing. The tests render records from a temporary repository and check the cards and the diff checks.
npx biome check on the scripts: no findings. npx tsc: exit 0.
make semantic-diff SEMANTIC_DIFF_BASE=origin/master: the page is written and opened.
Dry run of the workflow's render step in a fresh --filter=blob:none clone, against refs/pull/8862/head: the page renders. The upload, comment and cleanup steps run only on GitHub, on this PR and Compile integer operations with an identity operand to the operand #8871.

🤖 Generated with Claude Code

Integer `x + 0`, `0 + x`, `x - 0`, `x * 1`, `1 * x` and `x / 1` compile to `x`. A ReScript `int` value is a 32-bit integer in JavaScript, so the `| 0` truncation (or `<< 0` for `* 1`) that these operations emitted leaves the value unchanged. The JavaScript expression builders in `Js_exp_make` (`int32_add`, `int32_minus`, `int32_mul`, `int32_div`) return the other operand when one operand is the literal identity. The dropped operand is a literal, so evaluation of the kept operand is unchanged. Example (rescript): let addZero = (x: int) => x + 0 let subZero = (x: int) => x - 0 let mulOne = (x: int) => x * 1 let divOne = (x: int) => x / 1 Before (js): function addZero(x) { return x + 0 | 0; } function subZero(x) { return x - 0 | 0; } function mulOne(x) { return (x << 0); } function divOne(x) { return x / 1 | 0; } After (js): function addZero(x) { return x; } function subZero(x) { return x; } function mulOne(x) { return x; } function divOne(x) { return x; } Source: `bsc Example.res` (CommonJS, to stdout) with bsc built from aa2078e (Before) and from this commit (After); function bodies shown, the module header and exports are identical. Changelog entry: - Compile integer `x + 0`, `0 + x`, `x - 0`, `x * 1`, `1 * x` and `x / 1` to `x`. #8871 Review: Can an `int` operand hold a value outside the 32-bit range? Every int the compiler computes is already truncated: compiling `-x`, `Int.fromFloat(f)` and `Int.shiftRightUnsigned(x, y)` with this commit gives `-x | 0`, `f | 0` and `(x >>> y) | 0`. A value outside the range reaches an `int` only through an external or `%raw` typed `int`; for those the result changes, for example `Number.MAX_SAFE_INTEGER + 0` gave -1 and gives 9007199254740991. The compiler already relies on the same assumption: `Plsrint` by the literal 0 compiles to the operand (compiler/core/lam_compile_primitive.ml, `[e1; 0] -> e1`), dropping `(x >>> 0) | 0`. Review: Does `x / 1` still check for division by zero? The divisor is the literal 1, so no check applies; the existing rule for a nonzero literal divisor already skipped the runtime `div` call. Area: codegen tests Changelog: polish Output: changed integer `+ 0`, `0 +`, `- 0`, `* 1`, `1 *`, `/ 1` on a non-literal operand, now the bare operand; in tests/tests/src one existing snapshot changes (gpr_3609_test.mjs, `0 + state._0 | 0` -> `state._0`), and the stdlib build (make lib) emits no changed JS Formats: none Tests: +tests/tests/src/int_identity_ops_test.res +tests/tests/src/int_identity_ops_test.mjs ~tests/tests/src/gpr_3609_test.mjs Ran: make test on bc2e0e1 + this stack (exit 0: 318 ounit tests OK, tests/tests mocha 606 passing including Int_identity_ops_test, stdlib mocha 744 passing); make checkformat (exit 0); yarn changelog:check (passes) Depends: #8870 (stacked) PR: #8871 Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

pkg-pr-new · 2026-10-16T01:19:10Z

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript@8870

@rescript/belt

npm i https://pkg.pr.new/@rescript/belt@8870

@rescript/darwin-arm64

npm i https://pkg.pr.new/@rescript/darwin-arm64@8870

@rescript/darwin-x64

npm i https://pkg.pr.new/@rescript/darwin-x64@8870

@rescript/linux-arm64

npm i https://pkg.pr.new/@rescript/linux-arm64@8870

@rescript/linux-x64

npm i https://pkg.pr.new/@rescript/linux-x64@8870

@rescript/runtime

npm i https://pkg.pr.new/@rescript/runtime@8870

@rescript/win32-x64

npm i https://pkg.pr.new/@rescript/win32-x64@8870

commit: fb7a59b

github-actions · 2026-10-16T01:21:24Z

Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-8870

Integer `x + 0`, `0 + x`, `x - 0`, `x * 1`, `1 * x` and `x / 1` compile to `x`. A ReScript `int` value is a 32-bit integer in JavaScript, so the `| 0` truncation (or `<< 0` for `* 1`) that these operations emitted leaves the value unchanged. The JavaScript expression builders in `Js_exp_make` (`int32_add`, `int32_minus`, `int32_mul`, `int32_div`) return the other operand when one operand is the literal identity. The dropped operand is a literal, so evaluation of the kept operand is unchanged. Example (rescript): let addZero = (x: int) => x + 0 let subZero = (x: int) => x - 0 let mulOne = (x: int) => x * 1 let divOne = (x: int) => x / 1 Before (js): function addZero(x) { return x + 0 | 0; } function subZero(x) { return x - 0 | 0; } function mulOne(x) { return (x << 0); } function divOne(x) { return x / 1 | 0; } After (js): function addZero(x) { return x; } function subZero(x) { return x; } function mulOne(x) { return x; } function divOne(x) { return x; } Source: `bsc Example.res` (CommonJS, to stdout) with bsc built from aa2078e (Before) and from this commit (After); function bodies shown, the module header and exports are identical. Changelog entry: - Compile integer `x + 0`, `0 + x`, `x - 0`, `x * 1`, `1 * x` and `x / 1` to `x`. #8871 Review: Can an `int` operand hold a value outside the 32-bit range? Every int the compiler computes is already truncated: compiling `-x`, `Int.fromFloat(f)` and `Int.shiftRightUnsigned(x, y)` with this commit gives `-x | 0`, `f | 0` and `(x >>> y) | 0`. A value outside the range reaches an `int` only through an external or `%raw` typed `int`; for those the result changes, for example `Number.MAX_SAFE_INTEGER + 0` gave -1 and gives 9007199254740991. The compiler already relies on the same assumption: `Plsrint` by the literal 0 compiles to the operand (compiler/core/lam_compile_primitive.ml, `[e1; 0] -> e1`), dropping `(x >>> 0) | 0`. Review: Does `x / 1` still check for division by zero? The divisor is the literal 1, so no check applies; the existing rule for a nonzero literal divisor already skipped the runtime `div` call. Area: codegen tests Changelog: polish Output: changed integer `+ 0`, `0 +`, `- 0`, `* 1`, `1 *`, `/ 1` on a non-literal operand, now the bare operand; in tests/tests/src one existing snapshot changes (gpr_3609_test.mjs, `0 + state._0 | 0` -> `state._0`) and in the gentype example Docstrings.res.js six `+ 0` functions drop the addition (`a + 0 | 0` -> `a`), and the stdlib build (make lib) emits no changed JS Formats: none Tests: +tests/tests/src/int_identity_ops_test.res +tests/tests/src/int_identity_ops_test.mjs ~tests/tests/src/gpr_3609_test.mjs ~tests/gentype_tests/typescript-react-example/src/Docstrings.res.js Ran: make test on bc2e0e1 + this stack (exit 0: 318 ounit tests OK, tests/tests mocha 606 passing including Int_identity_ops_test, stdlib mocha 744 passing); make checkformat (exit 0); yarn changelog:check (passes); make test-gentype, test-analysis, test-tools (exit 0); make test-rewatch fails locally on a temp-directory symlink (Path is a directory: rescript-gentype-root-*-link), with no generated JS change Depends: #8870 (stacked) PR: #8871 Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

cknitt

Awesome feature! Some comments:

cknitt · 2026-10-16T06:12:29Z

+
+| Field | The question it answers | Where reviewers asked it |
+| --- | --- | --- |
+| `Area` | Which layer changed, so who reviews it and which suites apply | AGENTS.md routes by layer; AST changes broke editor analysis in #8606, #8619, #8633, #8746 |


I think it's not relevant for this skill to list issue numbers where a question was asked.

Or if any content from these PR reviews is indeed relevant, we should inline it here, the agent should not have to look up those PRs every time.

Agreed. The table now has two columns: the field, and what the record states, written as the concrete check (for example Formatting: whether rescript format rewrites existing code, and whether formatted output parses back to the same tree with its comments). The PR numbers are gone, so an agent needs nothing beyond the skill.

cknitt · 2026-10-16T06:15:58Z

+| `Editor` | Do completion, hover and inlay hints still work on the new nodes | #8606, #8619, #8746 |
+| `Errors` | Is `tests/ERROR_VARIANTS.md` in step with the fixtures | #8746 |
+| `Tests` | Which tests pin the change, and do they observe the claimed effect | missing coverage in #8667, #8771, #8773; assertion too weak in #8667; widths in #8677 |
+| `Ran` | Which suites ran | PR test plans (#8692) and replies ("syntax and roundtrip suites pass", #8674) |


I don't get the point of this. Which tests were run and passed (in CI) can be seen in the PR itself.

It would be more interesting which tests were added or modified. And if no tests were added, why not.

Agreed. Ran is removed. Tests is now required: it lists the tests added, changed or removed, or says Tests: none, because …. The page flags a record with no Tests trailer, and a Tests: none whose diff does change tests.

cknitt · 2026-10-16T06:17:20Z

+| `Tests` | Which tests pin the change, and do they observe the claimed effect | missing coverage in #8667, #8771, #8773; assertion too weak in #8667; widths in #8677 |
+| `Ran` | Which suites ran | PR test plans (#8692) and replies ("syntax and roundtrip suites pass", #8674) |
+| `Measured` | Performance before and after, on what input | #8769, #8771, #8773, #8801 |
+| `Depends` | Stacking and conflicts with open PRs | #8606 (conflict with #8608), #8771, #8773 (stacked) |


Stacking can also be seen directly in the PR.
And this information will get out of date quickly when PRs are rebased, merged, unstacked/restacked, ...

Agreed. Depends is removed.

cknitt · 2026-10-16T06:17:42Z

+| `Measured` | Performance before and after, on what input | #8769, #8771, #8773, #8801 |
+| `Depends` | Stacking and conflicts with open PRs | #8606 (conflict with #8608), #8771, #8773 (stacked) |
+| `PR` | The discussion behind the change | the renderer links it |
+| `Signed-off-by` | DCO | the most frequent automated finding: #8691, #8729, #8773, #8837, #8842, #8846, #8857 |


Would put that at the end of the table (as it actually needs to be add the end of the commit message).

Done: Signed-off-by is the last row of the table.

cristianoc · 2026-10-16T07:09:17Z

@cknitt this is a good iteration, if there's anything else to cut down we can do it now.
There's always time to add more later.

codecov · 2026-10-16T07:17:45Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.02%. Comparing base (bc2e0e1) to head (fb7a59b).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8870      +/-   ##
==========================================
+ Coverage   80.99%   81.02%   +0.02%     
==========================================
  Files         453      453              
  Lines       62989    63030      +41     
==========================================
+ Hits        51020    51070      +50     
+ Misses      11969    11960       -9

see 7 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

cknitt · 2026-10-16T07:32:56Z

Ah something else: The page header is always

Semantic Diff
x commits of the ReScript compiler, rendered from their records.

It would be more interesting to see the PR number and title in the header.

A commit record is a commit message whose prose sections and git trailers answer the questions reviewers of this repository ask on pull requests: which layer changed, the changelog category and entry, the generated JavaScript before and after, formatter, marshaled-format, PPX and editor effects, error-catalog rows, and the tests added or changed (or why none). .claude/skills/records/SKILL.md defines the format and what each field states. scripts/commitRecords.js renders a semantic diff page: one card per record, checks each record against its own diff (changelog fragment, modified generated JavaScript, magic-number bumps, v0 PPX bridge fixtures, ERROR_VARIANTS.md, the Tests trailer), and for a range adds a summary, a changelog draft and the files that more than one commit changes. `--pr` and `--title` head the page with the pull request. The page is one HTML file with no script. `make semantic-diff` renders master..HEAD to _build/semantic-diff.html and opens it. Example (sh): make semantic-diff node scripts/commitRecords.js --out review.html master..my-branch Changelog entry: - Add commit records, a commit message format that answers recurring review questions, and a semantic diff page rendered from them and linked on each pull request. #8870 Review: Does the renderer need the compiler? No: it reads git (messages, notes on refs/notes/records, and diffs) and has no dependencies. Area: scripts tests docs ci Changelog: internal Output: unchanged Tests: +scripts/commitRecords.test.js PR: #8870 Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

When a pull request is opened, reopened or pushed, the Semantic Diff workflow renders its commits with scripts/commitRecords.js, headed by the pull request number and title, uploads the page to the CDN at semantic-diff/pr-<N>/index.html, and posts its link as a comment that replaces the previous one, so the newest link is the last comment on the pull request. The link carries the rendered head commit as a query string. Closing the pull request deletes the page. A pull request from a branch of this repository runs on pull_request, with its own workflow and renderer, from its first push and on any base branch. A pull request from a fork runs on pull_request_target. Security: pull_request_target takes the workflow from the default branch and runs it with the repository's secrets, so for forks the job runs only the default branch's code. It checks out the base commit without persisted credentials, fetches refs/pull/<N>/head as git objects, and renders them with the base branch's renderer, which reads git data and escapes all text it writes into the page; the title reaches the shell only through an environment variable. Each pull request runs on one of the two events: the jobs compare the event with the head repository, and the concurrency group names the event. Review: Why the CDN and not GitHub Pages? Pages here is one deployment from master (the dev playground) that each deploy replaces whole; the CDN already holds the playground's per-pull-request bundles under dev-playground-bundles/pr-<N>/. Area: ci docs Changelog: none Output: unchanged Tests: none, because the workflow runs only on GitHub; its render step is the renderer that scripts/commitRecords.test.js covers PR: #8870 Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

cristianoc · 2026-10-16T07:51:41Z

@cknitt Done: the page is now headed by the PR number (linked) and title, and the browser tab shows them too. Locally, make semantic-diff uses the branch name. The links in the Semantic diff comments on #8870 and #8871 show it.

github-actions · 2026-10-16T07:52:02Z

Semantic diff: https://cdn.rescript-lang.org/semantic-diff/pr-8870/index.html?v=fb7a59bd8

cknitt

Looking at the finished HTML report in detail, I still have some minor nits, but I can do a follow-up PR on those later.

cristianoc · 2026-10-16T14:33:53Z

Looking at the finished HTML report in detail, I still have some minor nits, but I can do a follow-up PR on those later.

Let's put it to test and see how it behaves!

A commit record is a commit message whose prose sections and git trailers answer the questions reviewers of this repository ask on pull requests: which layer changed, the changelog category and entry, the generated JavaScript before and after, formatter, marshaled-format, PPX and editor effects, error-catalog rows, and the tests added or changed (or why none). .claude/skills/records/SKILL.md defines the format and what each field states. scripts/commitRecords.js renders a semantic diff page: one card per record, checks each record against its own diff (changelog fragment, modified generated JavaScript, magic-number bumps, v0 PPX bridge fixtures, ERROR_VARIANTS.md, the Tests trailer), and for a range adds a summary, a changelog draft and the files that more than one commit changes. `--pr` and `--title` head the page with the pull request. The page is one HTML file with no script. `make semantic-diff` renders master..HEAD to _build/semantic-diff.html and opens it. Example (sh): make semantic-diff node scripts/commitRecords.js --out review.html master..my-branch Changelog entry: - Add commit records, a commit message format that answers recurring review questions, and a semantic diff page rendered from them and linked on each pull request. #8870 Review: Does the renderer need the compiler? No: it reads git (messages, notes on refs/notes/records, and diffs) and has no dependencies. Area: scripts tests docs ci Changelog: internal Output: unchanged Tests: +scripts/commitRecords.test.js PR: #8870 Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

cristianoc force-pushed the cristianoc/semantic-diff branch from a7f0b38 to f38c0b2 Compare October 16, 2026 00:59

cristianoc mentioned this pull request Oct 16, 2026

Compile integer operations with an identity operand to the operand #8871

Closed

cristianoc added this pull request to stack #8872 October 16, 2026 01:00

cristianoc marked this pull request as ready for review October 16, 2026 01:00

This was referenced Oct 16, 2026

Compile integer operations with an identity operand to the operand #8862

Closed

Render review pages from structured commit messages #8861

Closed

cristianoc force-pushed the cristianoc/semantic-diff branch from f38c0b2 to 22cdcc4 Compare October 16, 2026 01:01

cristianoc requested a review from cknitt October 16, 2026 04:50

cknitt reviewed Oct 16, 2026

View reviewed changes

cristianoc force-pushed the cristianoc/semantic-diff branch from 22cdcc4 to 951330f Compare October 16, 2026 07:08

cristianoc force-pushed the cristianoc/semantic-diff branch from 951330f to e1d1cb1 Compare October 16, 2026 07:11

cristianoc and others added 2 commits October 16, 2026 09:51

cristianoc force-pushed the cristianoc/semantic-diff branch from e1d1cb1 to fb7a59b Compare October 16, 2026 07:51

cknitt approved these changes Oct 16, 2026

View reviewed changes

cristianoc merged commit e642d3d into master Oct 16, 2026
28 checks passed

cristianoc deleted the cristianoc/semantic-diff branch October 16, 2026 14:34

Conversation

cristianoc commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test plan

Uh oh!

pkg-pr-new Bot commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 16, 2026

Uh oh!

cknitt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cristianoc commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

cknitt commented Oct 16, 2026

Uh oh!

cristianoc commented Oct 16, 2026

Uh oh!

github-actions Bot commented Oct 16, 2026

Uh oh!

cknitt left a comment

Choose a reason for hiding this comment

Uh oh!

cristianoc commented Oct 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cristianoc commented Oct 16, 2026 •

edited

Loading

pkg-pr-new Bot commented Oct 16, 2026 •

edited

Loading

cristianoc commented Oct 16, 2026 •

edited

Loading

codecov Bot commented Oct 16, 2026 •

edited

Loading