Skip to content

ADR 0032: Thread compaction for durable agents and workflows - #88

Open
Ahmed Muhsin (ahmedmuhsin) wants to merge 13 commits into
feature/python-durable-thread-compactionfrom
python/adr-0032-durable-thread-compaction
Open

ADR 0032: Thread compaction for durable agents and workflows#88
Ahmed Muhsin (ahmedmuhsin) wants to merge 13 commits into
feature/python-durable-thread-compactionfrom
python/adr-0032-durable-thread-compaction

Conversation

@ahmedmuhsin

Copy link
Copy Markdown
Contributor

Design review for ADR 0032, ahead of the implementation PRs.

The ADR separates three pressures that usually get conflated. Context window and token cost are per-operation, and core's compaction already handles both. Storage capacity is cumulative and durable-only, bounded by the backend, where DTS caps a message at 1 MB and Azure Storage offloads to blob instead. The decision is to reuse core compaction for the first two and add durable retention for the third.

Status is proposed with deciders unset, so this is the place to settle the design before any implementation lands.

Targets feature/python-durable-thread-compaction. Four stacked PRs follow, covering workflow context forwarding, the durable history provider with autoswap and ownership, session persistence, and retention with state budgeting.

#59 stays open as the integrated reference until the stack lands.

Comment thread docs/decisions/0032-durable-thread-compaction.md Outdated
Comment thread docs/decisions/0032-durable-thread-compaction.md Outdated
Comment thread docs/decisions/0032-durable-thread-compaction.md Outdated
Comment thread docs/decisions/0032-durable-thread-compaction.md Outdated
Comment thread docs/decisions/0032-durable-thread-compaction.md Outdated
Comment thread docs/decisions/0032-durable-thread-compaction.md Outdated
Comment thread docs/decisions/0032-durable-thread-compaction.md Outdated
@larohra

Copy link
Copy Markdown

The provider/entity split is the right foundation: core compaction should decide what the model reads, while the durable layer independently manages storage capacity. My concern is that the proposed state model still asks conversationHistory to serve too many different lifecycles at once: model transcript, response delivery, duplicate suppression, provider state, workflow transport, and sometimes an audit record.

I recommend keeping Option 6, but decomposing the durable contracts:

  1. One authoritative history owner per durable session. Pin durable state, an external provider, or the model service as the primary owner. Treat ownership changes as explicit migration or fork operations rather than interpreting store independently on every turn.
  2. A response mailbox separate from the evictable transcript. Keep pending/completed response payloads or references until acknowledgement/expiry, then retain lightweight correlation tombstones so duplicate delivery cannot rerun the model or tools after payload cleanup.
  3. Provider-owned storage lifecycle. Add replace/flush, clear/delete, versioned snapshot/restore, and resolved ownership capabilities to the provider abstraction. Enforce one load-enabled primary provider and allow additional store-only audit/evaluation providers.
  4. Byte-exact, backend-aware retention. Measure the same serialized bytes the backend sees, subtract the non-evictable state floor, and evict only explicit atomic transcript groups. Keep automatic deletion opt-in until delivery and compatibility invariants are complete.
  5. Bound workflow transport independently. Send per-target unseen deltas or transcript references instead of repeatedly serializing full_conversation; execute custom projection in an activity if arbitrary user code must be kept out of orchestration replay.
  6. Define cross-language state evolution. Cover mixed-version workers, rollback, unknown entry preservation, and bidirectional Python/.NET state tests before persisting new polymorphic entry kinds.

This does add some up-front structure, but it replaces hidden interaction complexity with explicit ownership and lifecycle rules. It does not all need to land at once. A practical sequence would be: enforce the owner/provider invariants, separate responses and tombstones, ship exact opt-in retention, then add the broader provider and workflow transport capabilities. That preserves the ADR's configuration-parity goal without making response correctness, reset semantics, or transport safety depend on retention heuristics.

Separate compaction pruning from pressure eviction, isolate response delivery and duplicate suppression, bound workflow transport with per-target deltas, and define provider and state-evolution follow-ups.
@ahmedmuhsin

Copy link
Copy Markdown
Contributor Author

Thanks, this materially improved the design. The ADR now separates transcript retention, response delivery, duplicate suppression, provider state, workflow redelivery, and truncation evidence. Deletion is opt-in through independent compaction and pressure controls, workflow transport uses per-target deltas, provider cardinality is enforced, and cross-language rollout is reader-first. Per-run ownership remains aligned with core, while migrate/fork and provider lifecycle capabilities are recorded as follow-up work.

Separate execution and delivery from transcript ownership. Clarify the A1 compatibility trade-off, custom-ID deduplication, and migration for workers and polling clients, with updated visuals.

@larohra Laveesh Rohra (larohra) left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The revised ADR is materially stronger and addresses the major concerns from the first review. Execution and delivery are now independent from transcript ownership, deletion is opt-in, workflow transport is bounded by per-target deltas, provider cardinality is explicit, and cross-language rollout is reader-first.

The remaining comments are implementation requirements and longer-term lifecycle concerns rather than reasons to block the design. In particular, completion bookkeeping needs an eventual bound, owner transitions need explicit tests, custom filters need a monotonicity constraint for cursor-based deltas, and external-provider appends need an idempotency contract. With those tracked, I am comfortable approving the ADR.

