Story 5.3: Phase 2/3 roadmap + async and framework design intent (docs only) - #20
Merged
usmanabbas7 merged 2 commits intoApr 28, 2026
Merged
Conversation
…intent Story 5.3 — per its own framing, this is a design-intent + audit story, not an implementation story. Implementing AsyncCore/AsyncContext and the three framework helper distributions (convert-sdk-django, convert-sdk-fastapi, convert-sdk-flask) remains gated on explicit Phase 3 sign-off. What landed (docs only): docs/roadmap.md — Phase 1 (MVP shipped), Phase 2 (long-running service support; auto-refresh shipped today, durable storage / retry tuning / structured logging guide planned), Phase 3 (async public API + framework integrations + OpenTelemetry, all planned). docs/async.md — Phase 3 design intent. Records the architecture's five frozen rules for the sync-async coexistence model (parallel API not replacement; shared evaluation core; async transport adapter; DataStore stays sync with AsyncDataStore added in parallel; SDK never owns the event loop). Includes a per-module forward-compatibility audit table covering every MVP file. Bottom line: no MVP module needs to change to enable async — the architecture's "MVP must not be async-hostile" preparation work was effective. Public method names (run_experience, run_feature, track_conversion, refresh_now, close) already work as-is for both surfaces. Framework helpers ship as separate distributions; core stays framework-free per NFR13. Open questions (async transport boundary, DataStore Protocol shape, async event-bus) are flagged as Phase-3-sign-off decisions rather than pre-empted. Forward references — docs/index.md gains a "Looking ahead" table; docs/migration-from-javascript.md, docs/migration-from-rest.md, and docs/extending.md each gain a brief "Future async / framework support" note pointing at the new roadmap and async docs. No code changes. Sync-first MVP is unaffected; all release gates green (227 tests pass, project coverage 94.03%, evaluation/ 97.84%, parity 7/7). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request adds documentation for the project's roadmap and async design intent, specifically introducing docs/roadmap.md and docs/async.md. These documents outline a phased expansion plan and the architectural strategy for maintaining a sync-first MVP while preparing for parallel async support. Feedback was provided to resolve a contradiction in docs/async.md regarding the AsyncDataStore protocol definition.
Comment on lines
+178
to
+180
| - **DataStore Protocol shape**: a separate `AsyncDataStore` Protocol | ||
| vs. a dual-protocol convention (one Protocol, two suites of | ||
| methods — e.g., `load_context_state` / `aload_context_state`). |
There was a problem hiding this comment.
This open question appears to contradict one of the 'Five frozen rules for the async surface' (rule #4 on line 65), which states that AsyncDataStore will be a new sibling Protocol. Questioning the fundamental shape of the protocol here could be confusing for readers.
To improve consistency, I suggest rephrasing this point to focus on the implementation details of the AsyncDataStore protocol, rather than questioning its existence as a separate protocol. This would better align with the 'frozen' nature of the rules established earlier in the document.
Suggested change
| - **DataStore Protocol shape**: a separate `AsyncDataStore` Protocol | |
| vs. a dual-protocol convention (one Protocol, two suites of | |
| methods — e.g., `load_context_state` / `aload_context_state`). | |
| - **DataStore Protocol details**: What should be the exact method signatures within the `AsyncDataStore` Protocol? Should they mirror the sync `DataStore` methods with an `a` prefix (e.g., `aload_context_state`), or is there a better convention to follow? |
The adversarial review of PR #20 surfaced four documentation issues: 1. The forward-compatibility audit table marked the in-memory event bus, in-memory data store, and tracking queue as "✅ ready" with notes that walked the verdict back. From an async caller these modules block the event loop unless wrapped — they are "➕ wrap", not "drop-in async-ready." Verdict legend now distinguishes ✅ ready (reusable verbatim from a coroutine), ➕ sibling (add a parallel async class), and ➕ wrap (reach it through an async adapter or to_thread()). 2. The Open Questions section listed "Async transport boundary" as undecided while the rest of the doc described HttpxAsyncTransport as a concrete plan. The section is rewritten to identify which leanings are firm and which axes are still open at Phase 3 sign-off, plus a fourth open question (executor sizing for the to_thread() fallback) that the original list omitted. 3. The "Looking ahead" panel in docs/index.md placed AsyncCore / AsyncContext examples one click away from new users. The header now explicitly reads "planned, not yet shipped" with a paragraph warning readers that the symbols are not importable today. 4. The migration-from-REST guide recommended asyncio.to_thread() as the bridge for async hosts but did not mention that to_thread runs on the bounded default executor (min(32, cpu_count + 4) workers on CPython). A FastAPI service receiving hundreds of concurrent requests can serialise SDK calls behind that cap. A warning admonition and an executor-resize recipe are added. The aclose() naming choice is now justified explicitly (the asyncio convention is to spell shutdown methods with the a* prefix; reusing the sync name would mislead readers about what is awaitable). No code changes; sync-first MVP is unaffected. 204 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
AsyncCore/AsyncContext) and the three framework helper distributions (convert-sdk-django,convert-sdk-fastapi,convert-sdk-flask) remains gated on explicit Phase 3 sign-off.docs/roadmap.md,docs/async.md) and forward-references in 4 existing docs.What landed (docs only)
docs/roadmap.md— Phase 1 (MVP shipped), Phase 2 (long-running service support; auto-refresh shipped, durable storage / retry tuning / structured logging guide planned), Phase 3 (async public API + framework integrations + OpenTelemetry, all planned). Tracks shipped vs planned status against the architecture's deferral discipline and the PRD's Module Phase Table.docs/async.md— Phase 3 design intent in detail:httpx.AsyncClient;DataStorestays sync withAsyncDataStoreadded in parallel; SDK never owns the event loop).run_experience/run_feature/track_conversion/refresh_now/closework as-is for both surfaces;AsyncCore.aclose()follows the asyncio convention.AsyncClientadapter vsto_thread()wrapper),DataStoreProtocol shape, async event-bus — flagged as Phase-3-sign-off decisions rather than pre-empted.Forward references —
docs/index.mdgains a "Looking ahead" table;docs/migration-from-javascript.md,docs/migration-from-rest.md, anddocs/extending.mdeach gain a brief "Future async / framework support" note. The REST migration guide also notes that today's async-Python users can call the sync SDK from async code viaasyncio.to_thread()until the async surface ships.Audit findings (summary)
evaluation/*,domain/*tracking/payloads.py,tracking/conversions.pydiagnostics.py,errors.py,events.pyadapters/storage/in_memory.py,adapters/events/in_memory_event_bus.pyasyncio.to_thread())tracking/queue.pyports/transport.py,ports/storage.pyAsyncTransport/AsyncDataStoresiblings)adapters/transport/httpx_transport.pyHttpxAsyncTransport)config_loader/loader.py,config_loader/refresh.pycore.py,context.pyAsyncCore,AsyncContext)No MVP module needs to change to enable async. The seams above are "add a sibling", never "rewrite". The architecture's preparation work paid off.
Out-of-scope (gated on Phase 3 sign-off)
Per Story 5.3's Critical Warning #1 — "Do not ship any of this story's code as part of the MVP release. Opening it for implementation requires explicit Phase 3 sign-off." — the following are explicitly NOT in this PR:
src/convert_sdk/asyncio/package implementationsrc/convert_sdk/adapters/transport/httpx_async_transport.pyadapterconvert-sdk-django,convert-sdk-fastapi,convert-sdk-flaskseparate distributionsThese are recorded in
docs/async.mdas planned shape; opening any of them requires explicit sign-off.Test plan
docs/async.mdfor design completeness against the architecture's "deliberately deferred" list.Story tracking
5-3-add-planned-expansion-surfaces-for-async-and-framework-integrations.md— status flippedbacklog→reviewwith full Dev Agent Record covering the audit verdicts and the explicit out-of-scope list.in-progress; story 5-3backlog→review.🤖 Generated with Claude Code