feat: marker state machine by timl3136 · Pull Request #142 · cadence-workflow/cadence-python-client

timl3136 · 2026-06-24T21:00:56Z

What changed?
Implement marker state machine

Why?
Need the state machine for marker feature

How did you test it?
unit tests

Potential risks

Release notes

Documentation Changes

Signed-off-by: Tim Li <ltim@uber.com>

codecov · 2026-06-24T21:05:05Z

Codecov Report

❌ Patch coverage is 94.89051% with 7 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...internal/workflow/statemachine/decision_manager.py	88.23%	3 Missing and 3 partials ⚠️
cadence/testing/_workflow_environment.py	50.00%	1 Missing ⚠️

Files with missing lines	Coverage Δ
cadence/_internal/workflow/context.py	`93.38% <100.00%> (+1.13%)`	⬆️
...ce/_internal/workflow/statemachine/cancellation.py	`100.00% <100.00%> (ø)`
...rnal/workflow/statemachine/marker_state_machine.py	`100.00% <100.00%> (ø)`
.../_internal/workflow/statemachine/nondeterminism.py	`95.16% <100.00%> (+2.94%)`	⬆️
cadence/workflow.py	`95.51% <100.00%> (+0.04%)`	⬆️
cadence/testing/_workflow_environment.py	`85.14% <50.00%> (-0.21%)`	⬇️
...internal/workflow/statemachine/decision_manager.py	`92.41% <88.23%> (-1.38%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

natemort · 2026-06-26T17:48:24Z

+from cadence.api.v1 import decision, history
+from cadence.api.v1.common_pb2 import Payload
+
+MARKER_ID_HEADER_KEY = "__cadence_python_marker_id"


How does this compare to the MarkerName in RecordMarkerDecisionAttributes?

I'm not super familiar with Markers, but the Go SDK seems to use that as the ID.

I checked with Go SDK, and I added a permalink for the formatting "%v_%v" in the comments

natemort · 2026-06-26T17:52:36Z

+        DecisionId(DecisionType.MARKER, marker_id),
+        {
+            "marker_name": attrs.marker_name,
+            "details": attrs.details,


I don't think we need to store the details in the Expectation, I think we'll have to handle it special in DecisionManager. The Expectation exists only for non-determinism tracking.

Thanks, I removed details from both handlers.

natemort · 2026-06-26T18:00:29Z

+# on name+id alone. The history-side handler (below) carries 'details' so DecisionManager
+# can retrieve the recorded value and return it to the caller on replay.
+@to_expectation.register
+def _(attrs: decision.RecordMarkerDecisionAttributes) -> Expectation | None:


The Go SDK (haven't looked at Java) names markers with as %s_%s, where the first parameter is the type of the marker (local activity, side effect, version, etc.) and the second is the contextual identifier for that marker (activity ID, side effect ID, version marker). I think our state machine will unfortunately have to do something similar and behave differently depending on the type. We could even consider a different state machine for each, but LocalActivities is really the only one that's massively different.

Adding a new Version Marker for example is totally fine and we don't want to create an Expectation for either the decision or the event.

Adding a new LocalActivity on the other hand should be flagged on nondeterminism. I think side effects are similar.

For LocalActivities we'll also have to do immediate cancellation and expose a Future and everything, but we can ignore that for now.

Thanks, I have made those changes.

natemort · 2026-06-26T18:01:43Z

        )

-    def _validate_expectation(self, actual: Expectation) -> None:
+    def _validate_expectation(self, actual: Expectation) -> Expectation:


Nit: Can you document the new return value?

Added comments for checking expectation.

Signed-off-by: Tim Li <ltim@uber.com>

gitar-bot · 2026-07-06T18:39:55Z

Code Review 👍 Approved with suggestions 1 resolved / 2 findings

Implements the Marker state machine for tracking decision lifecycles and enforcing replay determinism. Update decode_marker_details to ensure foreign or raw payloads are handled correctly.

💡 Edge Case: Marker detail decoding can misread foreign/raw payloads

📄 cadence/_internal/workflow/statemachine/marker_state_machine.py:38-52 📄 cadence/_internal/workflow/statemachine/decision_manager.py:291-305

decode_marker_details (marker_state_machine.py) treats any Details blob whose first 4 bytes form a length that fits within the payload and whose next ctx_len bytes are valid UTF-8 as an encoded (context_id, user_data) pair. A raw marker Details value produced by another SDK, or a pre-encoding history event, can accidentally satisfy this heuristic, causing a bogus context_id to be extracted and the wrong slice returned as user_data. This then feeds _index_marker_details / _state_machine_for_marker_event in decision_manager.py, potentially returning a truncated/incorrect value to the caller of side_effect on replay instead of falling through to the context_id is None (foreign-SDK) path.

Within a pure Python client this is unlikely because every marker this SDK writes is length-prefixed, so the risk is low; hence minor. If cross-SDK histories are ever replayed, consider adding a distinguishing magic prefix/version byte to the encoding so foreign payloads are reliably rejected rather than heuristically parsed.

✅ 1 resolved

✅ Quality: Marker event routing silently no-ops on unknown ids

📄 cadence/_internal/workflow/statemachine/decision_manager.py:241-248 📄 cadence/_internal/workflow/statemachine/decision_manager.py:273-282
_state_machine_for_marker_event returns None (causing handle_history_event to return early as a no-op) whenever a marker has no SDK id or no matching state machine, whereas _state_machine_for_event raises KeyError for unknown non-marker events. This asymmetry is intentional for the preload phase (markers are processed before workflow code runs), and genuine non-determinism is still caught by the DeterminismTracker (complete_replay reports unmet expectations, since expectations are added from decision_events.output). However, a foreign/internal marker carrying the SDK header but with no corresponding machine during the Phase-4 output pass would be silently dropped without any log. Consider adding a debug log in the machine is None branch so unexpected marker routing is observable, since the silent path can mask wiring issues that aren't covered by the determinism layer (e.g. when running without replay validation).

🤖 Prompt for agents

Code Review: Implements the Marker state machine for tracking decision lifecycles and enforcing replay determinism. Update `decode_marker_details` to ensure foreign or raw payloads are handled correctly.

1. 💡 Edge Case: Marker detail decoding can misread foreign/raw payloads
   Files: cadence/_internal/workflow/statemachine/marker_state_machine.py:38-52, cadence/_internal/workflow/statemachine/decision_manager.py:291-305

   `decode_marker_details` (marker_state_machine.py) treats any Details blob whose first 4 bytes form a length that fits within the payload and whose next `ctx_len` bytes are valid UTF-8 as an encoded `(context_id, user_data)` pair. A raw marker Details value produced by another SDK, or a pre-encoding history event, can accidentally satisfy this heuristic, causing a bogus `context_id` to be extracted and the wrong slice returned as `user_data`. This then feeds `_index_marker_details` / `_state_machine_for_marker_event` in decision_manager.py, potentially returning a truncated/incorrect value to the caller of `side_effect` on replay instead of falling through to the `context_id is None` (foreign-SDK) path.
   
   Within a pure Python client this is unlikely because every marker this SDK writes is length-prefixed, so the risk is low; hence minor. If cross-SDK histories are ever replayed, consider adding a distinguishing magic prefix/version byte to the encoding so foreign payloads are reliably rejected rather than heuristically parsed.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

`Auto-apply`	`Compact`
`gitar auto-apply:on`	`gitar display:verbose`

_{Was this helpful? React with 👍 / 👎 | Gitar}

natemort · 2026-07-06T19:04:07Z

+    Format: 4-byte big-endian length of context_id, then context_id UTF-8, then user_data.
+    """
+    ctx = context_id.encode("utf-8")
+    return len(ctx).to_bytes(4, "big") + ctx + user_data


I think the other SDKs align on this at least being a JSON object, let's do the same rather than introducing a more complex encoding.

I think we can punt that to a subsequent PR as well. For the state machine and non-determinism I don't think we really care what specifically is in the Marker, just the type (which we infer from the name).

When adding LocalActivities or other things we might alter the non-determinism

Thanks, they are now encoded as a json object.

natemort · 2026-07-06T19:07:53Z

+    if 4 + ctx_len > len(data):
+        return None, data
+    try:
+        context_id = data[4 : 4 + ctx_len].decode("utf-8")


We can parse this from the marker name rather than the data.

since we encode them as json object, for decoding part, we can just json.decode to get the context_id and data

natemort · 2026-07-06T19:09:31Z

+LOCAL_ACTIVITY_MARKER_NAME = "LocalActivity"
+MUTABLE_SIDE_EFFECT_MARKER_NAME = "MutableSideEffect"
+
+KNOWN_MARKER_NAMES = frozenset(


One additional one is the immediate cancellation marker Cancel_ which is unique to Python.

I added a cancel bypass in decision_manager.

natemort · 2026-07-06T19:09:59Z

+        return None
+    return Expectation(
+        DecisionId(DecisionType.MARKER, f"{attrs.marker_name}_{context_id}"),
+        {"marker_name": attrs.marker_name},


We can have an empty expectation body, just the correct DecisionID is enough for now. That covers type and name. When we add LocalActivities and other things with more parameters we might introduce additional checks, but we want to be really specifi with them.

Signed-off-by: Tim Li <ltim@uber.com>

gitar-bot · 2026-07-21T02:57:47Z

Code Review ✅ Approved 2 resolved / 2 findings

Implements the MarkerStateMachine and side_effect API to ensure deterministic marker recording and replay. Resolved silent no-ops on unknown marker IDs and potential payload decoding errors.

✅ 2 resolved

✅ Quality: Marker event routing silently no-ops on unknown ids

📄 cadence/_internal/workflow/statemachine/decision_manager.py:241-248 📄 cadence/_internal/workflow/statemachine/decision_manager.py:273-282
_state_machine_for_marker_event returns None (causing handle_history_event to return early as a no-op) whenever a marker has no SDK id or no matching state machine, whereas _state_machine_for_event raises KeyError for unknown non-marker events. This asymmetry is intentional for the preload phase (markers are processed before workflow code runs), and genuine non-determinism is still caught by the DeterminismTracker (complete_replay reports unmet expectations, since expectations are added from decision_events.output). However, a foreign/internal marker carrying the SDK header but with no corresponding machine during the Phase-4 output pass would be silently dropped without any log. Consider adding a debug log in the machine is None branch so unexpected marker routing is observable, since the silent path can mask wiring issues that aren't covered by the determinism layer (e.g. when running without replay validation).

✅ Edge Case: Marker detail decoding can misread foreign/raw payloads

📄 cadence/_internal/workflow/statemachine/marker_state_machine.py:38-52 📄 cadence/_internal/workflow/statemachine/decision_manager.py:291-305
decode_marker_details (marker_state_machine.py) treats any Details blob whose first 4 bytes form a length that fits within the payload and whose next ctx_len bytes are valid UTF-8 as an encoded (context_id, user_data) pair. A raw marker Details value produced by another SDK, or a pre-encoding history event, can accidentally satisfy this heuristic, causing a bogus context_id to be extracted and the wrong slice returned as user_data. This then feeds _index_marker_details / _state_machine_for_marker_event in decision_manager.py, potentially returning a truncated/incorrect value to the caller of side_effect on replay instead of falling through to the context_id is None (foreign-SDK) path.

Within a pure Python client this is unlikely because every marker this SDK writes is length-prefixed, so the risk is low; hence minor. If cross-SDK histories are ever replayed, consider adding a distinguishing magic prefix/version byte to the encoding so foreign payloads are reliably rejected rather than heuristically parsed.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

`Auto-apply`	`Compact`
`gitar auto-apply:on`	`gitar display:verbose`

_{Was this helpful? React with 👍 / 👎 | Gitar}

marker state machine

9f6db45

Signed-off-by: Tim Li <ltim@uber.com>

gitar-bot Bot reviewed Jun 24, 2026

View reviewed changes

Comment thread cadence/_internal/workflow/statemachine/decision_manager.py

Merge branch 'main' into ltim/marker-1

c418009

natemort reviewed Jun 26, 2026

View reviewed changes

timl3136 and others added 2 commits July 6, 2026 10:45

Merge branch 'main' into ltim/marker-1

f30f0ed

address comments

823fbc1

Signed-off-by: Tim Li <ltim@uber.com>

gitar-bot Bot reviewed Jul 6, 2026

View reviewed changes

Comment thread cadence/_internal/workflow/statemachine/marker_state_machine.py Outdated

natemort reviewed Jul 6, 2026

View reviewed changes

timl3136 added 3 commits July 6, 2026 14:16

address more comments

eb67915

Signed-off-by: Tim Li <ltim@uber.com>

Merge remote-tracking branch 'upstream/main' into ltim/marker-1

52cb7f3

address even more comments

19616f3

Signed-off-by: Tim Li <ltim@uber.com>

shijiesheng approved these changes Jul 20, 2026

View reviewed changes

Merge upstream/main into ltim/marker-1

58fd743

timl3136 merged commit 817c33c into cadence-workflow:main Jul 21, 2026
8 checks passed

Uh oh!

Conversation

timl3136 commented Jun 24, 2026

Uh oh!

codecov Bot commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gitar-bot Bot commented Jul 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gitar-bot Bot commented Jul 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Jun 24, 2026 •

edited

Loading