feat(cosim): add the full-capture observable golden, and close the rung-0 gate - #436
Merged
Merged
Conversation
…ng-0 gate
The rung-0 gate is "feed RustyNES's golden back in as if it were the DUT and
get 0 divergences from every diff CLI". It was not implementable for the
checkpoint format, and that was found by trying to build it rather than by
reading the spec.
THE CSV CANNOT RE-DERIVE THE CHECKPOINTS
irq.csv carries 23 columns. Neither pc nor put_cycle_post is among them -- two
of the nine fields Observable folds into the hash are simply absent from the
file. Verified by parsing the header against CycleRecord rather than assumed:
six of the struct's 29 fields never reach the CSV, and two of those six are
observable.
So an external testbench reading the CSV cannot reproduce the numbers in the
.ckpt.bin sitting beside it. The self-diff had nothing to run on.
<stem>.obs.bin closes that: repeated 16-byte records in the SAME wire encoding
the hash folds, headerless for the same reason the checkpoint stream is -- the
manifest already records the ROM hash, the seed and the frame count, and a
second place to state them is a second place for them to disagree.
Additive on purpose. The CSV is untouched, because scripts/irq_trace_cross_diff.py
and the committed golden/irq_trace/*.csv both depend on its shape, and a format
bump there to serve a new consumer would be paid for by every existing one.
The stream is also what a re-run of a located window consumes, so it would have
been needed regardless. Checkpoints say WHICH 4096-cycle window differs; the
re-run needs the records in it.
DECODE REFUSES WHAT IT DOES NOT UNDERSTAND
Observable::decode rejects a non-zero reserved pad byte, an undefined flag bit,
an unknown bus-access code, a short record, and a stream length that is not a
multiple of 16. That is not pedantry. Reading a record from a newer producer as
though nothing had changed is how a FORMAT divergence gets reported as a DUT
divergence -- and at this rung that is the single most expensive way to be
wrong, because the DUT is the thing under suspicion and the testbench is not.
The stream is emitted even when the checkpoints are REFUSED for overflow. A
hash over a truncated trace claims a coverage it does not have; the records
themselves are just records, and are worth keeping for a re-run.
MEASURED ACROSS THE REPOSITORY BOUNDARY, NOT ONLY IN UNIT TESTS
$ tb/selfdiff_check.sh (in the RustyNES_MiSTer checkout)
1/3 exporting goldens from the oracle
2/3 re-deriving checkpoints on this side
re-derived 22 checkpoints from 89335 records
3/3 comparing
checkpoints match: 22 compared, 0 divergences
negative control: one flipped bit must be located
DIVERGED at checkpoint 10
window to re-run with full capture: cycles (40967, 45063] (4096 cycles)
rung 0 self-diff: agreement recognised, and disagreement located
89,335 records of AccuracyCoin, re-derived in C++ from .obs.bin alone, hashing
to byte-identical checkpoints. The negative control runs in the SAME
invocation, because a positive control alone is satisfiable by a comparison
that always agrees -- and a harness that only ever says "match" passes
everything.
Also verified: a malformed record and a truncated stream are both refused with
exit 3 rather than hashed.
nes_golden_export's main() crossed clippy's too_many_lines while this landed.
Extracted write_irq_artifacts rather than allowing the lint -- main was doing
argument parsing, running, and five writes, and the extracted function is where
the one-take-three-artifacts constraint is now documented.
Gates. The emulation core is untouched; no file under
crates/rustynes-{cpu,ppu,apu,mappers,core} changes, so AccuracyCoin 141/141 and
nestest 0-diff hold by construction.
fmt (workspace and the excluded crate), clippy on both, rustdoc -D warnings
on both, markdownlint on both changed documents.
126 workspace suites / 2223 passed / 0 failed
4 excluded-crate suites / 42 passed / 0 failed (was 39)
|
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: Pro Plus 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)Adds a full-capture observable stream ( Blocking issues
Suggestions
Nitpicks
Automated first-pass review by |
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.
The rung-0 gate is "feed RustyNES's golden back in as if it were the DUT and get 0 divergences from every diff CLI." For the checkpoint format it was not implementable, and that was found by trying to build it rather than by reading the spec.
The CSV cannot re-derive the checkpoints
irq.csvcarries 23 columns, and neitherpcnorput_cycle_postis among them — two of the nine fieldsObservablefolds into the hash are simply absent. Verified by parsing the header againstCycleRecordrather than assumed: six of the struct's 29 fields never reach the CSV, and two of those six are observable.So an external testbench reading the CSV cannot reproduce the numbers in the
.ckpt.binbeside it. The self-diff had nothing to run on.<stem>.obs.bincloses that — repeated 16-byte records in the same wire encoding the hash folds, headerless for the same reason the checkpoint stream is. It is also the input a re-run of a located window consumes, so it would have been needed regardless: checkpoints say which 4096-cycle window differs, and the re-run needs the records in it.Additive on purpose. The CSV is untouched, because
scripts/irq_trace_cross_diff.pyand the committedgolden/irq_trace/*.csvboth depend on its shape, and a format bump there to serve a new consumer would be paid for by every existing one.Decode refuses what it does not understand
A non-zero reserved pad byte, an undefined flag bit, an unknown bus-access code, a short record, a stream length that is not a multiple of 16. Not pedantry: reading a record from a newer producer as though nothing had changed is how a format divergence gets reported as a DUT divergence — the single most expensive way to be wrong at this rung, because the DUT is the thing under suspicion and the testbench is not.
The stream is emitted even when the checkpoints are refused for overflow. A hash over a truncated trace claims a coverage it does not have; the records themselves are just records.
Measured across the repository boundary
89,335 records of AccuracyCoin, re-derived in C++ from
.obs.binalone, hashing to byte-identical checkpoints. The negative control runs in the same invocation, because a positive control alone is satisfiable by a comparison that always agrees — a harness that only ever says "match" passes everything.Also verified: a malformed record and a truncated stream are both refused with exit
3rather than hashed.Gates
Core untouched — no file under
crates/rustynes-{cpu,ppu,apu,mappers,core}changes — so AccuracyCoin 141/141 and nestest 0-diff hold by construction.fmt (workspace and the excluded crate) · clippy on both · rustdoc
-D warningson both · markdownlint.126 workspace suites / 2223 passed / 0 failed, plus 4 excluded-crate suites / 42 passed (was 39).
The matching C++ half is in the sibling repository (
tb/selfdiff.cpp,tb/selfdiff_check.sh), pushed alongside.