fix(processor): fail loudly on z3 unknown in the satisfiability solver - #1083
Closed
doublewhy wants to merge 1 commit into
Closed
fix(processor): fail loudly on z3 unknown in the satisfiability solver#1083doublewhy wants to merge 1 commit into
doublewhy wants to merge 1 commit into
Conversation
Under load the per-call 5s z3 timeout can return z3.unknown. The pinned adapter treated that non-decisive result as decisive: _reduce_unsat_core kept any clause whose removal did not return unsat, and _select_witness skipped any domain value that did not return sat. A timed-out check therefore silently emitted a non-minimal core or a non-canonical witness while the published SolverConfigurationModel still claimed subset-minimal core reduction and canonical-lexicographic witness selection, surfacing later as spurious replay_satisfiability_evidence failures instead of a diagnosable error. _check now raises SolverOperationalError on z3.unknown (reported as SatisfiabilityOperationalError at the service boundary), so every decision is decisive or fails loudly. It also asserts an explicit false for empty finite-domain memberships instead of relying on zero-argument z3.Or, and solve_model rejects duplicate clause ids at the boundary rather than letting them collapse in the tracking table (which z3 otherwise rejects with an opaque exception). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
|
Please note in the comments or body which issue this PR closes. If no issue exists, please create one and link it. Thank you! |
Author
15 tasks
Author
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.
Related issues
Closes #1114
Related to #826 and #1108.
What breaks
The satisfiability analyzer could publish a result that claims more than it actually established, and the overclaim only surfaced later as an unexplained replay failure.
Each solver call has a 5-second timeout, so z3 can answer "I don't know" (
unknown) rather than yes or no. The code treated that non-answer as a definite answer.Concretely
Analyzing a scenario emits a
ScenarioSatisfiabilityEvidencerecord. That record is labelled with the profile it was produced under:core_reduction: "sorted-deletion-subset-minimal/v1"— asserts the unsatisfiable core is subset-minimalwitness_selection: "canonical-lexicographic/v1"— asserts the witness is the canonical first choiceUnder load, a single probe times out. Because
unknownis notunsat, the core-reduction loop keeps a clause it would otherwise have removed; becauseunknownis notsat, the witness loop skips a value it would otherwise have chosen. The emitted record is structurally valid and still carries both labels — but neither property was checked. Nobody sees an error.The failure shows up much later, somewhere else:
replay_satisfiability_evidencere-derives the result, gets the correct core or witness, compares digests, and disagrees. The published evidence looks corrupt for no visible reason.For a project whose premise is that a specification plus its evidence is the reproducible unit of exchange, silently emitting evidence that overstates what was proven is the worst available outcome.
Why it happens
In
raes_processor/satisfiability/_solver.py,_checkreturns z3's raw result, and both callers compare against one outcome only:_reduce_unsat_corekeeps any clause whose removal is!= z3.unsat_select_witnessskips any value that is!= z3.satunknownfalls into the same branch as a decisive answer in both.The fix
_checknow raisesSolverOperationalErroronunknown, carrying z3's ownreason_unknown(). This is the single seam every caller shares, so no path can forge minimality or canonicality. The service boundary already converts that into the typedSatisfiabilityOperationalError("the pinned solver did not complete"), so no new error style is introduced — a timeout becomes a diagnosable operational failure instead of quiet bad evidence.Two smaller issues in the same file:
z3.Or(). In the pinned z3 that happens to assert as unsatisfiable, but it is undocumented edge behaviour; the empty case is now stated explicitly asBoolVal(False).Z3Exception. They are now refused at the model boundary with a clear message.The governed
z3-solver==4.16.0.0pin is untouched, and nothing undercontracts/schemas/orspecs/changes.How I know
4 new tests. A helper forces the Nth solver call to return
unknownand leaves the rest real:Z3Exception)The first three fail against the current code. The fourth passes before and after by design — it pins existing behaviour rather than demonstrating the bug, and is called out as such.
Full
nox -s verifygreen on Ubuntu 22.04 / Python 3.12, all six lanes, 91% total coverage.🤖 Generated with Claude Code