Materializing then parsing a conflict can drop a trailing `\r` byte from its content

[DRMacIver]

use bstr::BString;
use jj_lib::conflict_labels::ConflictLabels;
use jj_lib::conflicts::materialize_merge_result_to_bytes;
use jj_lib::conflicts::parse_conflict;
use jj_lib::conflicts::ConflictMarkerStyle;
use jj_lib::conflicts::ConflictMaterializeOptions;
use jj_lib::files::FileMergeHunkLevel;
use jj_lib::merge::Merge;
use jj_lib::merge::SameChange;
use jj_lib::tree_merge::MergeOptions;

fn main() {
    let merge = Merge::from_vec(vec![
        BString::from("left\r"),
        BString::from("base"),
        BString::from("right"),
    ]);

    let options = ConflictMaterializeOptions {
        marker_style: ConflictMarkerStyle::Git,
        marker_len: None,
        merge: MergeOptions {
            hunk_level: FileMergeHunkLevel::Line,
            same_change: SameChange::Keep,
        },
    };

    let materialized =
        materialize_merge_result_to_bytes(&merge, &ConflictLabels::unlabeled(), &options);

    let parsed = parse_conflict(&materialized, 2, 7).expect("should parse as a conflict");
    let side0 = parsed[0].iter().next().unwrap();
    println!("original left side:      {:?}", BString::from("left\r"));
    println!("round-tripped left side: {side0:?}");
}

original left side:      "left\r"
round-tripped left side: "left"

The merge's left side is the three bytes left\r, with no \n anywhere in any side. After materialize_merge_result_to_bytes writes it out and parse_conflict reads it back, the trailing \r is gone: "left\r" round-trips to "left".

Tested on jj-lib 0.43.0.

BTW, this bug was found using hegel. Happy to contribute the tests if you're interested.

This report was drafted by an AI agent (Claude Code) and reviewed by @DRMacIver before filing.

[arxanas]

I'd love to see the tests / hegel introduced! I think we'd want to migrate our existing dependency/tests on proptest, though.

[martinvonz]

What are the benefits of hegel over proptest?

[DRMacIver]

I'd love to see the tests / hegel introduced!

Great! I'll get a draft PR up for you at some point soonish (I'm on holiday at the moment and threw out a bunch of bug reports before I left so it might be next week some time)

I think we'd want to migrate our existing dependency/tests on proptest, though.

This should be easy, assuming you have no objections to an AI powered rewrite. We've got a claude code skill that's very good at porting proptest to hegel (that's not really its primary use case, it's for writing Hegel tests, but an existing property-based test is the easiest starting point for that). If you want I can get you a PR porting them over on a similar timeline.

What are the benefits of hegel over proptest?

With the caveats that I'm obviously quite biased in this:

• Much better shrinking
• Free interleaving of data generation and test-case execution which makes it much better for testing highly stateful systems
• The stateful testing interface (which generates arbitrary operations against a stateful system) in Hegel doesn't have any nearly as good equivalent in proptest
• I think the generator language is much richer and more powerful in Hegel (this one is the one I'm most biased on. We've got explicit generator combinators expressed as first class values, and I think this is a much better approach)

Hegel is also just much more actively maintained (we've got a lot of resources behind it right now and are somewhat aggressively working on power and devex of the library, while proptest is more or less in maintenance-only mode). e.g. right now we're really actively working on the feature-set for concurrent properties, but that might take another month or so before it's really exciting.

[arxanas]

For historical context, Hegel was not available at the time that we introduced proptest (AFAIK).

• Much better shrinking

Here's an example of specifically working around proptest shrinking:

jj/lib/testutils/src/proptest.rs

Lines 197 to 208 in 5b48bec

	// NOTE: Using `prop_oneof` here instead of `proptest::sample::select`
	// since it seems to minimize better?
	if !state.entries.is_empty() {
	prop_oneof![
	Just(Transition::Commit),
	state.arb_transition_create(),
	state.arb_transition_modify(),
	]
	.boxed()
	} else {
	prop_oneof![Just(Transition::Commit), state.arb_transition_create()].boxed()
	}

I don't know for certain if Hegel will shrink better, but I don't remember having to worry about this particular issue with Hypothesis.

• Free interleaving of data generation and test-case execution which makes it much better for testing highly stateful systems
• The stateful testing interface (which generates arbitrary operations against a stateful system) in Hegel doesn't have any nearly as good equivalent in proptest

We currently express stateful transitions in a very stilted way:

jj/lib/testutils/src/proptest.rs

Lines 157 to 175 in 5b48bec

	fn arb_transition_create(&self) -> impl Strategy<Value = Transition> + use<> {
	(
	self.arb_extant_directory(),
	proptest::collection::vec(arb_path_component(), 1..3),
	any::<DirEntry>(),
	)
	.prop_map(|(extant_dir_path, new_path_components, dir_entry)| {
	let mut path = extant_dir_path;
	path.extend(
	new_path_components
	.iter()
	.map(|c| RepoPathComponent::new(c).unwrap()),
	);
	Transition::SetDirEntry {
	path,
	dir_entry: Some(dir_entry),
	}
	})
	}

• Compare with hegel::stateful, which can directly use tc.draw.
• Alternatively, compare the ergonomics of proptest_compose! and tc.draw.