ADR 0032: Thread compaction for durable agents and workflows - #88
Conversation
|
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 I recommend keeping Option 6, but decomposing the durable contracts:
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.
|
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.
Laveesh Rohra (larohra)
left a comment
There was a problem hiding this comment.
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.
Separate the proposed contracts from prototype evidence, consolidate repeated explanations, and preserve the existing diagrams and compatibility requirements.
|
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. A few questions and suggestions:
|
| | `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. | |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
|
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? |
|
In reply to Ahmed's comment: Sure. The C# retention instrumentation is in 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. |
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
proposedwith 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.