docs: the v2.6.19 notes described a design the same release reverted - #522
Conversation
The published release body is the CHANGELOG's `[2.6.19]` section verbatim, and that section contradicted itself. Its first bullet asserted: "`oam2_overflowed` and `oam2_fetch_frozen` are TWO states with the dot-257 latch between them, because a wrap after 257 must not disturb the fetch already in progress." while the seed bullet forty lines below recorded "removing the dot-257 freeze latch" as one of the two RTL changes that superseded the release's own first seed table. Both statements shipped. The second is correct: 197da5b deleted the latch, no such signal is declared, and all four read sites take the live flag. This is the defect this project records most often -- prose asserting an intent the code no longer has -- and it reached users, because the bullet was written while the latch existed and nothing re-read it after the latch was removed later in the same release. An RTL reviewer caught the identical claim in the source comment beside the code; nobody re-checked the release notes for it. Corrected in place with the reason stated, rather than quietly replaced: the entry now says there is ONE live flag, why the latch was wrong (a single sample at dot 257 misses every re-enable later than 257, which is exactly the case the ROM names), why it was unnecessary once the increment window became the 32 even dots 258..320, and that the counter is NOT yet the read pointer -- which is what the one remaining divergence tests. Also adds the verification the entry never stated: ladder 147 of 147 with 0 failed, AccuracyCoin on the DUT 148 of 149 with fail=0 and coverage 149 of 149 on both sides, the unchanged emulation core holding 144/144 and nestest 0-diff by construction, the shipped `.rbf` named with its seed and size, and the standing disclosure that no hardware has run it and what the gates cannot reach. The GitHub release body is updated to match in the same change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Antigravity review (Gemini via Ultra)This PR corrects the v2.6.19 changelog to describe the shipped single-flag OAM design instead of an aborted two-flag draft, and appends the final verification results. Blocking issuesNone found. SuggestionsThis is a trivial documentation update, but the changelog entry is overly verbose and reads like an engineering diary. Consider moving the deep architectural justifications (e.g., the explanation of dot 257 vs dot 260) and the exhaustive test ladder results into developer docs or an ADR, keeping the changelog focused on user-facing behavior and known divergences. Nitpicks
Automated first-pass review by |
The published release body is the CHANGELOG
[2.6.19]section verbatim, and that section contradicted itself.Its first bullet asserted
oam2_overflowedandoam2_fetch_frozenare "two states with the dot-257 latch between them" — while the seed bullet forty lines below recorded "removing the dot-257 freeze latch" as one of the two RTL changes that superseded this release's own first seed table.Both shipped. The second is correct:
197da5bdeleted the latch, no such signal is declared, and all four read sites take the live flag. An RTL reviewer caught the identical claim in the source comment beside the code; nobody re-checked the release notes for it.Corrected, not quietly replaced
The entry now states there is one live flag, why the latch was wrong (a single sample at dot 257 misses every re-enable later than 257 — exactly the case AccuracyCoin names), why it became unnecessary once the increment window was the 32 even dots 258..320, and that the counter is not yet the read pointer, which is what the one remaining divergence tests.
Adds the verification the entry never stated
Ladder 147 of 147, 0 failed · AccuracyCoin on the DUT 148 of 149,
fail=0, coverage 149/149 both sides · core unchanged so 144/144 and nestest 0-diff by construction · the shipped.rbfnamed with seed and size · and the standing disclosure that no hardware has run it.The GitHub release body is updated to match.
Gates: markdownlint clean,
release_anchor_audit13/13.🤖 Generated with Claude Code
https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj