Skip to content

✅ Assert the fast emitter's output always re-parses in differential fuzz#148

Merged
frenck merged 1 commit into
mainfrom
frenck/fuzz-assert-emit-reparse
Jun 26, 2026
Merged

✅ Assert the fast emitter's output always re-parses in differential fuzz#148
frenck merged 1 commit into
mainfrom
frenck/fuzz-assert-emit-reparse

Conversation

@frenck

@frenck frenck commented Jun 26, 2026

Copy link
Copy Markdown
Owner

Breaking change

None. This change is fuzz-harness only; it touches no library code and no public API.

Proposed change

The differential fuzz oracle (check_emit_roundtrip, shared by the differential and differential_options targets) used to silently return when the re-emitted YAML failed to re-decode, split into several documents, or was invalid UTF-8. That hid the worst dumps bug class: output a reader rejects or reshapes, with no crash to flag it.

This tightens the oracle so each of those is a hard finding. The fast emitter takes a freshly decoded Value tree (no user edits), so it must always produce valid YAML that loads reads back as the same single document; anything else is a dumps bug. The lenient roundtrip target (the round-trip emitter, which re-emits modified documents and can legitimately produce composer-rejected output for exotic inputs) is left untouched.

This is the closing step of the fuzz-and-fix pass: the three emitter bugs the strict oracle surfaced are already fixed and merged (#144 quoting a ... document-end marker, #145 keeping a folded scalar off a marker line, #146 indenting a tagged sequence item under its dash). With those in, both differential targets now run clean under the strict oracle (702k and 1.5M executions locally, zero findings).

Type of change

  • Dependency or tooling upgrade
  • Bugfix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Deprecation (replaces or removes a feature, with a migration path)
  • Breaking change (a fix or feature that changes existing behavior)
  • Code quality, refactor, or test-only change
  • Documentation only

Additional information

Checklist

  • I have read the AI Policy, and this pull request was not created by an autonomous agent.
  • I fully understand the code in this pull request and can explain every line, including any AI-assisted changes.
  • The change is covered by tests, and uv run pytest passes locally. A pull request cannot be merged unless CI is green.
  • uv run ruff check . and uv run ruff format --check . pass.
  • cargo fmt --check and cargo clippy --all-targets -- -D warnings pass.
  • Round-trip fidelity is preserved: an unmodified document still re-emits byte-for-byte.
  • No commented-out or dead code is left in the pull request.

If the change is user-facing:

  • Documentation under docs/ is added or updated, and docs/verify_examples.py still passes.

The differential oracle (check_emit_roundtrip, shared by the differential
and differential_options targets) silently returned when the re-emitted
YAML failed to re-decode, split into several documents, or was invalid
UTF-8. That hid the worst dumps bug class: output a reader rejects or
reshapes, with no crash to flag it.

Tighten the oracle so each of those is a hard finding. The fast emitter
takes a freshly decoded Value tree (no user edits), so it must always
produce valid YAML that loads reads back as the same single document. The
lenient roundtrip target (round-trip emitter, re-emits modified documents)
is left untouched.

This closes the fuzz-and-fix pass: the three emitter bugs the strict oracle
surfaced are fixed and merged (#144, #145, #146), and both differential
targets now run clean under it (702k and 1.5M executions, zero findings).
Copilot AI review requested due to automatic review settings June 26, 2026 18:53
@frenck frenck added maintenance Generic maintenance tasks. rust labels Jun 26, 2026
@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 9d713f60-99b4-455a-83b3-3a87b4d9af99

📥 Commits

Reviewing files that changed from the base of the PR and between fc31ce5 and 9852a20.

📒 Files selected for processing (1)
  • src/lib.rs

📝 Walkthrough

Walkthrough

The differential fuzz harness now treats emitter roundtrip failures as hard assertions. It panics on invalid UTF-8, YAML re-decode failures, and reparsing into anything other than exactly one document, then continues with the existing values_equiv comparison.

Possibly related PRs

  • frenck/YAMLRocks#98 — This PR changes the same check_emit_roundtrip logic in src/lib.rs, so it is directly related to the stricter roundtrip validation here.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly reflects the main change: making the differential fuzz emitter round-trip checks stricter.
Description check ✅ Passed The description is detailed and directly matches the changes to the differential fuzz oracle.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codspeed-hq

codspeed-hq Bot commented Jun 26, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 16 untouched benchmarks


Comparing frenck/fuzz-assert-emit-reparse (9852a20) with main (fc31ce5)

Open in CodSpeed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens the differential fuzz oracle (check_emit_roundtrip, shared by the differential and differential_options targets) so that the fast emitter's output is held to a strict re-parse invariant. Previously, invalid UTF-8, re-decode failures, and multi-document output were silently returned (skipped); now each is a hard panic. Since the fast emitter operates on a freshly decoded Value tree with no user edits, any such outcome is a genuine dumps bug. The change is fuzz-harness only and touches no library code or public API. It is the closing step of an emitter hardening pass whose three surfaced bugs were already fixed in #144, #145, and #146.

Changes:

  • Convert the three lenient return skips (UTF-8 decode, re-decode, document-count) into hard assertions with descriptive panic messages that name the triggering options, input, and emitted text.
  • Update the differential and check_emit_roundtrip doc comments to reflect the strict-assertion contract and document the three bugs this oracle surfaced.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@frenck frenck merged commit 7607546 into main Jun 26, 2026
74 of 75 checks passed
@frenck frenck deleted the frenck/fuzz-assert-emit-reparse branch June 26, 2026 19:15
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jun 28, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

maintenance Generic maintenance tasks. rust

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants