Don't delete the constraints that the solver couldn't satisfy

[BoykoNeov]

Fixes a data-loss bug that is independent of any particular sketch: when a solve fails, SolveSpace deletes the constraints that failed.

I found this while working on solvespace#1247 — the constraint count dropped from 7 to 5 after a failed solve, and a later FindById on one of the missing handles asserted. It is not specific to that model or to that issue; any group that fails to solve loses the constraints the solver couldn't satisfy, and with them the value the user needs to edit to recover.

Root cause

PruneOrphans() and PruneRequestsAndConstraints() (src/generate.cpp) both follow the same protocol: walk the list, set tag = 1 on the elements to delete, then call RemoveTagged(), which removes every element whose tag is nonzero. Neither of them clears the tags first, so they delete their own selection plus whatever happened to be tagged already.

System::Solve() tags exactly the constraints whose equations it couldn't satisfy — that is its didnt_converge: path, where the tag is used to avoid listing the same constraint twice while building the report. A failed solve is immediately followed by a regeneration, the regeneration prunes, and the prune sees those leftover tags as delete-me marks.

So the constraints that fail are precisely the constraints that get destroyed.

Why the fix is safe

SK.request.ClearTags() / SK.constraint.ClearTags() before each tagging loop. Two things I checked rather than assumed:

• Nothing relies on tags surviving into the prune. The only callers are generate.cpp:227 and :265, and neither sets tags beforehand.
• The solver's report is unaffected. The tag is only a dedup marker while System::Solve() builds the list; what the UI actually shows comes from the bad list, which this doesn't touch.

The request lists get the same treatment. Nothing tags a request today, so that half is a no-op — but they are pruned by the identical tag-then-RemoveTagged() protocol and would lose requests the same way if anything ever did.

Regression test

test/core/prune/failed_solve_keeps_constraints — load a right triangle with an angle dimension, set that angle to 95°, which no triangle of this shape has (the direction cosine is 100/sqrt(100² + h²), positive for every h, and cos 95° is not), so the solver gives up and tags. The test then asserts the request and constraint counts are unchanged and that the angle constraint is still there with its value intact — a bare count would also pass if the wrong constraint vanished and another appeared.

Without the fix it fails on the count; with it, it passes.

Verification

• Full suite passes in Debug and Release: 263 cases / 931 checks (master is 262 / 925).
• The test was written and run against unfixed code first, as the go/no-go for this PR, and fails there at the count assertion.

Notes for review

• The fixture is a copy. test/core/prune/angle.slvs is byte-identical to the fixture used by my SolveSpace fails to solve solvable constraints solvespace/solvespace#1247 PR. They are separate PRs on purpose — this fix is four lines in generate.cpp and has nothing to do with the Newton step — so each carries its own copy rather than one depending on the other. If both are taken, the only conflict is one line in test/CMakeLists.txt; with both entries kept, the combination builds and passes (265 / 943, verified by a trial merge). Deduplicating the fixture afterwards would be a fine follow-up.
• I'd suggest taking this one first regardless of what you think of the others: it is small, it is independent of every other change I've proposed, and until it lands, every solver failure anywhere in SolveSpace is also a data-loss event.
• Please fetch and fast-forward this branch rather than using the merge button on my fork — that keeps it a clean fast-forward for upstream.

🤖 Generated with Claude Code

[ruevs]

This fixes the bug but causes a memory leak

[BoykoNeov]

Written by Claude Opus 5 — both this text and the code it describes; posted by @BoykoNeov.

@ruevs Fixed — pushed 81f473ff on top of this branch, so the ASan build should be clean on the next CI run.

Short version: the leak is not caused by clearing the tags. Group::Clear() frees every dynamically-allocated member of a group except solved.remove, the list System::Solve() fills with the constraints it couldn't satisfy, so a failed (or redundant) solve leaves one allocation live. It is pre-existing — I measured it with src/generate.cpp reverted to master and the same allocation is orphaned there — and this branch's test is simply the first thing in the suite that makes a solve fail.

Full write-up, including the answer to @phkahler's "did 3.0 or 3.1 delete the constraints too?" (they did not — the regression came in after v3.1 and first shipped in v3.2), is on the upstream PR: solvespace#1744 (comment)