Repository navigation
feat!: total graph evaluation with Result-returning error handling (spec 06) - #24
Merged
Merged
Conversation
…pec 06) ComputationNode::evaluate/evaluate_arithmetic/evaluate_bool now return Result<_, UncertainError> instead of panicking on unsupported node/domain combinations. evaluate_arithmetic absorbs evaluate_conditional_with_arithmetic (removed) and is now the total dispatcher for the arithmetic domain, including Conditional nodes. GraphProfiler::get_stats no longer relies on unreachable .expect() calls.
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.
Summary
ComputationNode::evaluate,evaluate_arithmetic, andevaluate_boolnow returnResult<_, UncertainError>instead of panicking on unsupported node/domain combinations. New error variantUncertainError::UnsupportedNode { expected, found }.evaluate_arithmeticabsorbs the formerevaluate_conditional_with_arithmetic(now deleted) and is the total dispatcher for the arithmetic domain, includingConditionalnodes.evaluate_freshkeeps its infallible-> Tsignature (used by ~20 existing callers, mostly tests) via an internal.expect()justified by the invariant that graphs built through the publicUncertain<T>API are always well-formed.GraphProfiler::get_stats's two now-unreachable.expect()calls replaced with direct indexing.#[should_panic]evaluation tests replaced withResult/error-variant assertions; added tests closing coverage gaps the rewrite exposed.specs/06-total-graph-evaluation.mdfor the full spec and an "Implementation notes" section documenting why this keeps three separate domain-scoped dispatchers (evaluate/evaluate_arithmetic/evaluate_bool) rather than one universal dispatcher — each is total over a different variant set dictated byT's trait bounds.crap_baseline.jsonregenerated: the three rewritten methods show a higher CRAP score (CC +1 to +5) from the addedResultplumbing, but all reached 100% line coverage in this change, so the increase is pure complexity, not a coverage gap.main(conflicts only inspecs/README.md/MIGRATION_GUIDE.md/CHANGELOG.md, resolved by combining both specs' notes).Test plan
just devgreen (fmt, clippy-D warnings, tests default +parallel, doc tests, audit, deny, crap regression gate) — reconfirmed after rebase--all-featuresand--features parallelcargo benchsanity check oncomputation_graphs— all three benchmarks within noiseCHANGELOG.md/MIGRATION_GUIDE.md/specs/README.mdupdatedhttps://claude.ai/code/session_01Xw68NoMZnNRLkxjK6Razu7