refactor(pumpkin-propagators): Move away from TestSolver in propagator tests

[maartenflippo]

The TestSolver is deprecated because it has unclear semantics on how
it works. The State abstraction is much better designed, so more
appropriate for test cases for propagators.

Not all tests are converted, because I wanted to avoid changing the
State API in this PR. Some tests use features in TestSolver that are
not present in State for various reasons.

This is entirely produced by claude code, to evaluate how well
it performs on these refactoring tasks. The task is not difficult, but
tedious for us to complete.

[EmirDe]

This looks overall fantastic. Great that we can finally move away from the TestSolver!

Let us carefully review all these changes...it seems doable.

Question: do we want the claude.md file? It might be useful but not fully sure.

[EmirDe]

I had a look at 20% of the code. I will do the review in batches.

Overall the changes are very welcome, using State directly is much, much better.

I also noticed a few other things that we can open issues or fix immediately (not necessarily as part of this PR).

• state.new_interval_variable -> the documentation does not explicitly say what happens when we pass "None" as the name.
• TestSolver still used, hopefully we can remove it later.
• Sometimes the code uses

let result = state.propagate_to_fixed_point();
assert!(result.is_err());

and

state.propagate_to_fixed_point().expect("Expected no conflict to be detected");

and

state.propagate_to_fixed_point().expect("no empty domains");

so perhaps we can get it to unify this. This is not a problem per se.

• pub fn get_propagation_reason -> the documentation is not explicit on whether it just extends the input buffer, or it clears the input buffer before adding the reason.

[maartenflippo]

Sometimes the code uses

let result = state.propagate_to_fixed_point();
assert!(result.is_err());

Fixed! Now we use expect/unwrap everywhere.

[maartenflippo]

state.new_interval_variable -> the documentation does not explicitly say what happens when we pass "None" as the name.

Should be a separate PR. I created an issue #400.

[maartenflippo]

TestSolver still used, hopefully we can remove it later.

Hopefully yes, but this PR is a good step toward that

[maartenflippo]

pub fn get_propagation_reason -> the documentation is not explicit on whether it just extends the input buffer, or it clears the input buffer before adding the reason.

Should be a separate PR. Created an issue #401.

[maartenflippo]

Question: do we want the claude.md file? It might be useful but not fully sure.

It definitely helps to have it if we want to use this tool going forward. It ensures that claude code can properly identify formatting and clippy issues.

[EmirDe]

I reviewed the PR about 50%. I will only be able to complete the rest next week. If Imko reviews the Pr it is fine with me to merge! Overall, it seems great so far.

[ImkoMarijnissen]

Some small comments about how reasons are created using PropositionalConjunction which is unnecessarily convoluted, some added generics in the time-tabling tests which are unnecessary, and clarifications on the StateExt trait.

I have a lot of comments to mark the instances in which this occurs so that it is easier to check later whether this was resolved.

Other than that, it looks good to me!

Review re-requested