detect_tag_changes.py: normalize RISC-V macros in the change report (#245)#247
Merged
Merged
Conversation
The tags backend stores raw AsciiDoc (see #169), so the reviewer-facing change report compared markup-migrated text as different: `mip` vs `csr:mip[]`, `11b` vs `0b11`, `--` vs `{endash}`. On the real riscv-isa-manual reference regeneration (#3249) this made 199 of 216 "Modified" tags (92%) markup-only false positives, and the tool exited non-zero, which consumers treat as a breaking normative change. Teach `_strip_asciidoc_formatting` the semantic macros so a pure markup migration reports as no change: csr:NAME[] -> NAME csr:NAME[FIELD] -> NAME.FIELD (field upper-cased, as prose renders) csr:[FIELD] -> FIELD csr::[FIELD] -> FIELD ext:NAME[] -> NAME (capitalized: F, D, Sscofpmf, Zicsr) insn:NAME[] -> NAME Also normalize binary renotation (11b -> 0b11) and en dash (-- -> {endash}), the other two forms in that migration. This is applied ONLY to the review comparison, never as a freshness check: erasing formatting is correct for "did the normative text change" and wrong for "is this file byte up to date" -- the exact conflation that caused ref/ to drift. Strict mode (--strict) still compares prose byte-for-byte and is unaffected. Verified end-to-end against the two committed riscv-isa-manual reference revisions spanning the #3249 macro migration: the report drops from Modified 216 to Modified 32, and the ~32 that remain are genuine normative edits (added clauses, table restructures) plus a few markup forms outside this migration's scope (insn: with operands, single-hyphen ranges) that are appropriate for a human to review. Add isolated fixtures (macros-plain/macros-markup) and make targets test-tag-changes-macros / test-tag-changes-strict-macros. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Bill Traynor <wmat@riscv.org>
Bill Traynor (wmat)
requested review from
Jordan Carlin (jordancarlin) and
Rafael Sene (rpsene)
July 16, 2026 14:57
Collaborator
Author
|
Validated against riscv-isa-manual via the manual's exact |
Jordan Carlin (jordancarlin)
approved these changes
Jul 16, 2026
Jordan Carlin (jordancarlin)
left a comment
There was a problem hiding this comment.
Seems like this might be tricky to maintain. It would be better to parse this from the macro definition file or something like that. Fine for now though and a clear improvement even as is.
Rafael Sene (rpsene)
approved these changes
Jul 17, 2026
Rafael Sene (rpsene)
left a comment
Contributor
There was a problem hiding this comment.
I like that normalization stays confined to the review report while --strict remains byte-exact. +1 to Jordan on eventually sourcing the macro list from the definitions rather than hardcoded regex.
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
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.
Part of #245 (item 2 of 3) — the highest-value item.
The tags backend stores raw AsciiDoc (#169), so the reviewer-facing change report compared markup-migrated text as different:
mipvscsr:mip[],11bvs0b11,--vs{endash}. On the real riscv-isa-manual reference regeneration (riscv/riscv-isa-manual#3249) this made 199 of 216 "Modified" tags (92%) markup-only false positives, and the tool exited non-zero — which consumers treat as a breaking normative change.Change
Teach
_strip_asciidoc_formattingthe semantic macros so a pure markup migration reports as no change:csr:NAME[]NAMEcsr:NAME[FIELD]NAME.FIELD(field upper-cased)csr:[FIELD]/csr::[FIELD]FIELDext:NAME[]NAME(capitalized:F,D,Sscofpmf,Zicsr)insn:NAME[]NAMEAlso normalizes binary renotation (
11b→0b11) and en dash (--→{endash}).This is applied only to the review comparison, never as a freshness check — erasing formatting is correct for "did the normative text change" and wrong for "is this file byte up to date", the exact conflation that caused
ref/to drift.--strictstill compares prose byte-for-byte and is unaffected.Validation
Ran the old vs new normalizer against the two committed riscv-isa-manual reference revisions spanning the #3249 macro migration:
Added 1, Deleted 2, Modified 216(matches Normative tag tooling: unsound --update-reference, normalizer misses RISC-V macros, tags.rb omits trailing newline #245 exactly)Added 1, Deleted 2, Modified 32The remaining 32 are genuine normative edits (added clauses, table restructures) plus a few markup forms outside this migration's scope (
insn:with operands, single-hyphen ranges) that are appropriate for a human to review.make testpasses.Note: the issue's lone
ext:f[]→Fexample was misleading — multi-letter extension names render capitalize-first (Sscofpmf, notSSCOFPMF); handled accordingly.🤖 Generated with Claude Code