[alt] Suppress CHASM metric emission while re-applying transitions (node method)#10846
Closed
tekkaya wants to merge 1 commit into
Closed
[alt] Suppress CHASM metric emission while re-applying transitions (node method)#10846tekkaya wants to merge 1 commit into
tekkaya wants to merge 1 commit into
Conversation
…thod) Alternative to the context-flag approach (#10845): express "replaying" as tree state on the node rather than on a per-context flag. Add a replaying bool to nodeBase plus Node.SetReplaying / Node.Replaying. While the tree is in replay mode, Context.MetricsHandler() returns metrics.NoopMetricsHandler, so side-effect metrics emitted inside any component's transition are not double-counted when the reset / conflict- resolution re-apply (cherry-pick) path re-runs them. Because every context reads its handler from the root node, the reset re-apply path just flips the tree into replay mode around the cherry-pick (no special context constructor, no flag propagation through context derivation, and no way to accidentally create a live context during re-apply). Guardrail only: the CHASM cherry-pick dispatch is not wired yet, so there is no production caller today; a functional reset/double-count test should land with that wiring. Tests: - chasm: SetReplaying(true) -> MetricsHandler is no-op on the same context; restored after SetReplaying(false). - chasm/lib/nexusoperation: a real TransitionSucceeded under a tree in replay mode emits nothing, while live mode emits the success counter.
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
1 participant
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.
What changed?
Alternative to #10845 (per review feedback: "have a method on the node to express Replaying"). Same goal — suppress CHASM caller-side metric emission during cherry-pick re-apply — but the
replayingstate lives on the node/tree instead of on a per-context flag.nodeBase.replaying bool+Node.SetReplaying(bool)/Node.Replaying() bool.Context.MetricsHandler()returnsmetrics.NoopMetricsHandlerwhile the tree is in replay mode.Why this shape
Because every context reads its handler from the root node, the reset/conflict-resolution re-apply path just flips the tree into replay mode around the cherry-pick — no special context constructor, no flag propagation through context derivation (
withValue), and no way to accidentally create a live context during re-apply. "Replaying" is genuinely tree-wide state (the whole execution is being rebuilt), and the metrics handler already lives on the node, so this keeps it in one place.Trade-off vs #10845: this introduces transient mutable state on the tree that must be paired (set true → restore false), whereas the context-flag version is immutable/explicit. CHASM execution is single-threaded under the workflow lock, so the mutation is safe; a scoped helper can be added if preferred.
Scope / not-yet-wired
Guardrail only — the CHASM cherry-pick dispatch isn't wired yet, so there's no production caller today. When it lands, it wraps the cherry-pick in
node.SetReplaying(true)(restored after), and a functional reset/double-count test should land with that wiring.How did you test it?
chasm:SetReplaying(true)makes the same context'sMetricsHandler()a no-op; restored afterSetReplaying(false).chasm/lib/nexusoperation: a realTransitionSucceeded.Applywith the tree in replay mode emits nothing; live mode emitsnexus_operation_success(exercises the realemitOnSucceededMetricspath; existing transition tests use a mock context that bypasses the guard).