Comment thread docs/decisions/0032-durable-thread-compaction.md Outdated
Comment thread docs/decisions/0032-durable-thread-compaction.md Outdated
Comment thread docs/decisions/0032-durable-thread-compaction.md Outdated
Comment thread docs/decisions/0032-durable-thread-compaction.md Outdated
Separate the proposed contracts from prototype evidence, consolidate repeated explanations, and preserve the existing diagrams and compatibility requirements.
@tamirdresher

Copy link
Copy Markdown
Contributor

Great work, Ahmed Muhsin (@ahmedmuhsin). I support this direction. There are some differences in how MAF is designed in C# and Python, so I think we should make the runtime-specific behavior explicit in the ADR rather than imply everything works the same way.
Separating execution and result delivery from the transcript makes sense to me. We should adopt that in C# too, so removing old context cannot remove a pending result or make a completed request run again.

A few questions and suggestions:

  1. History ownership: For C#, I’d prefer to keep the history owner selected when the session starts and resume that same inner provider or service conversation on later calls. That seems like a valid, simpler contract. Could we document per-run store switching as a separate capability rather than something every runtime must implement?

  2. Compaction support: C# currently cannot safely support FollowCompaction with the stateful compaction representation. Persisting that state duplicates messages, while dropping it loses exclusions and incremental summarization state. Could we make the supported capabilities explicit per runtime? I’d suggest deferring FollowCompaction from the shared baseline until .NET can support it safely.

  3. Pressure retention and defaults: The C# Auto mode removes old history under byte pressure. It does not follow compaction exclusions. Keeping that distinction clear will help users understand what can delete their data. Should we align C# with the ADR’s non-deleting default and make pressure eviction opt-in?

  4. Mailbox migration: I agree with the compatibility requirements here. For C#, duplicate lookup and poller still read transcript responses. Could we identify the worker and client versions needed before enabling mailbox writes, including supported deployment rollback behavior? Preserving the new JSON fields alone would not make an old worker behave correctly.

  5. Coverage and observability: Could we include mixed text, media and file messages, large tool results, and a newest message that cannot fit even after retention? I’d also like OTel measurements in the examples and tests. I added a C# retention metrics that could be a starting point, but we should pair them with assertions on actual model input and persisted state, since an emitted metric does not necessarily mean the state change committed.

Comment thread docs/decisions/0032-durable-thread-compaction.md Outdated
Comment thread docs/decisions/0032-durable-thread-compaction.md
Comment thread docs/decisions/0032-durable-thread-compaction.md
Comment thread docs/decisions/0032-durable-thread-compaction.md
Comment thread docs/decisions/0032-durable-thread-compaction.md Outdated
| `retention` | `keep_all` **(default)** | Do not delete merely because compaction excluded a message. |
| `retention` | `follow_compaction` | Prune excluded local messages after each turn. Without compaction there are no exclusions to prune. |
| `max_state_bytes` | `None` **(default)** | Disable pressure eviction. The backend can still reject an oversized write. |
| `max_state_bytes` | `"backend_limit"` | Use the host's known hard payload limit, 1,048,576 bytes for direct DTS. Fail registration if unresolved. |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im not if we need this value, if you set to none, the backend limit will be enforced anyway, and if you want it to be enforced in the client side, then just set to the explicit bytes value

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right that None still leaves the backend's hard limit enforced. The distinction is that an explicit budget enables retention before the write, rather than waiting for the backend to reject it. backend_limit is only a Python convenience for selecting a known DTS budget, not a correctness requirement or a guarantee that transport framing will fit. The ADR now makes None and explicit bytes the portable choices; leaving this open to settle whether the convenience is worth keeping.


## Validation Requirements

The following are acceptance requirements for the proposed implementation, not claims about the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahmed Muhsin (@ahmedmuhsin) have you performed testing of media and inline file data? we need to see how these behave and not only text based messages

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially. #59 includes inline-data/file-reference round trips, mixed binary/text tool results, large-tool pressure tests and oversized-newest-turn protection. What it does not yet establish is mixed media under retention with persisted-state readback and the subsequent model input; the live suites are still text-based. That is a remaining validation gap, not a deferred feature, so this should stay open until that coverage is added.

@ahmedmuhsin

Copy link
Copy Markdown
Contributor Author

Tamir Dresher (@tamirdresher)

Updated the ADR to separate shared invariants from runtime-specific ownership and compaction capabilities, with non-deleting defaults and explicit consumer/rollback requirements. #59 is the published prototype reference. The evidence section now distinguishes already-tested large-tool and oversized-turn behavior from the missing combined media/failure tests and retention-specific OTel instrumentation. Those gaps remain implementation work, not implied deferrals. Could you link the C# retention metrics change so we can align that instrumentation?

@tamirdresher

Copy link
Copy Markdown
Contributor

In reply to Ahmed's comment:

Sure. The C# retention instrumentation is in DurableAgentTelemetry.cs, introduced in commit 12aff1d4.

It measures retention operations, evicted messages and entries, reclaimed bytes, and state size before and after retention. The measurements describe the retention attempt, so the tests also need to verify the persisted state and the actual model input rather than treating the metric alone as proof that the state change committed.

I have a revised version locally that aligns this with the mailbox separation and removes terminal-result eviction. I have not pushed that revision yet because we are first agreeing on the shared schema contract in #92. Once that is settled, I can update the implementation link.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants