diff --git a/docs/adr/adr-014-hexagonal-architecture-enforcement.md b/docs/adr/adr-014-hexagonal-architecture-enforcement.md index 4e214351..5aa30a39 100644 --- a/docs/adr/adr-014-hexagonal-architecture-enforcement.md +++ b/docs/adr/adr-014-hexagonal-architecture-enforcement.md @@ -12,9 +12,9 @@ SDK integrations live in adapters. Ruff enforces general import hygiene, but it does not know the repository's dependency graph. A module can therefore import in the wrong direction while still satisfying ordinary lint rules. -The immediate need is roadmap item `1.5.4`: enforce the current service -scaffold boundaries. The deeper orchestration-specific checks for LangGraph -nodes, Celery task payloads, and checkpoint state remain roadmap item `2.4.5`. +The immediate need was roadmap item `1.5.4`: enforce the current service +scaffold boundaries. ADR-016 records the later orchestration-specific checks +for LangGraph nodes, Celery task payloads, and checkpoint state. ## Decision @@ -73,8 +73,8 @@ published structural surface. - Constraint-name constants used by service-layer conflict handling now live in `episodic.canonical.constraints`. SQLAlchemy models import those constants rather than owning the only copy. -- `2.4.5` remains responsible for LangGraph-node-specific policies, Celery - checkpoint payload audits, and deeper orchestration checks. +- ADR-016 extends this base policy with LangGraph-node-specific policies, + Celery task checks, and checkpoint payload audits. - Hecate replaces the former repo-local `episodic.architecture` checker. New architecture groups are added in `pyproject.toml`; generic checker semantics belong upstream in Hecate. @@ -87,7 +87,8 @@ Hecate adoption ExecPlan: `docs/execplans/adopt-hecate.md`.[^3] Hecate configuration: `[tool.hecate]` in `pyproject.toml`.[^4] Tests: `tests/test_architecture_enforcement.py`, `tests/test_port_contracts.py`, `tests/features/architecture_enforcement.feature`, and -`tests/steps/test_architecture_enforcement_steps.py`.[^5] +`tests/steps/test_architecture_enforcement_steps.py`.[^5] Orchestration +enforcement extension: ADR-016.[^6] [^1]: Roadmap items `1.5.4` and `2.4.5` in `docs/roadmap.md` [^2]: ExecPlan: @@ -98,3 +99,5 @@ configuration: `[tool.hecate]` in `pyproject.toml`.[^4] Tests: `tests/test_port_contracts.py`, `tests/features/architecture_enforcement.feature`, and `tests/steps/test_architecture_enforcement_steps.py` +[^6]: Orchestration architecture enforcement: + `docs/adr/adr-016-orchestration-architecture-enforcement.md` diff --git a/docs/adr/adr-016-orchestration-architecture-enforcement.md b/docs/adr/adr-016-orchestration-architecture-enforcement.md new file mode 100644 index 00000000..cfe38ee3 --- /dev/null +++ b/docs/adr/adr-016-orchestration-architecture-enforcement.md @@ -0,0 +1,97 @@ +# ADR-016: Orchestration architecture enforcement + +## Status + +Accepted, 2026-06-26. LangGraph node modules, Celery task modules, and +orchestration checkpoint payload modules are enforced as dedicated Hecate +groups. + +## Date + +2026-06-26. + +## Context and problem statement + +ADR-014 introduced Hecate as the import-boundary checker for the core hexagonal +architecture. That policy covered domain, application, adapter, and +composition-root modules, but roadmap item `2.4.5` still needed deeper +orchestration-specific checks. + +The risk was concentrated in three places: + +- LangGraph nodes could become convenient places to import storage, HTTP, or + vendor Software Development Kit (SDK) adapters directly. +- Celery task modules could bypass worker composition roots and instantiate + concrete infrastructure. +- Durable checkpoint payload DTOs could accrete canonical Object-Relational + Mapping (ORM) entities, provider SDK responses, or other non-JSON state. + +## Decision drivers + +- Preserve ports as the integration boundary for orchestration code. +- Keep LangGraph framework mechanics out of node functions. +- Keep Celery task modules independent of concrete worker runtime wiring. +- Keep checkpoint payloads provider-neutral and JSON-shaped. +- Make the policy visible in deterministic tests rather than relying only on + review discipline. + +## Decision outcome + +In the context of structured generation orchestration, facing boundary creep in +LangGraph nodes, Celery tasks, and durable checkpoint payloads, we decided for +dedicated Hecate groups plus structural checkpoint payload tests, and against a +single broad orchestration group or review-only convention, to achieve +deterministic import-boundary enforcement, accepting a more detailed +`pyproject.toml` group ordering and additional fixture maintenance. + +The accepted groups are: + +- `orchestration_nodes` for `episodic.orchestration._graph_nodes`, allowed to + depend on the `orchestration_checkpoint` DTO group and domain ports only. +- `orchestration` for graph builders, planning orchestration, and tool + execution policy, allowed to depend on application services, checkpoint DTOs, + and `orchestration_nodes`, but not adapters. +- `orchestration_tasks` for `episodic.worker.tasks`, allowed to depend on + domain services, domain ports, and `episodic.worker.workloads.WorkloadClass`. +- `orchestration_checkpoint` for checkpoint DTO and payload serialization + modules, allowed to depend on itself and domain-port value types only. + +`episodic.worker.workloads.WorkloadClass` is the canonical domain-port-like +worker contract, so task modules can describe workload routing without +importing the Celery runtime. `episodic.worker.topology.WorkloadClass` remains +an explicit compatibility alias only. + +## Consequences + +### Positive + +- `make lint` rejects adapter imports from LangGraph nodes, Celery tasks, and + checkpoint payload modules before review. +- The node/builder split keeps node functions small and easy to audit. +- Checkpoint payload DTOs are guarded by both Hecate and structural tests that + inspect field annotations. + +### Negative + +- Hecate group ordering now matters more. The dedicated + `orchestration_nodes` prefix must stay before the broader `orchestration` and + adapter prefixes. +- New orchestration fixtures must mirror production module prefixes closely or + they will not exercise the intended group. + +### Neutral + +- This decision does not change the public generation orchestration API. +- Durable checkpoint storage remains an outbound adapter that implements + `CheckpointPort`; it may import checkpoint DTOs to satisfy that port. + +## References + +See ADR-014 for the base Hecate adoption decision.[^1] See the orchestration +enforcement ExecPlan for the implementation milestones and validation +history.[^2] + +[^1]: Hexagonal architecture enforcement: + `docs/adr/adr-014-hexagonal-architecture-enforcement.md` +[^2]: Orchestration enforcement ExecPlan: + `docs/execplans/2-4-5-extend-architecture-enforcement-to-orchestration-code.md` diff --git a/docs/contents.md b/docs/contents.md index 1190d46a..04668ee9 100644 --- a/docs/contents.md +++ b/docs/contents.md @@ -101,6 +101,8 @@ or delivery planning. - source-intake upload storage and idempotency port decisions. - [ADR 016: Adopt Skylos dead-code detection](adr/adr-016-adopt-skylos-dead-code-detection.md) - blocking static dead-code detection and exception policy. +- [ADR 016: Orchestration architecture enforcement](adr/adr-016-orchestration-architecture-enforcement.md) + - LangGraph node, Celery task, and checkpoint payload enforcement decisions. - [ADR 017: No-QA generation execution and TEI persistence][adr-017] - generation launcher, draft persistence, recovery, and TEI retrieval decisions. @@ -162,6 +164,8 @@ or delivery planning. - orchestration checkpoint plan. - [Configure Celery queue routing](execplans/2-4-3-configure-celery-queue-routing.md) - worker routing plan. +- [Extend architecture enforcement to orchestration code](execplans/2-4-5-extend-architecture-enforcement-to-orchestration-code.md) + - orchestration architecture enforcement plan. - [LLM port adapter](execplans/3-2-1-llm-port-adapter.md) - large language model adapter plan. - [Introduce v1 target API prefix](execplans/4-1-1-introduce-v1-target-api-prefix.md) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 81317eb9..d4e0ab81 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -788,13 +788,33 @@ The enforced groups are: - `outbound_adapter`: SQLAlchemy storage, canonical ingestion adapters, and OpenAI-compatible LLM adapters, including `episodic.llm.openai_adapter`, the `episodic.llm.openai_api` helper package, and `episodic.llm.openai_client`. +- `orchestration_nodes`: LangGraph node functions under + `episodic.orchestration._graph_nodes`. This group may depend on the + `orchestration_checkpoint` DTO group and `domain_ports`, and must be ordered + before the broader `orchestration` group. +- `orchestration_checkpoint`: provider-neutral checkpoint payload DTO and + serialization modules. +- `orchestration`: LangGraph builders, graph state, planning orchestration, and + tool execution policy, excluding the dedicated node group. This group may + depend on `orchestration_nodes` because graph builders and the public facade + assemble and expose those nodes. +- `orchestration_tasks`: Celery task entrypoints. - `composition_root`: modules that wire concrete adapters, currently `episodic.api.runtime` and `episodic.worker.runtime`. When adding a new port or adapter, update `[tool.hecate]` in `pyproject.toml` -in the same change as the package. Keep composition-root prefixes before -broader adapter prefixes because Hecate uses first-match group ordering. Add or -adjust fixture coverage in `tests/fixtures/architecture/` and run: +in the same change as the package. Keep specific prefixes before broader +prefixes because Hecate uses first-match group ordering: `composition_root` +before adapter prefixes, `orchestration_nodes` before the broad `orchestration` +prefix, and `orchestration_tasks` before worker adapter prefixes. + +`episodic.worker.workloads.WorkloadClass` is the canonical worker workload +contract that task modules may import without pulling in the Celery app or +runtime wiring. `episodic.worker.topology.WorkloadClass` remains an explicit +compatibility alias only. Worker runtime modules own concrete Celery +configuration. + +Add or adjust fixture coverage in `tests/fixtures/architecture/` and run: ```shell uv run pytest -q tests/test_architecture_enforcement.py \ @@ -805,6 +825,13 @@ Port contract coverage lives in `tests/test_port_contracts.py`. Future behavioural tests that exercise real `LLMPort` inference paths should use Vidai Mock; structural conformance tests do not need an inference server. +For orchestration boundary fixtures, model the violating importer under the +same prefix Hecate will classify in production. Use +`orchestration/_graph_nodes.py` for node-only checks, `worker/tasks.py` for +Celery task checks, and `orchestration/_checkpoint_payload.py` for checkpoint +payload checks. Snapshot JSON diagnostics only after normalizing workspace +paths. + ### TEI payload compression Canonical TEI payload storage now supports transparent Zstandard compression @@ -1406,8 +1433,13 @@ Roadmap item `2.4.1` introduces a dedicated orchestration package in ### Package structure -- `episodic/orchestration/_dto.py` contains the orchestration DTOs and shared - checkpoint DTOs. +- `episodic/orchestration/_dto.py` contains public request/config DTOs and + compatibility re-exports. +- `episodic/orchestration/_payload_dto.py` contains provider-neutral planner, + plan, and action-result payload DTOs. +- `episodic/orchestration/_checkpoint_dto.py` and + `episodic/orchestration/_checkpoint_payload.py` contain checkpoint state DTOs + and JSON payload serialization helpers. - `episodic/orchestration/_protocols.py` contains the planner, executor, checkpoint, and resume ports that keep graph policy independent of storage, queue, and provider adapters. @@ -1421,9 +1453,13 @@ Roadmap item `2.4.1` introduces a dedicated orchestration package in `GuestBiosToolExecutor` implementation. It resolves the request's `series_profile_id`, optional `episode_id`, and optional `template_id` through the configured binding resolver before invoking the generation helper. -- `episodic/orchestration/langgraph.py` contains the in-process LangGraph path - used for `plan -> execute -> finish` and the checkpointing path that pauses - after planning. +- `episodic/orchestration/_graph_nodes.py` contains node functions that depend + on orchestration ports and DTOs only. +- `episodic/orchestration/_graph_builder.py` wires nodes, callbacks, and cost + recording into the compiled graph. +- `episodic/orchestration/langgraph.py` is the public graph facade for + `plan -> execute -> finish` and the checkpointing path that pauses after + planning. - `episodic/orchestration/checkpoints.py` contains the in-memory checkpoint adapter used by fast tests. - `episodic/canonical/storage/workflow_checkpoints.py` contains the SQLAlchemy diff --git a/docs/episodic-podcast-generation-system-design.md b/docs/episodic-podcast-generation-system-design.md index 4078e707..772c940c 100644 --- a/docs/episodic-podcast-generation-system-design.md +++ b/docs/episodic-podcast-generation-system-design.md @@ -21,6 +21,7 @@ Accepted decision records: - [ADR 013: Speech synthesis adapters](adr/adr-013-speech-synthesis-adapters.md) - [ADR 014: Hexagonal architecture enforcement](adr/adr-014-hexagonal-architecture-enforcement.md) - [ADR 015: Upload and idempotency ports](adr/adr-015-upload-and-idempotency-ports.md) +- [ADR 016: Orchestration architecture enforcement](adr/adr-016-orchestration-architecture-enforcement.md) - [ADR 018: Explicit repository-written versioning and history strategy](adr/adr-018-explicit-versioning-and-history-strategy.md) - [ADR 019: Retrievable episode TEI revision history](adr/adr-019-episode-tei-revision-history.md) @@ -118,14 +119,15 @@ Boundary rules: domain and ports, but never on outbound adapter implementations. - **Outbound adapters** (database, object storage, message broker, LLM/TTS vendors) depend on the domain and ports, but never on inbound adapters. -- **Orchestration code** (LangGraph nodes and Celery tasks) will depend on - domain services and ports only; direct adapter access is reserved for the - later orchestration-specific enforcement slice. +- **Orchestration code** (LangGraph nodes and Celery tasks) depends on domain + services, ports, and provider-neutral orchestration DTOs only. Direct adapter + access is rejected by orchestration-specific Hecate groups. - **Cross-adapter imports** are forbidden; interactions happen through ports or well-defined message schemas. -- **Checkpoint payloads** should hold orchestration metadata; canonical domain - state is persisted through repositories rather than state blobs. Dedicated - checkpoint audits are part of the later orchestration enforcement slice. +- **Checkpoint payloads** hold orchestration metadata and JSON-shaped + provider-neutral DTOs; canonical domain state is persisted through + repositories rather than state blobs. Hecate grouping and structural tests + audit this boundary. For screen readers: The following class diagram shows the module categories used by the Hecate architecture checker, the allowed dependency directions @@ -209,11 +211,13 @@ Enforcement mechanisms: adherence as part of `make test`. - The current Hecate policy covers canonical domain and port modules, application services, Falcon and worker adapter seams, SQLAlchemy and LLM - outbound adapters, and explicit composition roots. + outbound adapters, explicit composition roots, LangGraph node modules, Celery + task modules, and orchestration checkpoint payload modules. - Contract tests exercise port behaviour against adapter implementations, so adapters are verified without coupling to infrastructure in the domain. -- Roadmap item `2.4.5` extends the same mechanism to LangGraph-node-specific - imports, Celery task policies, and checkpoint payload boundaries. +- Roadmap item `2.4.5` delivered LangGraph-node-specific imports, Celery task + policies, and checkpoint payload boundary enforcement. ADR-016 records the + orchestration-specific decisions. - Code review checklists enforce idempotency keys, single-responsibility task scope, and checkpoint payload audits for orchestration changes. @@ -223,6 +227,13 @@ The following rules are normative for LangGraph nodes and Celery tasks: - Orchestration code depends on domain services and ports only; adapters are accessed exclusively through port interfaces. +- LangGraph node modules are classified separately from graph builders. Nodes + may depend on orchestration DTOs and ports, whilst builders and application + orchestration code may assemble domain services. +- Celery task modules depend on `WorkloadClass`, domain services, and ports; + worker runtime modules remain composition roots for concrete wiring. +- Checkpoint payload DTO modules are grouped before general orchestration + modules so Hecate's first-match ordering keeps them provider-neutral. - Celery tasks are single-responsibility and idempotent, with idempotency keys persisted per task or workflow step. - Checkpoint payloads store orchestration metadata only; canonical domain data diff --git a/docs/execplans/2-4-5-extend-architecture-enforcement-to-orchestration-code.md b/docs/execplans/2-4-5-extend-architecture-enforcement-to-orchestration-code.md new file mode 100644 index 00000000..955b5e28 --- /dev/null +++ b/docs/execplans/2-4-5-extend-architecture-enforcement-to-orchestration-code.md @@ -0,0 +1,892 @@ +# Extend architecture enforcement to orchestration code (2.4.5) + +This ExecPlan (execution plan) is a living document. The sections `Constraints`, +`Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, `Decision Log`, +and `Outcomes & Retrospective` must be kept up to date as work proceeds. + +Status: COMPLETE + +## Purpose / big picture + +Episodic enforces hexagonal architecture (ports and adapters) with the Hecate +import checker, run by `make check-architecture` (a dependency of `make lint`) +against the `[tool.hecate]` configuration in `pyproject.toml`. Today the +orchestration code is lumped into the generic `application` group and the +Celery worker tasks sit in the permissive `inbound_adapter` group. The system +design explicitly reserved orchestration-specific enforcement for this roadmap +slice (see `docs/episodic-podcast-generation-system-design.md`, the "Hexagonal +architecture enforcement" section, which states that "direct adapter access is +reserved for the later orchestration-specific enforcement slice"). + +Roadmap item 2.4.5 (`docs/roadmap.md`) asks us to: + +1. Validate LangGraph nodes depend on ports only. +2. Validate Celery tasks depend on ports only. +3. Audit checkpoint payload boundaries. + +After this change, a developer who tries to import a concrete adapter, storage +or vendor SDK into a LangGraph node, a Celery task, or a checkpoint payload DTO +will see `make lint` fail with a Hecate `ARCH001` violation, and a structural +test will fail if a checkpoint payload DTO grows a field whose type is not +provider-neutral (for example an ORM model or a canonical aggregate entity). +Success is observable by running `make lint` and `make test`: new architecture +fixtures and tests fail before the enforcement is added and pass after, while +the production `hecate check` continues to pass. + +This work is preventative: the current orchestration code is already free of +adapter imports, so the new rules pass once the supporting refactors land. The +value is named, fixtured, regression-tested boundaries plus two genuine +boundary fixes (Celery tasks reaching a `kombu`-coupled module for an enum, and +checkpoint DTOs coupling to application-tier generation DTOs). + +## Constraints + +Hard invariants that must hold throughout implementation. Violation requires +escalation, not a workaround. + +- The production `hecate check` (via `make check-architecture` and `make lint`) + must pass at the end of every milestone. Never weaken a rule to make + unrelated code pass; fix the boundary instead. +- Hecate behaviour is fixed at the pinned commit + `46f8c8798e7a80a3a1ab5a13c2a000a4423ffc12`. Do not bump the Hecate pin in + this slice. The enforcement model is allow-list only, first-match by config + order, with no per-rule identifiers; design within those limits. +- Public, importable names that other packages already consume must remain + importable from their current modules. In particular the orchestration barrel + `episodic.orchestration.__init__` and the worker barrel + `episodic.worker.__init__` must keep re-exporting every symbol they export + today (verify with `leta refs ` before moving a definition). +- No single code file may exceed 400 lines (AGENTS.md). + `episodic/orchestration/ langgraph.py` is already 460 lines; any split must + leave each resulting file under 400 lines. +- Domain purity: orchestration and worker code must not gain imports of + transport, storage, ORM, or vendor SDK modules. Cross-adapter imports remain + forbidden. +- All commentary and documentation use en-GB-oxendict spelling, except where + quoting external API names. +- Every milestone must pass `make check-fmt`, `make typecheck`, `make lint`, + and `make test` before its CodeRabbit review, and all CodeRabbit concerns + must be cleared before the next milestone begins. + +## Tolerances (exception triggers) + +Adjust per milestone; stop and escalate when a threshold is breached. + +- Scope: if a single milestone requires touching more than 25 files or more + than 600 net lines of production code (excluding tests and fixtures), stop + and escalate. +- Interface: if making a node or task ports-only would require changing a + public function or class signature that another package imports, stop and + escalate with the call sites (from `leta refs`). +- Dependencies: if any new runtime dependency is required, stop and escalate. + This slice should add no new runtime dependencies. +- Hecate expressivity: if a required rule cannot be expressed in Hecate without + more than two `[[tool.hecate.ignore_imports]]` entries, stop and escalate; + the coupling probably needs a refactor rather than an ignore. +- Iterations: if a milestone's gate suite still fails after 3 focused attempts, + stop and escalate with the failing transcript. +- Ambiguity: if the "ports only" interpretation in the Decision Log conflicts + with a reviewer's expectation, stop and reconcile before proceeding. + +## Risks + +- Risk: Hecate counts `TYPE_CHECKING`-guarded and function-body imports as + dependencies (confirmed from source: it walks the whole AST with `ast.walk` + and has no type-only exemption). A ports-only group will trip on a type-only + import of an application DTO. Severity: high. Likelihood: high. Mitigation: + design groups so type-only imports stay within the allowed set, or break the + coupling by extracting a provider-neutral DTO core. Use a single documented + `[[tool.hecate.ignore_imports]]` edge only as a last resort, paired with the + structural test as the binding guarantee. + +- Risk: Ungrouped modules inside the `episodic` root are invisible to Hecate + (imports of them are silently allowed, and they are not checked). A future + adapter placed in an ungrouped module would bypass enforcement. Severity: + medium. Likelihood: low. Mitigation: ensure every adapter-bearing module the + orchestration or worker code can reach is matched by a group prefix; add a + regression fixture proving an adapter import is caught. + +- Risk: Splitting `langgraph.py` or moving DTO definitions breaks an import that + another package relies on. Severity: medium. Likelihood: medium. Mitigation: + keep barrels (`__init__.py`) re-exporting all current names; verify every + moved symbol with `leta refs` before and after; rely on the full test suite + plus `make typecheck`. + +- Risk: First-match ordering mistakes silently misclassify a module (for + example a new specific prefix placed after a broader one never matches). + Severity: medium. Likelihood: medium. Mitigation: place specific prefixes + before broader ones, mirror the existing composition-root-before-adapter + convention, and add fixtures that would fail under a misordering. + +- Risk: The checkpoint DTO decoupling proves more invasive than expected because + `ActionExecutionResult` transitively references generation types. Severity: + medium. Likelihood: medium. Mitigation: if a clean ports-only checkpoint + group is not reachable within tolerance, scope the Hecate group to the + genuinely neutral payload modules, record a single governed `ignore_imports` + edge, and make the structural reflection test the primary guarantee. Escalate + if more than two ignores are needed. + +## Progress + +- [x] M0 Orientation and red harness (fixtures and failing tests, no + production changes). +- [x] M1 Dedicated `orchestration` Hecate group and node/builder split. +- [x] M2 Celery task enforcement and `WorkloadClass` extraction. +- [x] M3 Checkpoint payload boundary audit (Hecate group plus structural and + property tests). +- [x] M4 Behavioural tests, snapshots, documentation, and roadmap update. + +2026-06-26: Rebasing the branch onto `origin/main` completed cleanly with no +conflicts. Post-rebase gates passed: `make check-fmt`, `make test`, +`make typecheck`, and `make lint`. The branch was force-pushed with lease, the +PR title was updated to remove the `Plan:` prefix, and the PR references now +point at the active Lody session. + +2026-06-26: M0 added the fixture-only orchestration Hecate groups, synthetic +node/task/checkpoint fixtures, and the strict-xfailed production group +expectation. Focused architecture tests passed with `30 passed, 1 xfailed`. The +full milestone gates passed: `make check-fmt`, `make typecheck`, `make lint`, +and `make test` (`1020 passed, 3 skipped, 1 xfailed`). CodeRabbit review +completed with 0 findings. + +2026-06-26: M1 split `episodic/orchestration/langgraph.py` into +`_graph_nodes.py` for plan/execute/finish node functions, `_graph_builder.py` +for LangGraph assembly plus callback and cost wiring, and a 57-line +compatibility barrel that preserves the historical `langgraph` import path. The +production Hecate `orchestration` group is now ordered before `application` and +scoped to `_graph_builder`, `_graph_nodes`, and `langgraph`. Focused validation +passed with `68 passed, 1 xfailed`. The full milestone gates passed: +`make check-fmt`, `make typecheck`, `make lint`, and `make test` +(`1020 passed, 3 skipped, 1 xfailed`). CodeRabbit review completed with 0 +findings. + +2026-06-26: M2 extracted `WorkloadClass` to the provider-neutral +`episodic/worker/workloads.py` module, retargeted task/runtime imports to that +module, and kept `episodic.worker.WorkloadClass` plus +`episodic.worker.topology.WorkloadClass` importable. The production Hecate +config now classifies `episodic.worker.workloads` as `domain_ports` and +`episodic.worker.tasks` as `orchestration_tasks`, ordered before +`inbound_adapter`. Focused validation passed with `56 passed, 1 xfailed`. The +full milestone gates passed: `make check-fmt`, `make typecheck`, `make lint`, +and `make test` (`1020 passed, 3 skipped, 1 xfailed`). CodeRabbit review +completed with 0 findings. + +2026-06-26: M3 extracted provider-neutral payload DTOs and normalization +helpers into `episodic/orchestration/_payload_dto.py`, retargeted checkpoint +payload and checkpoint DTO modules away from the application-coupled `_dto` +barrel, and kept existing public orchestration imports working through +compatibility aliases. The production Hecate config now declares +`orchestration_checkpoint` before `orchestration`, and checkpoint modules may +only import their own group plus `domain_ports`. `WorkflowCheckpoint` now +rejects non-JSON payload values, and +`tests/test_checkpoint_payload_boundaries.py` adds a structural DTO field audit +plus a Hypothesis JSON round-trip property. Focused validation passed with +`88 passed`. The full milestone gates passed: `make check-fmt`, `make test` +(`1024 passed, 3 skipped`), `make typecheck`, and `make lint`. CodeRabbit +review completed with 0 findings. + +2026-06-26: M4 extended the architecture BDD feature with clean orchestration, +LangGraph-node, Celery-task, and checkpoint-payload scenarios; added a +normalized `hecate check --format json` snapshot covering representative +orchestration violations; and added a direct Vidai Mock-backed LangGraph +`plan -> execute -> finish` behavioural test. Focused validation passed with +`33 passed` before documentation updates. Documentation now records ADR-016, +the node/builder split, the `orchestration_nodes`, `orchestration_tasks`, and +`orchestration_checkpoint` groups, the checkpoint payload audit, and roadmap +item `2.4.5` as complete. The full milestone gates passed: `make check-fmt`, +`make test` (`1030 passed, 3 skipped`), `make typecheck`, `make lint`, +`make markdownlint`, and `make nixie`. CodeRabbit review completed with 0 +findings. + +## Surprises & discoveries + +- Observation: Hecate counts imports inside `if TYPE_CHECKING:` blocks and + inside function bodies identically to module-level imports. Evidence: source + review of the pinned Hecate commit; `collect_imports` uses `ast.walk` with no + guard inspection. Impact: ports-only groups must be reachable for type-only + imports too; drives the DTO-core decoupling in M3. + +- Observation: imports of ungrouped in-root modules are silently allowed and + ungrouped modules are not checked. Evidence: `_record_import_edge` returns + early when either side's group is `None`. Impact: `episodic.logging` and + similar cross-cutting modules need no group; but every adapter must stay + grouped or enforcement leaks. Add a guard fixture. + +- Observation: the current orchestration package imports no adapters; worker + tasks import only `WorkloadClass` from the `kombu`-coupled + `episodic.worker.topology`; checkpoint DTOs reach `episodic.generation` + (application tier) only through the `_dto` barrel (`_checkpoint_dto` to + `_dto` to `_action_result_dto` to `episodic.generation`). Evidence: import + map gathered with grep over `episodic/orchestration/*.py` and + `episodic/worker/*.py`. Impact: the enforcement is mostly preventative; the + two real fixes are the `WorkloadClass` extraction (M2) and the checkpoint DTO + decoupling (M3). + +- Observation: the fixture generator needs an explicit outbound `.adapter` + prefix as well as `.storage` so the `ungrouped_adapter_is_caught` fixture + fails if a reachable adapter-like module is left invisible to Hecate. + Evidence: helper-level tests now assert the outbound prefixes include both + modules. Impact: future fixture additions can model non-storage adapters + without adding fixture-specific TOML. + +- Observation: a broad production prefix of `episodic.orchestration` catches + the durable checkpoint adapter before M3 has separated checkpoint DTOs. + Evidence: the first M1 focused run failed `run_hecate_production_check()` + because `episodic.canonical.storage.workflow_checkpoints` imports + `WorkflowCheckpoint`, and because two adapters still imported the + orchestration-local `_log_event` helper. Impact: M1 moved the reusable + structured logging helper to `episodic.logging.log_event` and scoped the + production `orchestration` group to graph modules only. M3 remains + responsible for the checkpoint DTO group and the durable checkpoint adapter + edge. + +- Observation: `WorkloadClass` had only two internal production consumers that + needed retargeting away from `topology`: `worker.tasks` and `worker.runtime`. + Evidence: `leta refs WorkloadClass` after the move shows internal imports from + `episodic.worker.workloads`, while `topology` and the public worker barrel + re-export the same symbol for existing callers. Impact: M2 could preserve the + public worker API while giving the task module a vendor-free import path. + +- Observation: the durable SQLAlchemy checkpoint store is an outbound adapter + that legitimately implements `CheckpointPort` using `WorkflowCheckpoint`. + Evidence: `episodic.canonical.storage.workflow_checkpoints` maps SQLAlchemy + rows to `WorkflowCheckpoint` and accepts `WorkflowCheckpoint` in + `save_or_reuse`. Impact: the production `outbound_adapter` group must be + allowed to import `orchestration_checkpoint`; the checkpoint DTO group + remains strict because its own `allowed` list excludes both application and + adapter groups. + +- Observation: `ActionExecutionResult` carries rich show-notes and guest-bios + attachments for in-process orchestration results, but checkpoint payload + serialization deliberately ignores those attachment fields. Evidence: + `_action_result_to_payload` stores only action identity, kind, model tier, + model, summary, and usage, while tests still assert rich attachment + attributes on direct tool results. Impact: `_payload_dto.py` now uses local + structural Protocols for attachment shapes instead of importing generation + DTOs. The structural checkpoint audit skips these non-persisted attachment + fields and separately enforces `WorkflowCheckpoint.payload` JSON + serialisability. + +## Decision log + +- Decision: interpret "depend on ports only" as "depend on the application and + domain-ports layers only; never import adapters (inbound or outbound), + storage, ORM, or vendor SDKs", and apply the strictest reading (ports and + provider-neutral DTOs only) to checkpoint payloads. Rationale: the roadmap + wording "ports only" is reconciled with the system design, which states + orchestration "will depend on domain services and ports only". Domain + services live in the application layer and are not adapters. Checkpoint + payloads carry a stricter rule because the design says they must hold + orchestration metadata only, with canonical state persisted through + repositories. Date/Author: 2026-06-15, planning agent. + +- Decision: pursue genuine node-level ports-only enforcement by splitting + `episodic/orchestration/langgraph.py` into a ports-only node module and an + application-tier graph-builder module, rather than only renaming the group. + Rationale: it is the most faithful reading of "validate LangGraph nodes + depend on ports only", and it also resolves the existing 400-line file + violation. AGENTS.md requires actioning requested changes rather than + treating them as optional. Date/Author: 2026-06-15, planning agent. + +- Decision: enforce checkpoint payload boundaries with both a Hecate group and + a structural reflection test (plus a property test). Rationale: Hecate's + layer model cannot forbid embedding canonical domain entities, because those + classify as `domain_ports`. A reflection-based test over payload DTO field + types is required to fully cover the design rule. Date/Author: 2026-06-15, + planning agent. + +- Decision: the clarifying question on strictness and audit mechanism was + offered to the user but not answered; the plan adopts the more thorough + options and records them here so reviewers can scope down during PR review. + Rationale: a plan is cheaper to narrow than to re-expand, and PR review is + the approval gate. Date/Author: 2026-06-15, planning agent. + +- Decision: model production-like specific prefixes in architecture fixtures + (`orchestration._graph_nodes`, `orchestration._checkpoint_payload`, and + `worker.tasks`) instead of flat toy module names. Rationale: this makes M0 + cover the first-match ordering hazard that M1-M3 must preserve in the real + `[tool.hecate]` configuration. Date/Author: 2026-06-26, implementation agent. + +- Decision: scope the first production `orchestration` Hecate group to graph + modules (`_graph_builder`, `_graph_nodes`, and the compatibility `langgraph` + barrel) rather than the whole `episodic.orchestration` package. Rationale: M1 + proves the node/builder split and prevents graph modules from importing + adapters without prematurely grouping checkpoint DTOs. A package- wide prefix + would force the M3 checkpoint DTO decision into M1 and would make the durable + checkpoint adapter fail before its port DTO boundary has been audited. + Date/Author: 2026-06-26, implementation agent. + +- Decision: promote the structured event helper from + `episodic.orchestration._types._log_event` to `episodic.logging.log_event`, + while keeping `_types._log_event` as a compatibility alias. Rationale: + canonical adapters were using the helper for generic structured logging. + Keeping that helper in orchestration created an adapter-to- orchestration + dependency unrelated to graph policy; the logging module is the existing + neutral home for logging helpers. Date/Author: 2026-06-26, implementation + agent. + +- Decision: classify `episodic.worker.workloads` as `domain_ports`, not + `application`. Rationale: `WorkloadClass` is a provider-neutral routing + contract enum shared by task code and the Kombu-backed topology adapter. + Treating it as `domain_ports` lets both task and topology layers depend on it + without creating a task-to-topology edge or broadening task permissions. + Date/Author: 2026-06-26, implementation agent. + +- Decision: classify checkpoint DTO and payload modules in a dedicated + `orchestration_checkpoint` Hecate group, and allow outbound adapters to + import that group. Rationale: checkpoint DTOs are provider-neutral port + contracts. Outbound checkpoint stores need those contracts to implement + `CheckpointPort`, but the DTO modules themselves must not import application + services, storage, ORM models, or vendor SDKs. Date/Author: 2026-06-26, + implementation agent. + +- Decision: keep rich tool-result attachments on `ActionExecutionResult` as + provider-neutral structural Protocols rather than importing concrete + generation result DTOs. Rationale: existing callers rely on + `show_notes_result` and `guest_bios_result` attributes for direct + orchestration results, but checkpoint payload serialization does not persist + those attachments. Local Protocols preserve static type usefulness without + reintroducing a checkpoint-to-application import edge. Date/Author: + 2026-06-26, implementation agent. + +- Decision: no `docs/users-guide.md` update is required for M4. + Rationale: the slice changes maintainer-facing architecture enforcement, + tests, and documentation only; no public user workflow, command, or API + behaviour changed. Date/Author: 2026-06-26, implementation agent. + +## Outcomes & retrospective + +M1 outcome: the LangGraph node functions now live in +`episodic/orchestration/_graph_nodes.py`, graph assembly lives in +`episodic/orchestration/_graph_builder.py`, and the historical +`episodic.orchestration.langgraph` import path re-exports the moved symbols. +The production architecture gate now groups those graph modules separately from +`application`, while leaving checkpoint DTO grouping for M3. + +M2 outcome: Celery task code imports `WorkloadClass` from a provider-neutral +worker workload module, while topology remains responsible for Kombu queue +objects. The production architecture gate now groups `episodic.worker.tasks` as +`orchestration_tasks`, so task code may import application and domain port +contracts but not inbound or outbound adapters. + +M3 outcome: checkpoint payload modules now belong to +`orchestration_checkpoint`, and `make lint` fails if they import application or +adapter modules. A structural test audits checkpoint payload DTO field types, +and a property test verifies JSON-shaped `WorkflowCheckpoint.payload` values +round-trip unchanged through JSON serialization. + +## Context and orientation + +This section assumes no prior knowledge of the repository. + +### Architecture enforcement today + +The checker is Hecate, invoked by the `check-architecture` target in the +`Makefile`: + +```make +check-architecture: build ## Check hexagonal architecture import boundaries + $(UV_ENV) $(UV) run hecate check +``` + +`make lint` runs `check-architecture` first, then Ruff and Pylint. Hecate reads +`[tool.hecate]` from `pyproject.toml`. The configuration declares one root +package (`episodic`), a default rule identifier (`ARCH001`), and five ordered +groups. Each group has a `name`, a list of module `prefixes`, and an `allowed` +list naming the groups it may import from. Matching is first-match by config +order, so specific prefixes must precede broader ones. A group must list its +own name in `allowed` to permit imports between its own modules. + +The current groups (see `pyproject.toml`, the `[tool.hecate]` block) are: + +1. `composition_root` (`episodic.api.runtime`, `episodic.worker.runtime`) may + import every layer. +2. `domain_ports` (canonical domain and port protocols, `episodic.cost.ports`, + `episodic.cost.engine`, `episodic.llm.ports`, `episodic.metrics_ports`, and + related) may import only `domain_ports`. +3. `application` (canonical services, `episodic.generation`, + `episodic.orchestration`, `episodic.cost.recorder`, and related) may import + `application` and `domain_ports`. +4. `inbound_adapter` (`episodic.api`, `episodic.worker.tasks`, + `episodic.worker.topology`) may import `inbound_adapter`, `application`, + `domain_ports`. +5. `outbound_adapter` (storage, canonical adapters, OpenAI adapters, cost + storage, pricing catalogue) may import `outbound_adapter`, `application`, + `domain_ports`. + +### How the architecture tests are wired + +`tests/architecture_hecate_config.py` is a helper module (not a test) that +generates per-fixture Hecate TOML and invokes the Hecate CLI by subprocess. It +exposes `write_fixture_config(tmp_path, package_name)`, +`run_hecate_fixture_check(package_name, config_path)`, and +`run_hecate_production_check()`. Fixture packages live under +`tests/fixtures/architecture//` and are tiny synthetic packages +(`domain.py`, `service.py`, `api.py`, `storage.py`, `runtime.py`, and +`__init__.py`) that model one boundary scenario each. Existing fixtures cover +the allowed case, composition-root wiring, and several violation cases +including re-exported and star-re-exported barrels. + +`tests/test_architecture_hecate_config.py` tests the helper itself (TOML shape +and subprocess error wrapping). The fixtures are exercised by the architecture +test and behaviour-driven development (BDD) steps that assert exit codes and +emitted violations. Use `leta grep` and `leta refs` to locate the exact test +entry points before editing; do not assume file names. + +### The code under enforcement + +LangGraph is a library for building stateful graphs of "nodes" (functions that +take a state and return a state update). The orchestration package builds one +such graph for generation: + +- `episodic/orchestration/langgraph.py` (460 lines) defines the node functions + `_plan_node`, `_execute_node`, `_finish_node`, cost-recording helpers, and + the graph builder `build_generation_orchestration_graph`. It imports the + LangGraph library plus sibling orchestration modules. The node functions + receive their collaborators (`PlannerPort`, `ToolExecutorPort`) by injection; + the builder is the module's only consumer of `_planning_orchestrator` + (`StructuredPlanningOrchestrator`, application tier). +- `episodic/orchestration/_protocols.py` defines the port protocols + (`PlannerPort`, `ToolExecutorPort`, `CostRecorderPort`, and similar). +- `episodic/orchestration/_graph_state.py`, `_types.py`, `_usage.py`, + `_dto.py`, `_result_dto.py`, `_action_result_dto.py`, `_checkpoint_dto.py`, + `_checkpoint_payload.py`, `_checkpoint_resume.py`, `checkpoints.py`, + `generation.py`, and the executors round out the package. + +Celery is a distributed task queue. The worker package: + +- `episodic/worker/tasks.py` defines representative tasks via injected callable + seams (`WorkerDependencies`); its only cross-module import is `WorkloadClass` + from `episodic/worker/topology.py`. +- `episodic/worker/topology.py` imports `kombu` (a vendor SDK) and defines both + `WorkloadClass` (a pure `enum.StrEnum`) and the `kombu`-coupled queue specs. +- `episodic/worker/runtime.py` is the composition root that wires Celery. + +Checkpoint payloads are the durable orchestration state saved when a graph +pauses. `episodic/orchestration/_checkpoint_dto.py` defines +`WorkflowCheckpoint` (its `payload` is a `dict[str, object]`), +`SuspendedWorkflowResult`, `ResumeWorkflowCommand`, and `WorkflowStepIdentity`. +It imports `ActionExecutionResult` and two normalization helpers from the +`_dto` barrel; the barrel transitively imports `episodic.generation` +(application tier) through `_action_result_dto`. + +### Hecate facts that constrain the design (verified from source) + +- `TYPE_CHECKING`-guarded and function-body imports are counted as + dependencies. +- Imports of ungrouped in-root modules are allowed; ungrouped modules are not + checked. +- A group must list its own name in `allowed` to permit intra-group imports. +- Matching is first-match by config order; prefix containment is at dotted + boundaries. +- Re-exports, star re-exports, and relative imports resolve to the origin + module's group. +- `[[tool.hecate.ignore_imports]]` accepts `importer`, `imported`, and a + required non-empty `reason`; `hecate check` supports `--show-ignored` and + `--fail-on-unmatched-ignore`. +- CLI: `hecate check` accepts `--config`, `--package` plus `--root` (together), + `--include-external-packages`, and `--format {text,json}`. Exit codes: 0 + pass, 1 violations, 2 config or validation error. + +### Skills and documentation to consult + +Load and follow these skills while implementing: + +- `hexagonal-architecture` for layer boundaries and drift detection. +- `python-router`, then `python-data-shapes` (DTO design), + `python-types-and-apis` (Protocols and signatures), and `python-testing` + (fixtures, parametrization, snapshots). +- `python-verification`, then `hypothesis` for the checkpoint property test + (and `crosshair` only if a PEP 316 contract is added). +- `vidai-mock` for behavioural tests that exercise the generation graph against + a simulated inference service. +- `leta` for navigation and safe refactors; `commit-message` for commits; + `pr-creation` for the pull request. + +Read these documents (signposts): + +- `docs/episodic-podcast-generation-system-design.md` — "Hexagonal + architecture enforcement", "Orchestration guardrails", "Orchestration ports + and adapters", and "Checkpoint payload boundaries". This is where design + decisions are recorded. +- `docs/agentic-systems-with-langgraph-and-celery.md` and + `docs/langgraph-and-celery-in-hexagonal-architecture.md` — orchestration + component architecture; the latter is the home for internal-interface notes. +- `docs/developers-guide.md` — the "Architecture enforcement" section; update it + with the new groups and conventions. +- `docs/execplans/adopt-hecate.md` — how the groups and fixtures were + established, including the first-match ordering gotcha and the star-import + barrel surprise. +- `docs/adr/adr-014-hexagonal-architecture-enforcement.md` — the enforcement + ADR to extend or cross-reference. +- `docs/async-sqlalchemy-with-pg-and-falcon.md`, + `docs/testing-async-falcon-endpoints.md`, and + `docs/testing-sqlalchemy-with-pytest-and-py-pglite.md` — testing patterns for + any persistence-touching behavioural test. +- `docs/documentation-style-guide.md`, `docs/contents.md`, and + `docs/repository-layout.md` — documentation conventions and indices. + +## Plan of work + +The work proceeded through five milestones. Each followed Red-Green-Refactor: +add the smallest failing fixture or test first, confirm it fails for the +intended reason, make the minimal production or configuration change, then +refactor before re-running the gates. Architecture rules are validated through +the fixture harness (synthetic packages) for both positive and negative cases, +and through the production `hecate check` for the real code. + +### M0 Orientation and red harness (no production changes) + +Goal: establish failing tests that specify the new boundaries before any +production or configuration change, and confirm the current baseline. + +1. Confirm the baseline gates pass: run `make build`, then + `make check-architecture`, `make typecheck`, and the architecture tests. + Record the transcripts as evidence. +2. Add new synthetic fixture packages under `tests/fixtures/architecture/` that + model the orchestration boundaries (see "Fixtures to add" below). Add them + first as the Red stage; the new architecture test cases that assert their + expected exit codes and violations must be added and observed failing or + xfailing before the corresponding production or config change. +3. Extend the fixture-config generator in + `tests/architecture_hecate_config.py` so it can emit the new orchestration, + orchestration-tasks, and checkpoint groups for the synthetic packages. Keep + the generator data-driven; do not hard-code per-fixture TOML beyond what + already exists. +4. Add a production-level red expectation: a test asserting that the production + Hecate config declares the new groups (`orchestration`, + `orchestration_tasks`, `orchestration_checkpoint`). Mark it + `@pytest.mark.xfail(strict=True, reason="groups added in M1-M3")` and + confirm it xfails; remove the marker as the groups land. + +Validation: the new fixture tests fail or xfail for the expected reason; +existing gates still pass. No production code or `pyproject.toml` group change +yet. + +### M1 Dedicated `orchestration` group and node/builder split + +Goal: make a named, fixtured `orchestration` group and prove LangGraph node +code cannot import adapters, with node bodies isolated from application-tier +graph assembly. + +1. Refactor `episodic/orchestration/langgraph.py` into two modules: + - `episodic/orchestration/_graph_nodes.py`: the node functions and their + direct helpers, importing only port protocols (`_protocols`), graph state + (`_graph_state`), provider-neutral DTOs, `episodic.llm` (ports), + `episodic.cost.ports`, and `episodic.logging`. No import of + `_planning_orchestrator`, `episodic.generation`, or `episodic.cost.recorder`. + - `episodic/orchestration/_graph_builder.py` (or keep `langgraph.py` as the + builder): `build_generation_orchestration_graph` and any + application-tier wiring, importing the nodes plus the planning + orchestrator. + Keep both files under 400 lines. Preserve every name currently re-exported by + `episodic/orchestration/__init__.py`; verify with `leta refs` for each + moved symbol. +2. Add the `orchestration` group to `[tool.hecate]` in `pyproject.toml`, + placed before the `application` group (first-match). Remove + `episodic.orchestration` from the `application` group's prefixes and add it + under the new group. Decide the node-strictness expression: + - The node module prefix (`episodic.orchestration._graph_nodes`) gets the + strictest `allowed` (its own group plus `domain_ports` plus the checkpoint + group added in M3), proving nodes are ports-and-DTO only. + - The rest of `episodic.orchestration` keeps `allowed = ["orchestration", + "application", "domain_ports"]` (domain services permitted, adapters + forbidden). + If a single group cannot express both, introduce a separate + `orchestration_nodes` group (prefix `episodic.orchestration._graph_nodes`) + ordered before `orchestration`. +3. Add fixtures and tests proving: a node-tier module importing an outbound + adapter fails; a node-tier module importing a port passes; an orchestration + (non-node) module importing a domain service passes; any orchestration + module importing an inbound or outbound adapter fails. + +Validation: run `make check-fmt`, `make typecheck`, `make lint` (includes +`hecate check`), and `make test`. The new node-violation fixture fails the +fixture check; the production check passes; the previously xfailing "declares +orchestration group" expectation now passes (remove its marker). + +### M2 Celery task enforcement and `WorkloadClass` extraction + +Goal: make Celery task code ports-only by removing its dependence on the +`kombu`-coupled topology module, and enforce it with a dedicated group. + +1. Extract `WorkloadClass` from `episodic/worker/topology.py` into a new + provider-neutral module, for example `episodic/worker/workloads.py`, that + imports no vendor SDK. Re-export `WorkloadClass` from + `episodic/worker/topology.py` and `episodic/worker/__init__.py` so existing + importers keep working (verify with `leta refs WorkloadClass`). +2. Update `episodic/worker/tasks.py` to import `WorkloadClass` from + `episodic.worker.workloads`. +3. Classify `episodic.worker.workloads` as `domain_ports` (it is a + provider-neutral contract type that both tasks and topology must import; + both layers already allow `domain_ports`). Add its prefix to the + `domain_ports` group. +4. Add an `orchestration_tasks` group with prefix `episodic.worker.tasks`, + ordered before the `inbound_adapter` group, with + `allowed = ["orchestration_tasks", "application", "domain_ports"]` (no + inbound or outbound adapter). Remove `episodic.worker.tasks` from the + `inbound_adapter` group's prefixes. +5. Add fixtures and tests proving: a task-tier module importing an inbound + adapter fails; a task-tier module importing an outbound adapter fails; a + task-tier module importing a port and a domain service passes. + +Validation: the gate suite passes; the new task-violation fixtures fail the +fixture check; the production check passes. Confirm with `leta refs` that no +caller broke from the `WorkloadClass` move. + +### M3 Checkpoint payload boundary audit + +Goal: enforce that checkpoint payload DTOs hold orchestration metadata and +provider-neutral DTOs only, with no application coupling and no embedded +canonical or ORM state. + +1. Decouple the checkpoint DTOs from the application-coupled `_dto` barrel. + Inspect what `_checkpoint_dto.py` and `_checkpoint_payload.py` actually need + (`ActionExecutionResult` and the `_normalize_*` helpers) and whether those + transitively reference generation types (use `leta show` and + `leta calls --from`). Extract a provider-neutral DTO core module, for example + `episodic/orchestration/_payload_dto.py`, that defines or holds the neutral + DTOs and normalization helpers and imports only `episodic.llm` (ports) and + `_types`. Re-point `_checkpoint_dto.py`, `_checkpoint_payload.py`, and + `_result_dto.py` at the core. Keep the `_dto` barrel re-exporting every + current name for backward compatibility. +2. Add an `orchestration_checkpoint` group with prefixes for the payload + modules (`episodic.orchestration._checkpoint_payload`, + `episodic.orchestration._checkpoint_dto`, and the new `_payload_dto`), + ordered before the `orchestration` group, with + `allowed = ["orchestration_checkpoint", "domain_ports"]` (ports and neutral + DTOs only). If, after step 1, exactly one unavoidable type-only edge to an + application DTO remains, record a single `[[tool.hecate.ignore_imports]]` + entry with a clear reason; more than one such edge is a tolerance breach to + escalate. +3. Add a structural reflection test (the audit) over the checkpoint payload + DTOs that asserts each field's type is within an allow-list of + provider-neutral types (primitives, `enum` members, `datetime`, mappings of + primitives, `LLMUsage`, and other checkpoint DTOs) and explicitly rejects + SQLAlchemy ORM models and canonical aggregate entity types. This covers the + design rule that Hecate's layer model cannot express, because canonical + entities classify as `domain_ports`. +4. Add a Hypothesis property test asserting an invariant over the checkpoint + payload, for example: for any generated `WorkflowCheckpoint`, its `payload` + round-trips through JSON serialization unchanged (proving payloads stay + JSON-shaped and free of non-serializable adapter or ORM objects). Follow the + `hypothesis` skill; keep the strategy bounded and the regression database + committed per project convention. + +Validation: the gate suite passes; the structural and property tests fail +before the decoupling and pass after; the checkpoint-violation fixture (a +payload module importing storage) fails the fixture check; the production check +passes. + +### M4 Behavioural tests, snapshots, documentation, and roadmap update (completed) + +M4 delivered the behavioural scenarios, the normalized Hecate snapshot, the +Vidai Mock-backed graph test, the architecture documentation and ADR-016, and +the roadmap update. The dated entry above records the focused and final gate +results, including `make check-fmt`, `make typecheck`, `make lint`, `make test`, +`make markdownlint`, and `make nixie`. + +## Fixtures to add + +Add synthetic packages under `tests/fixtures/architecture/`, each a minimal +package with an `__init__.py`. Model the orchestration layer with modules named +to match the generated group prefixes the helper emits. Suggested fixtures +(adjust names to the generator's conventions): + +1. `orchestration_node_imports_outbound_adapter` — a node-tier module imports a + storage module; expected violation. +2. `orchestration_node_imports_port` — a node-tier module imports a port; + expected pass. +3. `orchestration_imports_domain_service` — a non-node orchestration module + imports an application service; expected pass. +4. `orchestration_imports_inbound_adapter` — an orchestration module imports an + inbound adapter; expected violation. +5. `celery_task_imports_inbound_adapter` — a task-tier module imports an inbound + adapter; expected violation. +6. `celery_task_imports_outbound_adapter` — a task-tier module imports an + outbound adapter; expected violation. +7. `checkpoint_payload_imports_storage` — a checkpoint payload module imports a + storage module; expected violation. +8. `checkpoint_payload_imports_application` — a checkpoint payload module + imports an application service; expected violation. +9. `ungrouped_adapter_is_caught` — a guard fixture proving that an adapter the + orchestration layer can reach is grouped (not invisible), so enforcement + does not leak through an ungrouped module. + +Each fixture gets a positive or negative test case parametrized in the +architecture test, mirroring the existing fixture tests. Extend the helper's +fixture-config generator to emit the orchestration, orchestration-tasks, and +checkpoint groups for these synthetic packages. + +## BDD feature + +Place under the project's feature directory (confirm the path and step-module +convention first). This specification records the completed M4 scenarios. + +```gherkin +Feature: Architecture enforcement + + Scenario: A clean orchestration fixture passes + Given the architecture fixture package "orchestration_node_imports_port" + When the architecture checker runs + Then the architecture check passes + + Scenario: A LangGraph node importing an adapter is rejected + Given the architecture fixture package "orchestration_node_imports_outbound_adapter" + When the architecture checker runs + Then the check fails with an ARCH001 violation + And the architecture diagnostic mentions "orchestration._graph_nodes" + + Scenario: A Celery task importing an adapter is rejected + Given the architecture fixture package "celery_task_imports_inbound_adapter" + When the architecture checker runs + Then the check fails with an ARCH001 violation + And the architecture diagnostic mentions "worker.tasks" + + Scenario: A checkpoint payload importing storage is rejected + Given the architecture fixture package "checkpoint_payload_imports_storage" + When the architecture checker runs + Then the check fails with an ARCH001 violation + And the architecture diagnostic mentions "orchestration._checkpoint_payload" +``` + +## Concrete steps + +Run all commands from the repository root +(`/home/leynos/.lody/repos/github---leynos---episodic/worktrees/...` in this +worktree). Capture long output with `tee` to a temporary log for review, per +project convention. + +1. Build and baseline: + + ```bash + make build + make check-architecture | tee /tmp/check-arch-baseline.out + make test | tee /tmp/test-baseline.out + ``` + + Expect `check-architecture` to exit 0 and the test suite to pass. + +2. Per milestone, add the Red fixture or test, run the focused test and confirm + the expected failure, implement the minimal change, then run the gates in + this order (sequential, to benefit from build caching; do not run them in + parallel): + + ```bash + make check-fmt | tee /tmp/check-fmt-$(git branch --show-current).out + make typecheck | tee /tmp/typecheck-$(git branch --show-current).out + make lint | tee /tmp/lint-$(git branch --show-current).out + make test | tee /tmp/test-$(git branch --show-current).out + ``` + +3. For documentation milestones additionally run: + + ```bash + make markdownlint | tee /tmp/mdlint-$(git branch --show-current).out + make nixie + ``` + + Run `make nixie` unsandboxed (it needs a browser sandbox of its own). Guard + any long inline code spans in Markdown to avoid MD013 from the formatter. + +4. After each milestone's gates pass, run the CodeRabbit review and clear every + concern before the next milestone: + + ```bash + coderabbit review --agent | tee /tmp/coderabbit-$(git branch --show-current).out + ``` + + CodeRabbit must not be used to catch issues the deterministic gates can + catch; run the gates first. + +5. Commit frequently with the `commit-message` skill (file-based messages, never + `-m`). Keep functional changes and pure refactors in separate atomic commits. + +## Validation and acceptance + +Acceptance is behavioural and observable: + +- Running `make lint` on the production tree passes (`hecate check` exits 0) + at every milestone boundary. +- A new architecture fixture in which a LangGraph node imports an outbound + adapter causes the fixture's Hecate check to exit 1 with an `ARCH001` + violation; the same node importing a port passes. This new test fails before + M1's group is added and passes after. +- A new fixture in which a Celery task imports an adapter causes the fixture + check to exit 1; a task importing a port and a domain service passes. Fails + before M2, passes after. +- A new fixture in which a checkpoint payload module imports storage causes the + fixture check to exit 1. The structural reflection test fails if a checkpoint + payload DTO declares a field whose type is an ORM model or canonical entity, + and passes for the current provider-neutral fields. Fails before M3, passes + after. +- The Hypothesis property test shows every generated `WorkflowCheckpoint` + payload round-trips through JSON unchanged. +- The `vidai-mock`-backed behavioural test shows the generation graph still + plans, executes, and finishes after the node/builder split. +- The BDD scenarios above pass. + +Quality criteria (what "done" means): + +- Tests: `make test` passes with the new unit, BDD, snapshot, structural, and + property tests included. +- Lint and typecheck: `make lint` and `make typecheck` pass; `hecate check` + exits 0. +- Markdown: `make markdownlint` and `make nixie` pass for all edited docs. +- Architecture: every new rule is proven by both a positive and a negative + fixture; no rule relies on an ungrouped module to pass. +- Review: CodeRabbit concerns cleared at each milestone. + +Quality method (how we check): the `Makefile` gate suite run sequentially after +each milestone, plus the fixture-harness positive and negative cases, plus +CodeRabbit. + +## Idempotence and recovery + +- Configuration and test additions are re-runnable; re-running the gates is + safe. +- Symbol moves are the only risky steps. Before moving any definition, record + its current import sites with `leta refs `; after moving, keep the + original module re-exporting the name and re-run `make typecheck` and + `make test`. If a downstream import breaks, restore the re-export rather than + editing the consumer, unless the consumer is within scope. +- If a Hecate group change makes the production check fail for an unforeseen + edge, revert the `pyproject.toml` group change, capture the violation, and + decide in the Decision Log whether to refactor the edge or adjust the group + ordering; do not add an `ignore_imports` to silence a genuine boundary + violation. + +## Interfaces and dependencies + +No new runtime dependencies. The following names must exist at the end of the +slice (paths are illustrative where the plan allows a choice; fix the names in +the Decision Log when chosen): + +- `episodic/orchestration/_graph_nodes.py` containing the node functions + (`_plan_node`, `_execute_node`, `_finish_node`, and helpers), importing ports + and provider-neutral DTOs only. +- `episodic/orchestration/_graph_builder.py` (or retained `langgraph.py`) + containing `build_generation_orchestration_graph`. +- `episodic/orchestration/_payload_dto.py` containing the provider-neutral + checkpoint DTO core and normalization helpers. +- `episodic/worker/workloads.py` containing `WorkloadClass`, re-exported from + `episodic/worker/topology.py` and `episodic/worker/__init__.py`. +- `[tool.hecate]` groups `orchestration` (and optionally `orchestration_nodes`), + `orchestration_tasks`, and `orchestration_checkpoint`, ordered before their + broader counterparts, with `domain_ports` extended to include + `episodic.worker.workloads`. +- Architecture fixtures and tests as listed under "Fixtures to add", a + structural reflection test, a Hypothesis property test, a `syrupy` snapshot, a + `pytest-bdd` feature, and a `vidai-mock`-backed behavioural test. +- `docs/adr/adr-016-*.md` recording the decisions, cross-referenced from + ADR-014 and the system design. + +The orchestration and worker public barrels +(`episodic/orchestration/__init__.py`, `episodic/worker/__init__.py`) must +export exactly the same names they export today. + +## Revision note + +2026-08-03: Set the plan status to `COMPLETE` and converted the remaining M4 +implementation instructions into a completed outcome. Preserved the completed +milestones and final-gate records; no implementation work remains. diff --git a/docs/langgraph-and-celery-in-hexagonal-architecture.md b/docs/langgraph-and-celery-in-hexagonal-architecture.md index 0a5f1469..25c0a68e 100644 --- a/docs/langgraph-and-celery-in-hexagonal-architecture.md +++ b/docs/langgraph-and-celery-in-hexagonal-architecture.md @@ -36,6 +36,42 @@ mixing infrastructure calls into what should be a domain workflow. Therefore, logic should remain in the application layer and invoke only domain services or ports, never directly calling outbound adapters in-line. +## Enforced Orchestration Boundaries + +Roadmap item `2.4.5` makes those orchestration rules executable through Hecate. +The production configuration gives LangGraph nodes a dedicated group before the +broad `orchestration` group and broader adapter prefixes, so the first matching +group is the strictest useful boundary. + +- `orchestration_nodes` covers `episodic.orchestration._graph_nodes`. Node + functions may import the `orchestration_checkpoint` DTO group and + `domain_ports`, but not Falcon, Celery, SQLAlchemy, OpenAI adapters, or other + concrete infrastructure. +- `orchestration` covers graph builders, planning orchestration, and tool + execution policy. This layer may depend on application services, domain + ports, and `orchestration_nodes`, but still cannot import inbound or outbound + adapters. +- `orchestration_tasks` covers `episodic.worker.tasks`. Tasks may import + `episodic.worker.workloads.WorkloadClass`, domain services, and ports; the + worker runtime remains the composition root that wires Celery. +- `orchestration_checkpoint` covers checkpoint DTO and payload serialization + modules. These modules may import domain-port value types and their own DTO + group only. `WorkflowCheckpoint` also rejects non-JSON payload values at + construction time. + +The node/builder split keeps framework mechanics out of node functions without +making LangGraph itself a domain dependency. `_graph_nodes.py` holds the +side-effect-free node bodies that receive injected planner, executor, +checkpoint, resume, and cost collaborators. `_graph_builder.py` owns LangGraph +assembly, callback wrapping, and cost-recording edges. The public +`langgraph.py` module stays as the stable facade. + +Checkpoint payload auditing is both static and runtime checked. Hecate prevents +payload modules from importing storage or application services. A structural +test walks the persisted checkpoint DTO annotations to reject provider-specific +or ORM-shaped fields, whilst the `WorkflowCheckpoint` constructor verifies that +payload mappings can be serialized as JSON. + **Graph-Based Logic vs. Domain Rules Clarity:** Another friction point is how business rules are encoded. In a hexagonal design, business rules belong in the domain layer or application logic, not scattered across infrastructure. diff --git a/docs/roadmap.md b/docs/roadmap.md index eaae2866..45d80775 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -268,10 +268,14 @@ metering. Completion enables reliable, auditable generation workflows. - Aggregate run totals with hierarchical ledger entries. - See [Cost accounting and budget enforcement](episodic-podcast-generation-system-design.md#cost-accounting-and-budget-enforcement). -- [ ] 2.4.5. Extend architecture enforcement to orchestration code. +- [x] 2.4.5. Extend architecture enforcement to orchestration code. - Validate LangGraph nodes depend on ports only. - Validate Celery tasks depend on ports only. - Audit checkpoint payload boundaries. + - Added Hecate groups for LangGraph nodes, orchestration task modules, and + checkpoint payload DTOs. + - Added BDD, snapshot, structural, property, and Vidai Mock-backed graph + coverage for the enforced boundaries. ### 2.5. Pricing catalogue and budget enforcement diff --git a/episodic/canonical/adapters/generation_runs.py b/episodic/canonical/adapters/generation_runs.py index 77255bd2..e66468d1 100644 --- a/episodic/canonical/adapters/generation_runs.py +++ b/episodic/canonical/adapters/generation_runs.py @@ -23,7 +23,7 @@ GenerationRunStatusUpdate, event_seq, ) -from episodic.orchestration._types import _log_event +from episodic.logging import log_event as _log_event from .generation_checkpoints import InMemoryGenerationCheckpointMixin diff --git a/episodic/logging.py b/episodic/logging.py index eb2055c7..7c1652c7 100644 --- a/episodic/logging.py +++ b/episodic/logging.py @@ -12,9 +12,12 @@ >>> logger.info("Started ingestion") """ +import datetime as dt import enum +import json import logging import typing as typ +import uuid import warnings from femtologging import basicConfig, get_logger, getLogger @@ -253,12 +256,60 @@ def log_error( ) +_event_log = getLogger(__name__) + + +def _serialize_log_field(value: object) -> object: + """Return a stable JSON-compatible representation for a log field.""" + match value: + case enum.Enum(): + return value.value + case dt.date() | dt.time(): + return value.isoformat() + case uuid.UUID() | BaseException(): + return str(value) + case _: + return str(value) + + +def log_event(level: str, message: str, **fields: object) -> None: + """Emit one structured log event with a JSON fallback. + + Logger convenience methods only accept ``exc_info`` and ``stack_info`` + beside the message. Structured fields are serialized into one JSON message + when needed. + + Examples + -------- + >>> log_event("info", "generation.started", workflow_id="workflow-42") + # Logger message: {"event": "generation.started", "workflow_id": "workflow-42"} + """ + log_method = getattr(_event_log, level) + allowed_kwargs = { + k: v for k, v in fields.items() if k in {"exc_info", "stack_info"} + } + extra_fields = {k: v for k, v in fields.items() if k not in allowed_kwargs} + if extra_fields: + payload = {"event": message, **extra_fields} + log_method( + json.dumps(payload, default=_serialize_log_field, sort_keys=True), + **allowed_kwargs, + ) + return + try: + log_method(message, **allowed_kwargs) + except TypeError: + payload = {"event": message} + log_method(json.dumps(payload, sort_keys=True), **allowed_kwargs) + + __all__ = ( "LogLevel", "configure_logging", "getLogger", "get_logger", "log_error", + "log_event", "log_info", "log_warning", ) diff --git a/episodic/orchestration/_action_result_dto.py b/episodic/orchestration/_action_result_dto.py index a9aff0af..3ffbb837 100644 --- a/episodic/orchestration/_action_result_dto.py +++ b/episodic/orchestration/_action_result_dto.py @@ -1,77 +1,6 @@ -"""Action and planner result DTOs for generation orchestration.""" +"""Compatibility imports for action-result orchestration DTOs.""" -import dataclasses as dc +from ._payload_dto import ActionExecutionResult as ActionExecutionResult +from ._payload_dto import PlannerResult as PlannerResult -from episodic.generation import ( - GuestBiosEnrichmentResult, # noqa: TC001 -- Python 3.14 lazy dataclass annotations are inspected by Hypothesis at runtime. - ShowNotesResult, # noqa: TC001 -- Python 3.14 lazy dataclass annotations are inspected by Hypothesis at runtime. -) -from episodic.llm import ( - LLMProviderOperation, - LLMUsage, - ProviderCallUsage, -) - -from ._dto import ExecutionPlan, _normalize_non_empty_text -from ._types import ActionKind, ModelTier - - -@dc.dataclass(frozen=True, slots=True) -class PlannerResult: - """Planner output plus normalized provider metadata.""" - - plan: ExecutionPlan - usage: LLMUsage | None - model: str - provider_response_id: str - finish_reason: str | None - provider_call_usage: ProviderCallUsage | None = None - provider_operation: LLMProviderOperation | str = ( - LLMProviderOperation.CHAT_COMPLETIONS - ) - - -@dc.dataclass(frozen=True, slots=True) -class ActionExecutionResult: - """Typed result for one executed orchestration action.""" - - action_id: str - action_kind: ActionKind - model_tier: ModelTier - model: str - summary: str - usage: LLMUsage | None = None - provider_call_usage: ProviderCallUsage | None = None - provider_operation: LLMProviderOperation | str = ( - LLMProviderOperation.CHAT_COMPLETIONS - ) - show_notes_result: ShowNotesResult | None = None - guest_bios_result: GuestBiosEnrichmentResult | None = None - - def __post_init__(self) -> None: - """Reject blank text fields and normalize enum-shaped fields.""" - for field_name in ("action_id", "model", "summary"): - value = getattr(self, field_name) - object.__setattr__( - self, field_name, _normalize_non_empty_text(value, field_name) - ) - try: - action_kind = ( - self.action_kind - if isinstance(self.action_kind, ActionKind) - else ActionKind(str(self.action_kind).strip()) - ) - except ValueError: - msg = f"Unknown action kind: {self.action_kind!r}" - raise ValueError(msg) from None - try: - model_tier = ( - self.model_tier - if isinstance(self.model_tier, ModelTier) - else ModelTier(str(self.model_tier).strip()) - ) - except ValueError: - msg = f"Unknown model tier: {self.model_tier!r}" - raise ValueError(msg) from None - object.__setattr__(self, "action_kind", action_kind) - object.__setattr__(self, "model_tier", model_tier) +__all__ = ["ActionExecutionResult", "PlannerResult"] diff --git a/episodic/orchestration/_checkpoint_dto.py b/episodic/orchestration/_checkpoint_dto.py index 65d778a4..7d758611 100644 --- a/episodic/orchestration/_checkpoint_dto.py +++ b/episodic/orchestration/_checkpoint_dto.py @@ -1,17 +1,15 @@ """Checkpoint DTOs for resumable generation orchestration.""" import dataclasses as dc -import typing as typ +import datetime as dt # noqa: TC003 - runtime annotation inspection needs this name. +import json -from ._dto import ( +from ._payload_dto import ( ActionExecutionResult, _normalize_non_empty_text, _normalize_string_fields, ) -if typ.TYPE_CHECKING: - import datetime as dt - @dc.dataclass(frozen=True, slots=True) class WorkflowCheckpoint: @@ -49,6 +47,14 @@ def __post_init__(self) -> None: if not isinstance(self.payload, dict): msg = "payload must be a mapping object." raise TypeError(msg) + try: + encoded_payload = json.dumps(self.payload, allow_nan=False) + except (TypeError, ValueError) as exc: + msg = "payload must be JSON-serializable." + raise TypeError(msg) from exc + if json.loads(encoded_payload) != self.payload: + msg = "payload must be JSON-serializable without data loss." + raise TypeError(msg) object.__setattr__(self, "payload", dict(self.payload)) diff --git a/episodic/orchestration/_checkpoint_payload.py b/episodic/orchestration/_checkpoint_payload.py index e9b45114..dc7d22f6 100644 --- a/episodic/orchestration/_checkpoint_payload.py +++ b/episodic/orchestration/_checkpoint_payload.py @@ -17,15 +17,13 @@ import typing as typ from episodic.llm import LLMUsage - -if typ.TYPE_CHECKING: - import importlib - - from episodic.orchestration import _dto as dto -else: - import importlib - - dto = importlib.import_module("episodic.orchestration._dto") +from episodic.orchestration._payload_dto import ( + ActionExecutionResult, + ExecutionPlan, + PlannedAction, + PlannerResult, +) +from episodic.orchestration._types import ActionKind, ModelTier def _usage_to_payload(usage: LLMUsage | None) -> dict[str, int] | None: @@ -132,7 +130,7 @@ def _usage_from_payload(payload: object) -> LLMUsage: ) -def _plan_to_payload(plan: dto.ExecutionPlan) -> dict[str, object]: +def _plan_to_payload(plan: ExecutionPlan) -> dict[str, object]: """Return a JSON-compatible execution-plan checkpoint payload.""" return { "plan_version": plan.plan_version, @@ -151,22 +149,22 @@ def _plan_to_payload(plan: dto.ExecutionPlan) -> dict[str, object]: } -def _planned_action_from_payload(payload: object) -> dto.PlannedAction: +def _planned_action_from_payload(payload: object) -> PlannedAction: """Return one PlannedAction from a checkpoint plan-step payload.""" step = _as_object_payload(payload, "plan step") - return dto.PlannedAction( + return PlannedAction( action_id=_required_string(step, "action_id", context="plan step"), action_kind=_required_enum( step, "action_kind", - dto.ActionKind, + ActionKind, context="plan step", ), rationale=_required_string(step, "rationale", context="plan step"), model_tier=_required_enum( step, "model_tier", - dto.ModelTier, + ModelTier, context="plan step", ), required_inputs=_required_string_list( @@ -177,14 +175,14 @@ def _planned_action_from_payload(payload: object) -> dto.PlannedAction: ) -def _plan_from_payload(payload: object) -> dto.ExecutionPlan: +def _plan_from_payload(payload: object) -> ExecutionPlan: """Return an ExecutionPlan from a checkpoint payload.""" plan_payload = _as_object_payload(payload, "plan") steps = plan_payload.get("steps") if not isinstance(steps, list): msg = "checkpoint plan steps must be a list." raise TypeError(msg) - return dto.ExecutionPlan( + return ExecutionPlan( plan_version=_required_string( plan_payload, "plan_version", @@ -204,7 +202,7 @@ def _plan_from_payload(payload: object) -> dto.ExecutionPlan: ) -def _planner_result_to_payload(result: dto.PlannerResult) -> dict[str, object]: +def _planner_result_to_payload(result: PlannerResult) -> dict[str, object]: """Return a JSON-compatible planner-result checkpoint payload.""" return { "plan": _plan_to_payload(result.plan), @@ -215,7 +213,7 @@ def _planner_result_to_payload(result: dto.PlannerResult) -> dict[str, object]: } -def _planner_result_from_payload(payload: object) -> dto.PlannerResult: +def _planner_result_from_payload(payload: object) -> PlannerResult: """Return a PlannerResult from a checkpoint payload.""" planner_payload = _as_object_payload(payload, "planner_result") try: @@ -223,7 +221,7 @@ def _planner_result_from_payload(payload: object) -> dto.PlannerResult: except KeyError as exc: msg = "checkpoint planner_result missing required field: plan" raise TypeError(msg) from exc - return dto.PlannerResult( + return PlannerResult( plan=_plan_from_payload(plan_payload), usage=( None @@ -248,7 +246,7 @@ def _planner_result_from_payload(payload: object) -> dto.PlannerResult: ) -def _action_result_to_payload(result: dto.ActionExecutionResult) -> dict[str, object]: +def _action_result_to_payload(result: ActionExecutionResult) -> dict[str, object]: """Return a JSON-compatible action-result checkpoint payload.""" return { "action_id": result.action_id, @@ -260,23 +258,23 @@ def _action_result_to_payload(result: dto.ActionExecutionResult) -> dict[str, ob } -def _action_result_from_payload(payload: object) -> dto.ActionExecutionResult: +def _action_result_from_payload(payload: object) -> ActionExecutionResult: """Return an ActionExecutionResult from a checkpoint payload.""" action_payload = _as_object_payload(payload, "action_result") - return dto.ActionExecutionResult( + return ActionExecutionResult( action_id=_required_string( action_payload, "action_id", context="action_result" ), action_kind=_required_enum( action_payload, "action_kind", - dto.ActionKind, + ActionKind, context="action_result", ), model_tier=_required_enum( action_payload, "model_tier", - dto.ModelTier, + ModelTier, context="action_result", ), model=_required_string(action_payload, "model", context="action_result"), diff --git a/episodic/orchestration/_dto.py b/episodic/orchestration/_dto.py index e35fac45..afc4dc10 100644 --- a/episodic/orchestration/_dto.py +++ b/episodic/orchestration/_dto.py @@ -1,8 +1,6 @@ """Typed DTOs and validation helpers for generation orchestration.""" -import collections.abc as cabc import dataclasses as dc -import typing as typ import uuid # noqa: TC003 - runtime annotation inspection needs this name. from episodic.llm import ( @@ -10,168 +8,61 @@ LLMTokenBudget, ) -from ._types import ( - ActionKind, - ModelTier, - PlanningResponseFormatError, +from ._payload_dto import ( + ActionExecutionResult as ActionExecutionResult, ) - - -def _require_object(value: object, field_name: str) -> dict[str, object]: - """Raise TypeError if value is not a plain dict.""" - if isinstance(value, dict): - return typ.cast("dict[str, object]", value) - msg = f"{field_name} must be an object." - raise PlanningResponseFormatError(msg) - - -def _require_non_empty_string(value: object, field_name: str) -> str: - """Raise ValueError if value is not a non-empty string.""" - if not isinstance(value, str) or not value.strip(): - msg = f"{field_name} must be a non-empty string." - raise PlanningResponseFormatError(msg) - return value.strip() - - -def _require_optional_string_list( - value: object, - field_name: str, -) -> tuple[str, ...]: - """Raise ValueError if value is neither None nor a list of strings.""" - if value is None: - return () - if not isinstance(value, list): - msg = f"{field_name} must be a list of strings." - raise PlanningResponseFormatError(msg) - items: list[str] = [] - for item in value: - if not isinstance(item, str) or not item.strip(): - msg = f"{field_name} must contain only non-empty strings." - raise PlanningResponseFormatError(msg) - items.append(item.strip()) - return tuple(items) - - -def _require_plan_step_list(value: object) -> list[dict[str, object]]: - """Raise ValueError if value is not a non-empty list.""" - if not isinstance(value, list): - msg = "steps must be a list." - raise PlanningResponseFormatError(msg) - return [_require_object(item, "step") for item in value] - - -def _coerce_action_kind(value: object) -> ActionKind: - """Return the ActionKind enum for value, raising ValueError for unknown kinds.""" - if not isinstance(value, str): - msg = "action_kind must be a non-empty string." - raise PlanningResponseFormatError(msg) - try: - return ActionKind(value.strip()) - except ValueError as exc: - expected = ", ".join(action.value for action in ActionKind) - msg = f"action_kind must be one of: {expected}." - raise PlanningResponseFormatError(msg) from exc - - -def _coerce_model_tier(value: object) -> ModelTier: - """Return the ModelTier enum for value, raising ValueError for unknown tiers.""" - if not isinstance(value, str): - msg = "model_tier must be a non-empty string." - raise PlanningResponseFormatError(msg) - try: - return ModelTier(value.strip()) - except ValueError as exc: - msg = ( - "model_tier must be one of: " - f"{ModelTier.PLANNING.value}, {ModelTier.EXECUTION.value}." - ) - raise PlanningResponseFormatError(msg) from exc - - -def _coerce_single_action_kind(element: ActionKind | str) -> ActionKind: - """Return the ActionKind enum for element, raising ValueError for unknown kinds.""" - if isinstance(element, ActionKind): - return element - try: - return ActionKind(str(element).strip()) - except ValueError: - msg = f"Unknown action kind: {element!r}" - raise ValueError(msg) from None - - -def _coerce_single_model_tier(element: ModelTier | str) -> ModelTier: - """Return the ModelTier enum for element, raising ValueError for unknown tiers.""" - if isinstance(element, ModelTier): - return element - try: - return ModelTier(str(element).strip()) - except ValueError: - msg = f"Unknown model tier: {element!r}" - raise ValueError(msg) from None - - -def _raise_required_inputs_value_error() -> typ.Never: - """Raise ValueError for malformed required_inputs values.""" - msg = "required_inputs must be an iterable of non-empty strings." - raise ValueError(msg) - - -def _normalize_required_inputs(value: object) -> tuple[str, ...]: - """Normalise required_inputs to a tuple of non-empty strings.""" - if isinstance(value, str): - _raise_required_inputs_value_error() - if not isinstance(value, cabc.Iterable): - _raise_required_inputs_value_error() - return tuple(_normalize_non_empty_text(item, "required_inputs") for item in value) - - -def _coerce_action_kinds( - kinds: tuple[ActionKind | str, ...], -) -> tuple[ActionKind, ...]: - """Return normalised ActionKind enums, raising ValueError for unknown kinds.""" - if not kinds: - msg = "enabled_action_kinds must not be empty." - raise ValueError(msg) - return tuple(_coerce_single_action_kind(element) for element in kinds) - - -def _coerce_provider_operation( - value: LLMProviderOperation | str, - field_name: str, -) -> LLMProviderOperation: - """Return the provider-operation enum or raise ValueError for unknown values.""" - if isinstance(value, LLMProviderOperation): - return value - normalised = _normalize_non_empty_text(value, field_name) - try: - return LLMProviderOperation(normalised) - except ValueError: - msg = f"Unknown {field_name}: {value!r}" - raise ValueError(msg) from None - - -def _normalize_string_fields( - obj: object, - field_names: tuple[str, ...], -) -> None: - """Normalize each named string field on a frozen dataclass instance in-place.""" - for field_name in field_names: - value = getattr(obj, field_name) - object.__setattr__( - obj, field_name, _normalize_non_empty_text(value, field_name) - ) - - -def _normalize_non_empty_text(value: object, field_name: str) -> str: - """Strip value and raise ValueError if the result is empty.""" - if not isinstance(value, str): - msg = f"{field_name} must be a non-empty string." - raise ValueError(msg) # noqa: TRY004 -- ValueError is intentional at this DTO validation raise: normalisation enforces string-shaped fields; TypeError is used for wrong Python types elsewhere. - stripped = value.strip() - if not stripped: - msg = f"{field_name} must be a non-empty string." - raise ValueError(msg) - return stripped +from ._payload_dto import ( + ExecutionPlan as ExecutionPlan, +) +from ._payload_dto import ( + PlannedAction as PlannedAction, +) +from ._payload_dto import ( + PlannerResult as PlannerResult, +) +from ._payload_dto import ( + _coerce_action_kind as _coerce_action_kind, +) +from ._payload_dto import ( + _coerce_action_kinds as _coerce_action_kinds, +) +from ._payload_dto import ( + _coerce_model_tier as _coerce_model_tier, +) +from ._payload_dto import ( + _coerce_provider_operation as _coerce_provider_operation, +) +from ._payload_dto import ( + _coerce_single_action_kind as _coerce_single_action_kind, +) +from ._payload_dto import ( + _coerce_single_model_tier as _coerce_single_model_tier, +) +from ._payload_dto import ( + _normalize_non_empty_text as _normalize_non_empty_text, +) +from ._payload_dto import ( + _normalize_required_inputs as _normalize_required_inputs, +) +from ._payload_dto import ( + _normalize_string_fields as _normalize_string_fields, +) +from ._payload_dto import ( + _raise_required_inputs_value_error as _raise_required_inputs_value_error, +) +from ._payload_dto import ( + _require_non_empty_string as _require_non_empty_string, +) +from ._payload_dto import ( + _require_object as _require_object, +) +from ._payload_dto import ( + _require_optional_string_list as _require_optional_string_list, +) +from ._payload_dto import ( + _require_plan_step_list as _require_plan_step_list, +) +from ._types import ActionKind @dc.dataclass(frozen=True, slots=True) @@ -271,66 +162,6 @@ def __post_init__(self) -> None: ) -@dc.dataclass(frozen=True, slots=True) -class PlannedAction: - """One typed step emitted by the structured planner.""" - - action_id: str - action_kind: ActionKind | str - rationale: str - model_tier: ModelTier | str - required_inputs: tuple[str, ...] = () - - def __post_init__(self) -> None: - """Reject blank identifiers, rationale text, and unknown enum fields.""" - _normalize_string_fields(self, ("action_id", "rationale")) - object.__setattr__( - self, "action_kind", _coerce_single_action_kind(self.action_kind) - ) - object.__setattr__( - self, "model_tier", _coerce_single_model_tier(self.model_tier) - ) - object.__setattr__( - self, "required_inputs", _normalize_required_inputs(self.required_inputs) - ) - - -@dc.dataclass(frozen=True, slots=True) -class ExecutionPlan: - """Structured plan derived from one planner response.""" - - plan_version: str - selected_planning_model: str - selected_execution_model: str - steps: tuple[PlannedAction, ...] - - def __post_init__(self) -> None: - """Validate top-level plan metadata and freeze the step sequence.""" - for field_name in ( - "plan_version", - "selected_planning_model", - "selected_execution_model", - ): - value = getattr(self, field_name) - object.__setattr__( - self, field_name, _normalize_non_empty_text(value, field_name) - ) - steps = tuple(self.steps) - for index, step in enumerate(steps): - if not isinstance(step, PlannedAction): - msg = ( - f"steps[{index}] must be a PlannedAction; got {type(step).__name__}" - ) - raise TypeError(msg) - object.__setattr__(self, "steps", steps) - - -from ._action_result_dto import ( # noqa: E402 # Re-export after dependent DTOs exist. - ActionExecutionResult as ActionExecutionResult, -) -from ._action_result_dto import ( # noqa: E402 # Re-export after dependent DTOs exist. - PlannerResult as PlannerResult, -) from ._checkpoint_dto import ( # noqa: E402 # Re-export after dependent DTOs exist. ResumeWorkflowCommand as ResumeWorkflowCommand, ) diff --git a/episodic/orchestration/_graph_builder.py b/episodic/orchestration/_graph_builder.py new file mode 100644 index 00000000..46471f87 --- /dev/null +++ b/episodic/orchestration/_graph_builder.py @@ -0,0 +1,276 @@ +"""LangGraph topology builder for generation orchestration.""" + +import dataclasses as dc +import importlib +import typing as typ + +from langgraph.graph import END, START, StateGraph + +from episodic.orchestration._checkpoint_resume import _suspend_execute_node +from episodic.orchestration._graph_nodes import ( + ExecuteNodeFn, + _execute_node, + _finish_node, + _plan_node, +) +from episodic.orchestration._graph_state import GenerationGraphState +from episodic.orchestration._planning_orchestrator import ( + _cost_provider_operations, + _current_billing_period_key, + _provider_call_record, + _ProviderCallContext, +) +from episodic.orchestration._types import _log_event + +if typ.TYPE_CHECKING: + import collections.abc as cabc + + from langgraph.graph.state import CompiledStateGraph + + from episodic.cost import BillingPeriodKey, CostRecorderPort + from episodic.orchestration import _dto as dto + from episodic.orchestration import _protocols as protocols +else: + dto = importlib.import_module("episodic.orchestration._dto") + protocols = importlib.import_module("episodic.orchestration._protocols") + + +@dc.dataclass(slots=True) +class GenerationGraphExtensions: + """Optional collaborators for the generation orchestration graph. + + Attributes + ---------- + checkpoint_port : CheckpointPort | None + Persistence boundary used to suspend and resume graph execution. + finish_callback : Callable[[GenerationOrchestrationResult], None] | None + Optional callback invoked with the completed orchestration result. + cost_recorder : CostRecorderPort | None + Optional port used to persist provider-call cost records. + """ + + checkpoint_port: protocols.CheckpointPort | None = None + finish_callback: cabc.Callable[[dto.GenerationOrchestrationResult], None] | None = ( + None + ) + cost_recorder: CostRecorderPort | None = None + + +def _invoke_finish_callback( + finish_callback: cabc.Callable[[dto.GenerationOrchestrationResult], None], + result: dto.GenerationOrchestrationResult, + correlation_id: str | None, +) -> None: + """Invoke *finish_callback* with the aggregated domain result. + + Logs a debug event on success and an error event on failure. + Exceptions are swallowed so that callback failures do not replace + the already-computed graph result. The callback is invoked synchronously + in the graph execution context; callbacks shared across concurrent graph + invocations must provide their own synchronization. + """ + try: + finish_callback(result) + _log_event( + "debug", + "generation_graph.finish_node.callback.finish", + correlation_id=correlation_id, + ) + except Exception as exc: # noqa: BLE001 - Callback failures must not replace a completed graph result. + _log_event( + "error", + "generation_graph.finish_node.callback.error", + correlation_id=correlation_id, + error=str(exc), + ) + + +async def _record_planner_cost_if_available( + cost_recorder: CostRecorderPort, + *, + workflow_run_id: str, + planner_result: dto.PlannerResult, + billing_period_key: BillingPeriodKey, +) -> None: + """Record a planner provider-call cost entry when usage is available.""" + if planner_result.provider_call_usage is None: + return + await cost_recorder.record_provider_call( + _provider_call_record( + context=_ProviderCallContext( + workflow_run_id=workflow_run_id, + workflow_node="planner", + logical_call_id=planner_result.provider_response_id, + model=planner_result.model, + operation=str(planner_result.provider_operation), + ), + provider_call_usage=planner_result.provider_call_usage, + billing_period_key=billing_period_key, + ) + ) + + +async def _record_action_costs_from_results( + cost_recorder: CostRecorderPort, + *, + workflow_run_id: str, + action_results: tuple[dto.ActionExecutionResult, ...], + billing_period_key: BillingPeriodKey, +) -> None: + """Record a provider-call cost entry for each action result that carries usage.""" + for action_result in action_results: + if action_result.provider_call_usage is None: + continue + await cost_recorder.record_provider_call( + _provider_call_record( + context=_ProviderCallContext( + workflow_run_id=workflow_run_id, + workflow_node=action_result.action_kind.value, + logical_call_id=action_result.action_id, + model=action_result.model, + operation=str(action_result.provider_operation), + ), + provider_call_usage=action_result.provider_call_usage, + billing_period_key=billing_period_key, + ) + ) + + +async def _record_costs_from_finished_state( + state: GenerationGraphState, + *, + cost_recorder: CostRecorderPort | None, +) -> None: + """Record graph provider-call costs from the finished direct path.""" + if cost_recorder is None: + return + if state.request is None or state.planner_result is None: + return + billing_period_key = _current_billing_period_key() + planner_result = state.planner_result + workflow_run_id = state.request.correlation_id + providers = _cost_provider_operations(planner_result) + if providers: + await cost_recorder.pin_run_pricing( + workflow_run_id, providers, billing_period_key + ) + await _record_planner_cost_if_available( + cost_recorder, + workflow_run_id=workflow_run_id, + planner_result=planner_result, + billing_period_key=billing_period_key, + ) + await _record_action_costs_from_results( + cost_recorder, + workflow_run_id=workflow_run_id, + action_results=state.action_results, + billing_period_key=billing_period_key, + ) + await cost_recorder.finalize_run(workflow_run_id, None) + + +def _build_execute_node( + tool_executor: protocols.ToolExecutorPort, + checkpoint_port: protocols.CheckpointPort | None, +) -> tuple[ExecuteNodeFn, str]: + """Return *(execute_node_fn, execute_target)* for the graph. + + When *checkpoint_port* is ``None``, returns the direct execute node + targeting ``"finish"``. Otherwise returns the suspend-before-execute node + targeting ``END``. + + Returns + ------- + The execute node callable and its graph target. + """ + if checkpoint_port is None: + + async def _run_execute_node( + state: GenerationGraphState, + ) -> dict[str, tuple[dto.ActionExecutionResult, ...]]: + """Async entry point for the execute graph node.""" + return await _execute_node(state, tool_executor=tool_executor) + + return _run_execute_node, "finish" + + async def _run_suspend_execute_node( + state: GenerationGraphState, + ) -> dict[str, dto.SuspendedWorkflowResult]: + """Async entry point for the suspend-before-execute graph node.""" + return await _suspend_execute_node( + state, + checkpoint_port=checkpoint_port, + ) + + return _run_suspend_execute_node, END + + +def build_generation_orchestration_graph( + *, + planner: protocols.PlannerPort, + tool_executor: protocols.ToolExecutorPort, + extensions: GenerationGraphExtensions | None = None, +) -> CompiledStateGraph[ + GenerationGraphState, + None, + GenerationGraphState, + GenerationGraphState, +]: + """Build the in-process generation orchestration graph. + + The returned graph plans a structured generation request, either executes + the first planned action directly and aggregates a final + `GenerationOrchestrationResult`, or suspends after planning when + `checkpoint_port` is provided. + + Args: + planner: Port used by the `plan` node to produce an execution plan. + tool_executor: Port used by the direct `execute` node to run planned + actions. + extensions: Optional persistence, callback, and cost-recording + collaborators for graph execution. + + Returns + ------- + The compiled generation orchestration graph. + """ + graph_extensions = extensions or GenerationGraphExtensions() + graph = StateGraph(GenerationGraphState) + + async def _run_plan_node( + state: GenerationGraphState, + ) -> dict[str, dto.PlannerResult]: + """Async entry point for the plan graph node.""" + return await _plan_node(state, planner=planner) + + async def _run_finish_node( + state: GenerationGraphState, + ) -> dict[str, dto.GenerationOrchestrationResult]: + """Entry point for the finish graph node.""" + result = _finish_node(state) + await _record_costs_from_finished_state( + state, cost_recorder=graph_extensions.cost_recorder + ) + if graph_extensions.finish_callback is not None: + correlation_id = ( + state.request.correlation_id if state.request is not None else None + ) + _invoke_finish_callback( + graph_extensions.finish_callback, + result["orchestration_result"], + correlation_id, + ) + return result + + execute_node, execute_target = _build_execute_node( + tool_executor, graph_extensions.checkpoint_port + ) + + graph.add_node("plan", _run_plan_node) + graph.add_node("execute", execute_node) + graph.add_node("finish", _run_finish_node) + graph.add_edge(START, "plan") + graph.add_edge("plan", "execute") + graph.add_edge("execute", execute_target) + graph.add_edge("finish", END) + return graph.compile() diff --git a/episodic/orchestration/_graph_nodes.py b/episodic/orchestration/_graph_nodes.py new file mode 100644 index 00000000..13a28948 --- /dev/null +++ b/episodic/orchestration/_graph_nodes.py @@ -0,0 +1,186 @@ +"""Ports-only graph nodes for structured generation orchestration.""" + +import importlib +import time +import typing as typ + +from episodic.orchestration._types import _log_event +from episodic.orchestration._usage import build_generation_result + +if typ.TYPE_CHECKING: + import collections.abc as cabc + + from episodic.orchestration import _dto as dto + from episodic.orchestration import _protocols as protocols + from episodic.orchestration._graph_state import GenerationGraphState +else: + dto = importlib.import_module("episodic.orchestration._dto") + protocols = importlib.import_module("episodic.orchestration._protocols") + + +type ExecuteNodeResult = ( + dict[str, tuple[dto.ActionExecutionResult, ...]] + | dict[str, dto.SuspendedWorkflowResult] +) + + +class ExecuteNodeFn(typ.Protocol): + """Callable protocol for async execute graph nodes.""" + + def __call__( + self, state: GenerationGraphState + ) -> cabc.Awaitable[ExecuteNodeResult]: + """Return the async execute-node update for *state*.""" + ... + + +async def _plan_node( + state: GenerationGraphState, + *, + planner: protocols.PlannerPort, +) -> dict[str, dto.PlannerResult]: + """Validate state and invoke the planner to produce a PlannerResult.""" + request = state.request + correlation_id = request.correlation_id if request is not None else None + _log_event( + "debug", + "generation_graph.plan_node.start", + correlation_id=correlation_id, + ) + if request is None: + msg = "missing required state value: request" + raise ValueError(msg) + try: + planner_result = await planner.plan(request) + except Exception as exc: + _log_event( + "error", + "generation_graph.plan_node.error", + correlation_id=request.correlation_id, + error=str(exc), + ) + raise + result = {"planner_result": planner_result} + _log_event( + "debug", + "generation_graph.plan_node.finish", + correlation_id=request.correlation_id, + ) + return result + + +async def _execute_single_action( + action: dto.PlannedAction, + request: dto.GenerationOrchestrationRequest, + *, + tool_executor: protocols.ToolExecutorPort, + selected_execution_model: str, +) -> dto.ActionExecutionResult: + """Execute one planned action and emit diagnostic log events.""" + started_at = time.monotonic() + action_fields = { + "correlation_id": request.correlation_id, + "action_id": action.action_id, + "action_kind": str(action.action_kind), + "model_tier": str(action.model_tier), + "execution_model": selected_execution_model, + } + try: + action_result = await tool_executor.execute(action, request) + except Exception as exc: + _log_event( + "error", + "generation_graph.execute_node.action.error", + **action_fields, + elapsed_ms=round((time.monotonic() - started_at) * 1000, 1), + error=str(exc), + ) + raise + action_fields["execution_model"] = action_result.model + _log_event( + "debug", + "generation_graph.execute_node.action.finish", + **action_fields, + elapsed_ms=round((time.monotonic() - started_at) * 1000, 1), + ) + return action_result + + +async def _execute_node( + state: GenerationGraphState, + *, + tool_executor: protocols.ToolExecutorPort, +) -> dict[str, tuple[dto.ActionExecutionResult, ...]]: + """Validate state and execute each planned action through the tool executor.""" + request = state.request + correlation_id = request.correlation_id if request is not None else None + _log_event( + "debug", + "generation_graph.execute_node.start", + correlation_id=correlation_id, + ) + if request is None: + msg = "missing required state value: request" + raise ValueError(msg) + planner_result = state.planner_result + if planner_result is None: + msg = "missing required state value: planner_result" + raise ValueError(msg) + + # Keep tool execution ordered so the graph mirrors application-service semantics. + action_results = [ + await _execute_single_action( + action, + request, + tool_executor=tool_executor, + selected_execution_model=planner_result.plan.selected_execution_model, + ) + for action in planner_result.plan.steps + ] + result = {"action_results": tuple(action_results)} + _log_event( + "debug", + "generation_graph.execute_node.finish", + correlation_id=request.correlation_id, + ) + return result + + +def _finish_node( + state: GenerationGraphState, +) -> dict[str, dto.GenerationOrchestrationResult]: + """Aggregate planner and action results into a GenerationOrchestrationResult.""" + request = state.request + correlation_id = request.correlation_id if request is not None else None + _log_event( + "debug", + "generation_graph.finish_node.start", + correlation_id=correlation_id, + ) + if request is None: + msg = "missing required state value: request" + raise ValueError(msg) + planner_result = state.planner_result + if planner_result is None: + msg = "missing required state value: planner_result" + raise ValueError(msg) + try: + orchestration_result = build_generation_result( + planner_result, + state.action_results, + ) + except Exception as exc: + _log_event( + "error", + "generation_graph.finish_node.error", + correlation_id=correlation_id, + error=str(exc), + ) + raise + result = {"orchestration_result": orchestration_result} + _log_event( + "debug", + "generation_graph.finish_node.finish", + correlation_id=correlation_id, + ) + return result diff --git a/episodic/orchestration/_payload_dto.py b/episodic/orchestration/_payload_dto.py new file mode 100644 index 00000000..10e951ed --- /dev/null +++ b/episodic/orchestration/_payload_dto.py @@ -0,0 +1,354 @@ +"""Provider-neutral DTOs for orchestration payload boundaries.""" + +import collections.abc as cabc +import dataclasses as dc +import typing as typ + +from episodic.llm import ( + LLMProviderOperation, + LLMUsage, + ProviderCallUsage, +) + +from ._types import ( + ActionKind, + ModelTier, + PlanningResponseFormatError, +) + + +def _require_object(value: object, field_name: str) -> dict[str, object]: + """Raise TypeError if value is not a plain dict.""" + if isinstance(value, dict): + return typ.cast("dict[str, object]", value) + msg = f"{field_name} must be an object." + raise PlanningResponseFormatError(msg) + + +def _require_non_empty_string(value: object, field_name: str) -> str: + """Raise ValueError if value is not a non-empty string.""" + if not isinstance(value, str) or not value.strip(): + msg = f"{field_name} must be a non-empty string." + raise PlanningResponseFormatError(msg) + return value.strip() + + +def _require_optional_string_list( + value: object, + field_name: str, +) -> tuple[str, ...]: + """Raise ValueError if value is neither None nor a list of strings.""" + if value is None: + return () + if not isinstance(value, list): + msg = f"{field_name} must be a list of strings." + raise PlanningResponseFormatError(msg) + items: list[str] = [] + for item in value: + if not isinstance(item, str) or not item.strip(): + msg = f"{field_name} must contain only non-empty strings." + raise PlanningResponseFormatError(msg) + items.append(item.strip()) + return tuple(items) + + +def _require_plan_step_list(value: object) -> list[dict[str, object]]: + """Raise ValueError if value is not a non-empty list.""" + if not isinstance(value, list): + msg = "steps must be a list." + raise PlanningResponseFormatError(msg) + return [_require_object(item, "step") for item in value] + + +def _coerce_action_kind(value: object) -> ActionKind: + """Return the ActionKind enum for value, raising ValueError for unknown kinds.""" + if not isinstance(value, str): + msg = "action_kind must be a non-empty string." + raise PlanningResponseFormatError(msg) + try: + return ActionKind(value.strip()) + except ValueError as exc: + expected = ", ".join(action.value for action in ActionKind) + msg = f"action_kind must be one of: {expected}." + raise PlanningResponseFormatError(msg) from exc + + +def _coerce_model_tier(value: object) -> ModelTier: + """Return the ModelTier enum for value, raising ValueError for unknown tiers.""" + if not isinstance(value, str): + msg = "model_tier must be a non-empty string." + raise PlanningResponseFormatError(msg) + try: + return ModelTier(value.strip()) + except ValueError as exc: + msg = ( + "model_tier must be one of: " + f"{ModelTier.PLANNING.value}, {ModelTier.EXECUTION.value}." + ) + raise PlanningResponseFormatError(msg) from exc + + +def _coerce_single_action_kind(element: ActionKind | str) -> ActionKind: + """Return the ActionKind enum for element, raising ValueError for unknown kinds.""" + if isinstance(element, ActionKind): + return element + try: + return ActionKind(str(element).strip()) + except ValueError: + msg = f"Unknown action kind: {element!r}" + raise ValueError(msg) from None + + +def _coerce_single_model_tier(element: ModelTier | str) -> ModelTier: + """Return the ModelTier enum for element, raising ValueError for unknown tiers.""" + if isinstance(element, ModelTier): + return element + try: + return ModelTier(str(element).strip()) + except ValueError: + msg = f"Unknown model tier: {element!r}" + raise ValueError(msg) from None + + +def _raise_required_inputs_value_error() -> typ.Never: + """Raise ValueError for malformed required_inputs values.""" + msg = "required_inputs must be an iterable of non-empty strings." + raise ValueError(msg) + + +def _normalize_required_inputs(value: object) -> tuple[str, ...]: + """Normalise required_inputs to a tuple of non-empty strings.""" + if isinstance(value, str): + _raise_required_inputs_value_error() + if not isinstance(value, cabc.Iterable): + _raise_required_inputs_value_error() + return tuple(_normalize_non_empty_text(item, "required_inputs") for item in value) + + +def _coerce_action_kinds( + kinds: tuple[ActionKind | str, ...], +) -> tuple[ActionKind, ...]: + """Return normalised ActionKind enums, raising ValueError for unknown kinds.""" + if not kinds: + msg = "enabled_action_kinds must not be empty." + raise ValueError(msg) + return tuple(_coerce_single_action_kind(element) for element in kinds) + + +def _coerce_provider_operation( + value: LLMProviderOperation | str, + field_name: str, +) -> LLMProviderOperation: + """Return the provider-operation enum or raise ValueError for unknown values.""" + if isinstance(value, LLMProviderOperation): + return value + normalised = _normalize_non_empty_text(value, field_name) + try: + return LLMProviderOperation(normalised) + except ValueError: + msg = f"Unknown {field_name}: {value!r}" + raise ValueError(msg) from None + + +def _normalize_string_fields( + obj: object, + field_names: tuple[str, ...], +) -> None: + """Normalize each named string field on a frozen dataclass instance in-place.""" + for field_name in field_names: + value = getattr(obj, field_name) + object.__setattr__( + obj, field_name, _normalize_non_empty_text(value, field_name) + ) + + +def _normalize_non_empty_text(value: object, field_name: str) -> str: + """Strip value and raise ValueError if the result is empty.""" + if not isinstance(value, str): + msg = f"{field_name} must be a non-empty string." + raise ValueError(msg) # noqa: TRY004 -- normalisation reports invalid field values. + stripped = value.strip() + if not stripped: + msg = f"{field_name} must be a non-empty string." + raise ValueError(msg) + return stripped + + +@dc.dataclass(frozen=True, slots=True) +class PlannedAction: + """One typed step emitted by the structured planner.""" + + action_id: str + action_kind: ActionKind | str + rationale: str + model_tier: ModelTier | str + required_inputs: tuple[str, ...] = () + + def __post_init__(self) -> None: + """Reject blank identifiers, rationale text, and unknown enum fields.""" + _normalize_string_fields(self, ("action_id", "rationale")) + object.__setattr__( + self, "action_kind", _coerce_single_action_kind(self.action_kind) + ) + object.__setattr__( + self, "model_tier", _coerce_single_model_tier(self.model_tier) + ) + object.__setattr__( + self, "required_inputs", _normalize_required_inputs(self.required_inputs) + ) + + +@dc.dataclass(frozen=True, slots=True) +class ExecutionPlan: + """Structured plan derived from one planner response.""" + + plan_version: str + selected_planning_model: str + selected_execution_model: str + steps: tuple[PlannedAction, ...] + + def __post_init__(self) -> None: + """Validate top-level plan metadata and freeze the step sequence.""" + for field_name in ( + "plan_version", + "selected_planning_model", + "selected_execution_model", + ): + value = getattr(self, field_name) + object.__setattr__( + self, field_name, _normalize_non_empty_text(value, field_name) + ) + steps = tuple(self.steps) + for index, step in enumerate(steps): + if not isinstance(step, PlannedAction): + msg = ( + f"steps[{index}] must be a PlannedAction; got {type(step).__name__}" + ) + raise TypeError(msg) + object.__setattr__(self, "steps", steps) + + +@dc.dataclass(frozen=True, slots=True) +class PlannerResult: + """Planner output plus normalized provider metadata.""" + + plan: ExecutionPlan + usage: LLMUsage | None + model: str + provider_response_id: str + finish_reason: str | None + provider_call_usage: ProviderCallUsage | None = None + provider_operation: LLMProviderOperation | str = ( + LLMProviderOperation.CHAT_COMPLETIONS + ) + + +class ShowNotesEntryAttachment(typ.Protocol): + """Provider-neutral fields consumed from one show-notes entry.""" + + @property + def topic(self) -> str: + """Return the show-note topic.""" + raise NotImplementedError + + @property + def tei_locator(self) -> str | None: + """Return the optional source TEI locator.""" + raise NotImplementedError + + +class ShowNotesResultAttachment(typ.Protocol): + """Provider-neutral shape for show-notes tool result attachments.""" + + @property + def usage(self) -> LLMUsage: + """Return token usage for the attached tool result.""" + raise NotImplementedError + + @property + def entries(self) -> tuple[ShowNotesEntryAttachment, ...]: + """Return show-note entries without importing generation DTOs.""" + raise NotImplementedError + + +class GenerationResultAttachment(typ.Protocol): + """Provider-neutral shape for nested generation result attachments.""" + + @property + def model(self) -> str: + """Return the provider model recorded by the nested generation result.""" + raise NotImplementedError + + +class GuestBioSourceAttachment(typ.Protocol): + """Provider-neutral fields consumed from one guest-bio source.""" + + @property + def reference_document_revision_id(self) -> str: + """Return the pinned source revision identifier.""" + raise NotImplementedError + + +class GuestBiosResultAttachment(typ.Protocol): + """Provider-neutral shape for guest-bios tool result attachments.""" + + @property + def generation_result(self) -> GenerationResultAttachment: + """Return the nested generation result attachment.""" + raise NotImplementedError + + @property + def sources(self) -> tuple[GuestBioSourceAttachment, ...]: + """Return source attachments without importing canonical DTOs.""" + raise NotImplementedError + + @property + def tei_xml(self) -> str: + """Return the enriched TEI payload.""" + raise NotImplementedError + + +@dc.dataclass(frozen=True, slots=True) +class ActionExecutionResult: + """Typed result for one executed orchestration action.""" + + action_id: str + action_kind: ActionKind + model_tier: ModelTier + model: str + summary: str + usage: LLMUsage | None = None + provider_call_usage: ProviderCallUsage | None = None + provider_operation: LLMProviderOperation | str = ( + LLMProviderOperation.CHAT_COMPLETIONS + ) + show_notes_result: ShowNotesResultAttachment | None = None + guest_bios_result: GuestBiosResultAttachment | None = None + + def __post_init__(self) -> None: + """Reject blank text fields and normalize enum-shaped fields.""" + for field_name in ("action_id", "model", "summary"): + value = getattr(self, field_name) + object.__setattr__( + self, field_name, _normalize_non_empty_text(value, field_name) + ) + try: + action_kind = ( + self.action_kind + if isinstance(self.action_kind, ActionKind) + else ActionKind(str(self.action_kind).strip()) + ) + except ValueError: + msg = f"Unknown action kind: {self.action_kind!r}" + raise ValueError(msg) from None + try: + model_tier = ( + self.model_tier + if isinstance(self.model_tier, ModelTier) + else ModelTier(str(self.model_tier).strip()) + ) + except ValueError: + msg = f"Unknown model tier: {self.model_tier!r}" + raise ValueError(msg) from None + object.__setattr__(self, "action_kind", action_kind) + object.__setattr__(self, "model_tier", model_tier) diff --git a/episodic/orchestration/_result_dto.py b/episodic/orchestration/_result_dto.py index 2ddc4fb4..e07f96b0 100644 --- a/episodic/orchestration/_result_dto.py +++ b/episodic/orchestration/_result_dto.py @@ -3,7 +3,7 @@ import dataclasses as dc import typing as typ -from ._dto import ActionExecutionResult, ExecutionPlan +from ._payload_dto import ActionExecutionResult, ExecutionPlan if typ.TYPE_CHECKING: from episodic.llm import LLMUsage diff --git a/episodic/orchestration/_types.py b/episodic/orchestration/_types.py index b7f686fe..05af7c26 100644 --- a/episodic/orchestration/_types.py +++ b/episodic/orchestration/_types.py @@ -1,34 +1,10 @@ """Domain enums and exceptions for generation orchestration.""" import enum -import json - -from episodic.logging import getLogger - -_log = getLogger(__name__) - - -def _log_event(level: str, message: str, **fields: object) -> None: - """Emit one structured log event with a JSON fallback. - - Logger convenience methods (``debug``, ``info``, ...) only accept - ``exc_info`` / ``stack_info`` besides the message. Structured fields are - serialized into one JSON message when needed. - """ - log_method = getattr(_log, level) - allowed_kwargs = { - k: v for k, v in fields.items() if k in {"exc_info", "stack_info"} - } - extra_fields = {k: v for k, v in fields.items() if k not in allowed_kwargs} - if extra_fields: - payload = {"event": message, **extra_fields} - log_method(json.dumps(payload, sort_keys=True), **allowed_kwargs) - return - try: - log_method(message, **allowed_kwargs) - except TypeError: - payload = {"event": message} - log_method(json.dumps(payload, sort_keys=True), **allowed_kwargs) + +from episodic.logging import ( + log_event as _log_event, # noqa: F401 - Compatibility re-export for orchestration callers. +) class ActionKind(enum.StrEnum): diff --git a/episodic/orchestration/checkpoints.py b/episodic/orchestration/checkpoints.py index 0affdd63..f4afe8f3 100644 --- a/episodic/orchestration/checkpoints.py +++ b/episodic/orchestration/checkpoints.py @@ -17,7 +17,7 @@ import dataclasses as dc import datetime as dt -from episodic.orchestration._dto import WorkflowCheckpoint +from episodic.orchestration._checkpoint_dto import WorkflowCheckpoint from episodic.orchestration._types import _log_event type TimeProvider = cabc.Callable[[], dt.datetime] diff --git a/episodic/orchestration/langgraph.py b/episodic/orchestration/langgraph.py index cfd67aa9..5217f96b 100644 --- a/episodic/orchestration/langgraph.py +++ b/episodic/orchestration/langgraph.py @@ -1,20 +1,10 @@ -"""LangGraph wrapper for structured generation orchestration. +"""Compatibility exports for the generation LangGraph orchestration. -This module owns the in-process graph topology for structured content -generation. The default graph plans, executes, and aggregates results. When a -`CheckpointPort` is supplied, the graph switches to a suspend path that -persists the planned state before the side-effecting execution step and returns -a `SuspendedWorkflowResult`; `resume_generation_orchestration` later rebuilds -the saved planner state and folds in an externally supplied action result. +The node implementations live in `_graph_nodes.py`; graph assembly and +side-effecting callback/cost wiring live in `_graph_builder.py`. This module +keeps the historical import path stable for tests and external consumers. """ -import dataclasses as dc -import importlib -import time -import typing as typ - -from langgraph.graph import END, START, StateGraph - from episodic.orchestration._checkpoint_payload import ( _action_result_from_payload as _action_result_from_payload, ) @@ -40,434 +30,28 @@ _usage_to_payload as _usage_to_payload, ) from episodic.orchestration._checkpoint_resume import ( - _suspend_execute_node as _suspend_execute_node, + resume_generation_orchestration as resume_generation_orchestration, ) -from episodic.orchestration._checkpoint_resume import ( - _validate_suspend_preconditions as _validate_suspend_preconditions, +from episodic.orchestration._graph_builder import ( + GenerationGraphExtensions as GenerationGraphExtensions, ) -from episodic.orchestration._checkpoint_resume import ( - resume_generation_orchestration as resume_generation_orchestration, +from episodic.orchestration._graph_builder import ( + _build_execute_node as _build_execute_node, ) -from episodic.orchestration._graph_state import GenerationGraphState -from episodic.orchestration._planning_orchestrator import ( - _cost_provider_operations, - _current_billing_period_key, - _provider_call_record, - _ProviderCallContext, +from episodic.orchestration._graph_builder import ( + _record_costs_from_finished_state as _record_costs_from_finished_state, ) -from episodic.orchestration._types import _log_event -from episodic.orchestration._usage import build_generation_result - -if typ.TYPE_CHECKING: - import collections.abc as cabc - - from langgraph.graph.state import CompiledStateGraph - - from episodic.cost import BillingPeriodKey, CostRecorderPort - from episodic.orchestration import _dto as dto - from episodic.orchestration import _protocols as protocols -else: - dto = importlib.import_module("episodic.orchestration._dto") - protocols = importlib.import_module("episodic.orchestration._protocols") - - -type ExecuteNodeResult = ( - dict[str, tuple[dto.ActionExecutionResult, ...]] - | dict[str, dto.SuspendedWorkflowResult] +from episodic.orchestration._graph_builder import ( + build_generation_orchestration_graph as build_generation_orchestration_graph, +) +from episodic.orchestration._graph_nodes import ExecuteNodeFn as ExecuteNodeFn +from episodic.orchestration._graph_nodes import ExecuteNodeResult as ExecuteNodeResult +from episodic.orchestration._graph_nodes import _execute_node as _execute_node +from episodic.orchestration._graph_nodes import ( + _execute_single_action as _execute_single_action, +) +from episodic.orchestration._graph_nodes import _finish_node as _finish_node +from episodic.orchestration._graph_nodes import _plan_node as _plan_node +from episodic.orchestration._graph_state import ( + GenerationGraphState as GenerationGraphState, ) - - -class ExecuteNodeFn(typ.Protocol): - """Callable protocol for async execute graph nodes.""" - - def __call__( - self, state: GenerationGraphState - ) -> cabc.Awaitable[ExecuteNodeResult]: - """Return the async execute-node update for *state*.""" - ... - - -@dc.dataclass(slots=True) -class GenerationGraphExtensions: - """Optional collaborators for the generation orchestration graph.""" - - checkpoint_port: protocols.CheckpointPort | None = None - finish_callback: cabc.Callable[[dto.GenerationOrchestrationResult], None] | None = ( - None - ) - cost_recorder: CostRecorderPort | None = None - - -async def _plan_node( - state: GenerationGraphState, - *, - planner: protocols.PlannerPort, -) -> dict[str, dto.PlannerResult]: - """Validate state and invoke the planner to produce a PlannerResult.""" - request = state.request - correlation_id = request.correlation_id if request is not None else None - _log_event( - "debug", - "generation_graph.plan_node.start", - correlation_id=correlation_id, - ) - if request is None: - msg = "missing required state value: request" - raise ValueError(msg) - try: - planner_result = await planner.plan(request) - except Exception as exc: - _log_event( - "error", - "generation_graph.plan_node.error", - correlation_id=request.correlation_id, - error=str(exc), - ) - raise - result = {"planner_result": planner_result} - _log_event( - "debug", - "generation_graph.plan_node.finish", - correlation_id=request.correlation_id, - ) - return result - - -async def _execute_single_action( - action: dto.PlannedAction, - request: dto.GenerationOrchestrationRequest, - *, - tool_executor: protocols.ToolExecutorPort, - selected_execution_model: str, -) -> dto.ActionExecutionResult: - """Execute one planned action and emit diagnostic log events.""" - started_at = time.monotonic() - action_fields = { - "correlation_id": request.correlation_id, - "action_id": action.action_id, - "action_kind": str(action.action_kind), - "model_tier": str(action.model_tier), - "execution_model": selected_execution_model, - } - try: - action_result = await tool_executor.execute(action, request) - except Exception as exc: - _log_event( - "error", - "generation_graph.execute_node.action.error", - **action_fields, - elapsed_ms=round((time.monotonic() - started_at) * 1000, 1), - error=str(exc), - ) - raise - action_fields["execution_model"] = action_result.model - _log_event( - "debug", - "generation_graph.execute_node.action.finish", - **action_fields, - elapsed_ms=round((time.monotonic() - started_at) * 1000, 1), - ) - return action_result - - -async def _execute_node( - state: GenerationGraphState, - *, - tool_executor: protocols.ToolExecutorPort, -) -> dict[str, tuple[dto.ActionExecutionResult, ...]]: - """Validate state and execute each planned action through the tool executor.""" - request = state.request - correlation_id = request.correlation_id if request is not None else None - _log_event( - "debug", - "generation_graph.execute_node.start", - correlation_id=correlation_id, - ) - if request is None: - msg = "missing required state value: request" - raise ValueError(msg) - planner_result = state.planner_result - if planner_result is None: - msg = "missing required state value: planner_result" - raise ValueError(msg) - - # Keep tool execution ordered so the graph mirrors application-service semantics. - action_results = [ - await _execute_single_action( - action, - request, - tool_executor=tool_executor, - selected_execution_model=planner_result.plan.selected_execution_model, - ) - for action in planner_result.plan.steps - ] - result = {"action_results": tuple(action_results)} - _log_event( - "debug", - "generation_graph.execute_node.finish", - correlation_id=request.correlation_id, - ) - return result - - -def _finish_node( - state: GenerationGraphState, -) -> dict[str, dto.GenerationOrchestrationResult]: - """Aggregate planner and action results into a GenerationOrchestrationResult.""" - request = state.request - correlation_id = request.correlation_id if request is not None else None - _log_event( - "debug", - "generation_graph.finish_node.start", - correlation_id=correlation_id, - ) - if request is None: - msg = "missing required state value: request" - raise ValueError(msg) - planner_result = state.planner_result - if planner_result is None: - msg = "missing required state value: planner_result" - raise ValueError(msg) - try: - orchestration_result = build_generation_result( - planner_result, - state.action_results, - ) - except Exception as exc: - _log_event( - "error", - "generation_graph.finish_node.error", - correlation_id=correlation_id, - error=str(exc), - ) - raise - result = {"orchestration_result": orchestration_result} - _log_event( - "debug", - "generation_graph.finish_node.finish", - correlation_id=correlation_id, - ) - return result - - -def _invoke_finish_callback( - finish_callback: cabc.Callable[[dto.GenerationOrchestrationResult], None], - result: dict[str, dto.GenerationOrchestrationResult], - correlation_id: str | None, -) -> None: - """Invoke *finish_callback* with the aggregated domain result. - - Logs a debug event on success and an error event on failure. - Exceptions are swallowed so that callback failures do not replace - the already-computed graph result. The callback is invoked synchronously - in the graph execution context; callbacks shared across concurrent graph - invocations must provide their own synchronization. - """ - try: - finish_callback(result["orchestration_result"]) - _log_event( - "debug", - "generation_graph.finish_node.callback.finish", - correlation_id=correlation_id, - ) - except Exception as exc: # noqa: BLE001 # Deliberately swallow callback failures to preserve the computed graph result. - _log_event( - "error", - "generation_graph.finish_node.callback.error", - correlation_id=correlation_id, - error=str(exc), - ) - - -async def _record_planner_cost_if_available( - cost_recorder: CostRecorderPort, - *, - workflow_run_id: str, - planner_result: dto.PlannerResult, - billing_period_key: BillingPeriodKey, -) -> None: - """Record a planner provider-call cost entry when usage is available.""" - if planner_result.provider_call_usage is None: - return - await cost_recorder.record_provider_call( - _provider_call_record( - context=_ProviderCallContext( - workflow_run_id=workflow_run_id, - workflow_node="planner", - logical_call_id=planner_result.provider_response_id, - model=planner_result.model, - operation=str(planner_result.provider_operation), - ), - provider_call_usage=planner_result.provider_call_usage, - billing_period_key=billing_period_key, - ) - ) - - -async def _record_action_costs_from_results( - cost_recorder: CostRecorderPort, - *, - workflow_run_id: str, - action_results: tuple[dto.ActionExecutionResult, ...], - billing_period_key: BillingPeriodKey, -) -> None: - """Record a provider-call cost entry for each action result that carries usage.""" - for action_result in action_results: - if action_result.provider_call_usage is None: - continue - await cost_recorder.record_provider_call( - _provider_call_record( - context=_ProviderCallContext( - workflow_run_id=workflow_run_id, - workflow_node=action_result.action_kind.value, - logical_call_id=action_result.action_id, - model=action_result.model, - operation=str(action_result.provider_operation), - ), - provider_call_usage=action_result.provider_call_usage, - billing_period_key=billing_period_key, - ) - ) - - -async def _record_costs_from_finished_state( - state: GenerationGraphState, - *, - cost_recorder: CostRecorderPort | None, -) -> None: - """Record graph provider-call costs from the finished direct path.""" - if cost_recorder is None: - return - if state.request is None or state.planner_result is None: - return - billing_period_key = _current_billing_period_key() - planner_result = state.planner_result - workflow_run_id = state.request.correlation_id - providers = _cost_provider_operations(planner_result) - if providers: - await cost_recorder.pin_run_pricing( - workflow_run_id, providers, billing_period_key - ) - await _record_planner_cost_if_available( - cost_recorder, - workflow_run_id=workflow_run_id, - planner_result=planner_result, - billing_period_key=billing_period_key, - ) - await _record_action_costs_from_results( - cost_recorder, - workflow_run_id=workflow_run_id, - action_results=state.action_results, - billing_period_key=billing_period_key, - ) - await cost_recorder.finalize_run(workflow_run_id, None) - - -def _build_execute_node( - tool_executor: protocols.ToolExecutorPort, - checkpoint_port: protocols.CheckpointPort | None, -) -> tuple[ExecuteNodeFn, str]: - """Return *(execute_node_fn, execute_target)* for the graph. - - When *checkpoint_port* is ``None``, returns the direct execute node - targeting ``"finish"``. Otherwise returns the suspend-before-execute node - targeting ``END``. - - Returns - ------- - tuple[ExecuteNodeFn, str] - The execute-node callable and its graph target. The target is - ``"finish"`` for direct execution or ``END`` for checkpoint - suspension. - """ - if checkpoint_port is None: - - async def _run_execute_node( - state: GenerationGraphState, - ) -> dict[str, tuple[dto.ActionExecutionResult, ...]]: - """Async entry point for the execute graph node.""" - return await _execute_node(state, tool_executor=tool_executor) - - return _run_execute_node, "finish" - - async def _run_suspend_execute_node( - state: GenerationGraphState, - ) -> dict[str, dto.SuspendedWorkflowResult]: - """Async entry point for the suspend-before-execute graph node.""" - return await _suspend_execute_node( - state, - checkpoint_port=checkpoint_port, - ) - - return _run_suspend_execute_node, END - - -def build_generation_orchestration_graph( - *, - planner: protocols.PlannerPort, - tool_executor: protocols.ToolExecutorPort, - extensions: GenerationGraphExtensions | None = None, -) -> CompiledStateGraph[ - GenerationGraphState, - None, - GenerationGraphState, - GenerationGraphState, -]: - """Build the in-process generation orchestration graph. - - The returned graph plans a structured generation request, either executes - the first planned action directly and aggregates a final - `GenerationOrchestrationResult`, or suspends after planning when - `checkpoint_port` is provided. - - Args: - planner: Port used by the `plan` node to produce an execution plan. - tool_executor: Port used by the direct `execute` node to run planned - actions. - extensions: Optional persistence, callback, and cost-recording - collaborators for graph execution. - - Returns - ------- - CompiledStateGraph - The compiled orchestration graph containing the ``plan``, ``execute``, - and ``finish`` nodes. - """ - graph_extensions = extensions or GenerationGraphExtensions() - graph = StateGraph(GenerationGraphState) - - async def _run_plan_node( - state: GenerationGraphState, - ) -> dict[str, dto.PlannerResult]: - """Async entry point for the plan graph node.""" - return await _plan_node(state, planner=planner) - - async def _run_finish_node( - state: GenerationGraphState, - ) -> dict[str, dto.GenerationOrchestrationResult]: - """Entry point for the finish graph node.""" - result = _finish_node(state) - await _record_costs_from_finished_state( - state, cost_recorder=graph_extensions.cost_recorder - ) - if graph_extensions.finish_callback is not None: - correlation_id = ( - state.request.correlation_id if state.request is not None else None - ) - _invoke_finish_callback( - graph_extensions.finish_callback, result, correlation_id - ) - return result - - execute_node, execute_target = _build_execute_node( - tool_executor, graph_extensions.checkpoint_port - ) - - graph.add_node("plan", _run_plan_node) - graph.add_node("execute", execute_node) - graph.add_node("finish", _run_finish_node) - graph.add_edge(START, "plan") - graph.add_edge("plan", "execute") - graph.add_edge("execute", execute_target) - graph.add_edge("finish", END) - return graph.compile() diff --git a/episodic/worker/__init__.py b/episodic/worker/__init__.py index a22a23f3..64d9155c 100644 --- a/episodic/worker/__init__.py +++ b/episodic/worker/__init__.py @@ -19,7 +19,8 @@ IoDiagnosticResult, WorkerDependencies, ) -from .topology import DEFAULT_WORKER_TOPOLOGY, WorkerTopology, WorkloadClass +from .topology import DEFAULT_WORKER_TOPOLOGY, WorkerTopology +from .workloads import WorkloadClass __all__ = [ "CPU_DIAGNOSTIC_TASK_NAME", diff --git a/episodic/worker/runtime.py b/episodic/worker/runtime.py index 098de9ad..531ef035 100644 --- a/episodic/worker/runtime.py +++ b/episodic/worker/runtime.py @@ -27,7 +27,8 @@ from episodic.logging import get_logger from .tasks import SCAFFOLD_TASK_WORKLOADS, WorkerDependencies, register_scaffold_tasks -from .topology import DEFAULT_WORKER_TOPOLOGY, WorkerTopology, WorkloadClass +from .topology import DEFAULT_WORKER_TOPOLOGY, WorkerTopology +from .workloads import WorkloadClass logger = get_logger(__name__) diff --git a/episodic/worker/tasks.py b/episodic/worker/tasks.py index f64d8ae7..445770d6 100644 --- a/episodic/worker/tasks.py +++ b/episodic/worker/tasks.py @@ -14,7 +14,7 @@ import hashlib import typing as typ -from .topology import WorkloadClass +from .workloads import WorkloadClass if typ.TYPE_CHECKING: from celery import Celery diff --git a/episodic/worker/topology.py b/episodic/worker/topology.py index 8481884d..86917752 100644 --- a/episodic/worker/topology.py +++ b/episodic/worker/topology.py @@ -8,19 +8,13 @@ import collections.abc as cabc # noqa: TC003 # This type remains available at runtime for annotation introspection. import dataclasses as dc -import enum import types from kombu import Exchange, Queue -MIN_DOTTED_TASK_NAME_PARTS = 2 - +from .workloads import WorkloadClass as WorkloadClass -class WorkloadClass(enum.StrEnum): - """Canonical workload classes for routed Celery tasks.""" - - IO_BOUND = "io_bound" - CPU_BOUND = "cpu_bound" +MIN_DOTTED_TASK_NAME_PARTS = 2 def _validate_queue_spec_strings( diff --git a/episodic/worker/workloads.py b/episodic/worker/workloads.py new file mode 100644 index 00000000..8e545e23 --- /dev/null +++ b/episodic/worker/workloads.py @@ -0,0 +1,24 @@ +"""Provider-neutral workload classifications for worker routing. + +Examples +-------- +>>> workload_class = WorkloadClass.IO_BOUND +>>> workload_class.value +'io_bound' +""" + +import enum + + +class WorkloadClass(enum.StrEnum): + """Canonical workload classes for routed Celery tasks. + + Examples + -------- + >>> workload_class = WorkloadClass.IO_BOUND + >>> workload_class.value + 'io_bound' + """ + + IO_BOUND = "io_bound" + CPU_BOUND = "cpu_bound" diff --git a/pyproject.toml b/pyproject.toml index eacc3597..09f03e31 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -794,6 +794,10 @@ allowed = [ "composition_root", "domain_ports", "inbound_adapter", + "orchestration", + "orchestration_checkpoint", + "orchestration_nodes", + "orchestration_tasks", "outbound_adapter", ] @@ -823,9 +827,38 @@ prefixes = [ "episodic.cost.engine", "episodic.llm.ports", "episodic.metrics_ports", + "episodic.worker.workloads", ] allowed = ["domain_ports"] +[[tool.hecate.groups]] +name = "orchestration_checkpoint" +prefixes = [ + "episodic.orchestration._checkpoint_payload", + "episodic.orchestration._checkpoint_dto", + "episodic.orchestration._payload_dto", +] +allowed = ["orchestration_checkpoint", "domain_ports"] + +[[tool.hecate.groups]] +name = "orchestration_nodes" +prefixes = ["episodic.orchestration._graph_nodes"] +allowed = ["orchestration_nodes", "domain_ports", "orchestration_checkpoint"] + +[[tool.hecate.groups]] +name = "orchestration" +prefixes = [ + "episodic.orchestration._graph_builder", + "episodic.orchestration.langgraph", +] +allowed = [ + "orchestration", + "application", + "domain_ports", + "orchestration_checkpoint", + "orchestration_nodes", +] + [[tool.hecate.groups]] name = "application" prefixes = [ @@ -838,15 +871,20 @@ prefixes = [ "episodic.canonical.reference_documents", "episodic.cost.recorder", "episodic.generation", - "episodic.orchestration", ] allowed = ["application", "domain_ports"] +[[tool.hecate.groups]] +name = "orchestration_tasks" +prefixes = [ + "episodic.worker.tasks", +] +allowed = ["orchestration_tasks", "application", "domain_ports"] + [[tool.hecate.groups]] name = "inbound_adapter" prefixes = [ "episodic.api", - "episodic.worker.tasks", "episodic.worker.topology", ] allowed = ["inbound_adapter", "application", "domain_ports"] @@ -862,7 +900,7 @@ prefixes = [ "episodic.llm.openai_adapter", "episodic.llm.openai_client", ] -allowed = ["outbound_adapter", "application", "domain_ports"] +allowed = ["outbound_adapter", "application", "domain_ports", "orchestration_checkpoint"] [build-system] requires = ["uv_build>=0.12.5,<0.13.0"] diff --git a/tests/__snapshots__/test_architecture_enforcement.ambr b/tests/__snapshots__/test_architecture_enforcement.ambr index 5828c260..dff95102 100644 --- a/tests/__snapshots__/test_architecture_enforcement.ambr +++ b/tests/__snapshots__/test_architecture_enforcement.ambr @@ -9,3 +9,49 @@ ''' # --- +# name: test_orchestration_json_diagnostics_match_snapshot + dict({ + 'celery_task_imports_inbound_adapter': dict({ + 'ok': False, + 'violations': list([ + dict({ + 'imported': 'tests.fixtures.architecture.celery_task_imports_inbound_adapter.api', + 'imported_group': 'inbound_adapter', + 'importer': 'tests.fixtures.architecture.celery_task_imports_inbound_adapter.worker.tasks', + 'importer_group': 'orchestration_tasks', + 'line': 3, + 'rule_id': 'ARCH001', + 'source_path': 'tests/fixtures/architecture/celery_task_imports_inbound_adapter/worker/tasks.py', + }), + ]), + }), + 'checkpoint_payload_imports_storage': dict({ + 'ok': False, + 'violations': list([ + dict({ + 'imported': 'tests.fixtures.architecture.checkpoint_payload_imports_storage.storage', + 'imported_group': 'outbound_adapter', + 'importer': 'tests.fixtures.architecture.checkpoint_payload_imports_storage.orchestration._checkpoint_payload', + 'importer_group': 'orchestration_checkpoint', + 'line': 3, + 'rule_id': 'ARCH001', + 'source_path': 'tests/fixtures/architecture/checkpoint_payload_imports_storage/orchestration/_checkpoint_payload.py', + }), + ]), + }), + 'orchestration_node_imports_outbound_adapter': dict({ + 'ok': False, + 'violations': list([ + dict({ + 'imported': 'tests.fixtures.architecture.orchestration_node_imports_outbound_adapter.storage', + 'imported_group': 'outbound_adapter', + 'importer': 'tests.fixtures.architecture.orchestration_node_imports_outbound_adapter.orchestration._graph_nodes', + 'importer_group': 'orchestration_nodes', + 'line': 3, + 'rule_id': 'ARCH001', + 'source_path': 'tests/fixtures/architecture/orchestration_node_imports_outbound_adapter/orchestration/_graph_nodes.py', + }), + ]), + }), + }) +# --- diff --git a/tests/__snapshots__/test_architecture_hecate_config.ambr b/tests/__snapshots__/test_architecture_hecate_config.ambr deleted file mode 100644 index cc2cb5cd..00000000 --- a/tests/__snapshots__/test_architecture_hecate_config.ambr +++ /dev/null @@ -1,15 +0,0 @@ -# serializer version: 1 -# name: test_fixture_check_uses_injected_python_and_explicit_arguments - list([ - '/custom/python', - '-m', - 'hecate', - 'check', - '--config', - '', - '--package', - 'tests.fixtures.architecture.allowed_case', - '--root', - '', - ]) -# --- diff --git a/tests/architecture_hecate_config.py b/tests/architecture_hecate_config.py index e305a99e..dcef2f97 100644 --- a/tests/architecture_hecate_config.py +++ b/tests/architecture_hecate_config.py @@ -27,10 +27,34 @@ "composition_root", "domain", "inbound_adapter", + "orchestration", + "orchestration_checkpoint", + "orchestration_nodes", + "orchestration_tasks", "outbound_adapter", ) DOMAIN_GROUPS: tuple[str, ...] = ("domain",) APPLICATION_GROUPS: tuple[str, ...] = ("application", "domain") +ORCHESTRATION_CHECKPOINT_GROUPS: tuple[str, ...] = ( + "orchestration_checkpoint", + "domain", +) +ORCHESTRATION_NODE_GROUPS: tuple[str, ...] = ( + "orchestration_nodes", + "domain", + "orchestration_checkpoint", +) +ORCHESTRATION_GROUPS: tuple[str, ...] = ( + "orchestration", + "application", + "domain", + "orchestration_checkpoint", +) +ORCHESTRATION_TASK_GROUPS: tuple[str, ...] = ( + "orchestration_tasks", + "application", + "domain", +) INBOUND_ADAPTER_GROUPS: tuple[str, ...] = ("inbound_adapter", "application", "domain") OUTBOUND_ADAPTER_GROUPS: tuple[str, ...] = ( "outbound_adapter", @@ -101,6 +125,7 @@ def run_hecate_fixture_check( config_path: Path, *, python_executable: str | Path = sys.executable, + output_format: str = "text", ) -> subprocess.CompletedProcess[str]: """Run Hecate against one architecture fixture package. @@ -113,6 +138,9 @@ def run_hecate_fixture_check( python_executable : str | Path Python executable used to invoke `python -m hecate`. Tests may inject a substitute executable when validating command construction. + output_format : str + Hecate output renderer to request. Supported values are `text` and + `json`. Returns ------- @@ -143,6 +171,8 @@ def run_hecate_fixture_check( package, "--root", str(package_root), + "--format", + output_format, ] try: return subprocess.run( # noqa: S603 # shell=False with trusted test args. @@ -208,13 +238,19 @@ def run_hecate_production_check( def _fixture_config(package: str, *, treats_package_barrel_as_outbound: bool) -> str: """Return fixture-specific Hecate TOML.""" outbound_prefixes = ( - f'"{package}.storage", "{package}"' + f'"{package}.storage", "{package}.adapter", "{package}"' if treats_package_barrel_as_outbound - else f'"{package}.storage"' + else f'"{package}.storage", "{package}.adapter"' ) composition_root_allowed = _toml_string_array(COMPOSITION_ROOT_GROUPS) domain_allowed = _toml_string_array(DOMAIN_GROUPS) application_allowed = _toml_string_array(APPLICATION_GROUPS) + orchestration_checkpoint_allowed = _toml_string_array( + ORCHESTRATION_CHECKPOINT_GROUPS, + ) + orchestration_node_allowed = _toml_string_array(ORCHESTRATION_NODE_GROUPS) + orchestration_allowed = _toml_string_array(ORCHESTRATION_GROUPS) + orchestration_task_allowed = _toml_string_array(ORCHESTRATION_TASK_GROUPS) inbound_adapter_allowed = _toml_string_array(INBOUND_ADAPTER_GROUPS) outbound_adapter_allowed = _toml_string_array(OUTBOUND_ADAPTER_GROUPS) return textwrap.dedent( @@ -230,9 +266,33 @@ def _fixture_config(package: str, *, treats_package_barrel_as_outbound: bool) -> [[tool.hecate.groups]] name = "domain" - prefixes = ["{package}.domain"] + prefixes = ["{package}.domain", "{package}.worker.workloads"] allowed = {domain_allowed} + [[tool.hecate.groups]] + name = "orchestration_checkpoint" + prefixes = [ + "{package}.orchestration._checkpoint_payload", + "{package}.orchestration._checkpoint_dto", + "{package}.orchestration._payload_dto", + ] + allowed = {orchestration_checkpoint_allowed} + + [[tool.hecate.groups]] + name = "orchestration_nodes" + prefixes = ["{package}.orchestration._graph_nodes"] + allowed = {orchestration_node_allowed} + + [[tool.hecate.groups]] + name = "orchestration_tasks" + prefixes = ["{package}.worker.tasks"] + allowed = {orchestration_task_allowed} + + [[tool.hecate.groups]] + name = "orchestration" + prefixes = ["{package}.orchestration"] + allowed = {orchestration_allowed} + [[tool.hecate.groups]] name = "application" prefixes = ["{package}.service"] @@ -240,7 +300,7 @@ def _fixture_config(package: str, *, treats_package_barrel_as_outbound: bool) -> [[tool.hecate.groups]] name = "inbound_adapter" - prefixes = ["{package}.api"] + prefixes = ["{package}.api", "{package}.worker.topology"] allowed = {inbound_adapter_allowed} [[tool.hecate.groups]] diff --git a/tests/features/architecture_enforcement.feature b/tests/features/architecture_enforcement.feature index efa6a92c..d8996a1a 100644 --- a/tests/features/architecture_enforcement.feature +++ b/tests/features/architecture_enforcement.feature @@ -20,3 +20,29 @@ Feature: Architecture enforcement Given the architecture fixture package "composition_root_allows_wiring" When the architecture checker runs Then the architecture check passes + + Scenario: A clean orchestration fixture passes + Given the architecture fixture package "orchestration_node_imports_port" + When the architecture checker runs + Then the architecture check passes + + Scenario: A LangGraph node importing an adapter is rejected + Given the architecture fixture package "orchestration_node_imports_outbound_adapter" + When the architecture checker runs + Then the architecture check fails + And the architecture diagnostic mentions "ARCH001" + And the architecture diagnostic mentions "orchestration._graph_nodes" + + Scenario: A Celery task importing an adapter is rejected + Given the architecture fixture package "celery_task_imports_inbound_adapter" + When the architecture checker runs + Then the architecture check fails + And the architecture diagnostic mentions "ARCH001" + And the architecture diagnostic mentions "worker.tasks" + + Scenario: A checkpoint payload importing storage is rejected + Given the architecture fixture package "checkpoint_payload_imports_storage" + When the architecture checker runs + Then the architecture check fails + And the architecture diagnostic mentions "ARCH001" + And the architecture diagnostic mentions "orchestration._checkpoint_payload" diff --git a/tests/fixtures/architecture/celery_task_imports_domain_service/__init__.py b/tests/fixtures/architecture/celery_task_imports_domain_service/__init__.py new file mode 100644 index 00000000..6b57d4f0 --- /dev/null +++ b/tests/fixtures/architecture/celery_task_imports_domain_service/__init__.py @@ -0,0 +1 @@ +"""Fixture package where a Celery task imports domain-facing contracts.""" diff --git a/tests/fixtures/architecture/celery_task_imports_domain_service/domain.py b/tests/fixtures/architecture/celery_task_imports_domain_service/domain.py new file mode 100644 index 00000000..7b8f1f36 --- /dev/null +++ b/tests/fixtures/architecture/celery_task_imports_domain_service/domain.py @@ -0,0 +1,3 @@ +"""Domain fixture for an allowed task dependency.""" + +VALUE: str = "domain" diff --git a/tests/fixtures/architecture/celery_task_imports_domain_service/service.py b/tests/fixtures/architecture/celery_task_imports_domain_service/service.py new file mode 100644 index 00000000..48b24b8b --- /dev/null +++ b/tests/fixtures/architecture/celery_task_imports_domain_service/service.py @@ -0,0 +1,5 @@ +"""Application-service fixture for an allowed task dependency.""" + +from tests.fixtures.architecture.celery_task_imports_domain_service import domain + +VALUE: str = domain.VALUE diff --git a/tests/fixtures/architecture/celery_task_imports_domain_service/worker/__init__.py b/tests/fixtures/architecture/celery_task_imports_domain_service/worker/__init__.py new file mode 100644 index 00000000..884fdbfa --- /dev/null +++ b/tests/fixtures/architecture/celery_task_imports_domain_service/worker/__init__.py @@ -0,0 +1 @@ +"""Worker package for an allowed task fixture.""" diff --git a/tests/fixtures/architecture/celery_task_imports_domain_service/worker/tasks.py b/tests/fixtures/architecture/celery_task_imports_domain_service/worker/tasks.py new file mode 100644 index 00000000..02fbaed8 --- /dev/null +++ b/tests/fixtures/architecture/celery_task_imports_domain_service/worker/tasks.py @@ -0,0 +1,8 @@ +"""Allowed Celery task fixture.""" + +from tests.fixtures.architecture.celery_task_imports_domain_service import service +from tests.fixtures.architecture.celery_task_imports_domain_service.worker import ( + workloads, +) + +VALUE: tuple[str, str] = (service.VALUE, workloads.VALUE) diff --git a/tests/fixtures/architecture/celery_task_imports_domain_service/worker/workloads.py b/tests/fixtures/architecture/celery_task_imports_domain_service/worker/workloads.py new file mode 100644 index 00000000..5db5296f --- /dev/null +++ b/tests/fixtures/architecture/celery_task_imports_domain_service/worker/workloads.py @@ -0,0 +1,3 @@ +"""Domain-port workload contract fixture.""" + +VALUE: str = "workload" diff --git a/tests/fixtures/architecture/celery_task_imports_inbound_adapter/__init__.py b/tests/fixtures/architecture/celery_task_imports_inbound_adapter/__init__.py new file mode 100644 index 00000000..372ef437 --- /dev/null +++ b/tests/fixtures/architecture/celery_task_imports_inbound_adapter/__init__.py @@ -0,0 +1 @@ +"""Fixture package where a Celery task imports an inbound adapter.""" diff --git a/tests/fixtures/architecture/celery_task_imports_inbound_adapter/api.py b/tests/fixtures/architecture/celery_task_imports_inbound_adapter/api.py new file mode 100644 index 00000000..466d744c --- /dev/null +++ b/tests/fixtures/architecture/celery_task_imports_inbound_adapter/api.py @@ -0,0 +1,3 @@ +"""Inbound-adapter fixture.""" + +VALUE = "api" diff --git a/tests/fixtures/architecture/celery_task_imports_inbound_adapter/worker/__init__.py b/tests/fixtures/architecture/celery_task_imports_inbound_adapter/worker/__init__.py new file mode 100644 index 00000000..22bcd7d1 --- /dev/null +++ b/tests/fixtures/architecture/celery_task_imports_inbound_adapter/worker/__init__.py @@ -0,0 +1 @@ +"""Worker package for a task boundary violation fixture.""" diff --git a/tests/fixtures/architecture/celery_task_imports_inbound_adapter/worker/tasks.py b/tests/fixtures/architecture/celery_task_imports_inbound_adapter/worker/tasks.py new file mode 100644 index 00000000..c5179f2c --- /dev/null +++ b/tests/fixtures/architecture/celery_task_imports_inbound_adapter/worker/tasks.py @@ -0,0 +1,5 @@ +"""Violating Celery task fixture.""" + +from tests.fixtures.architecture.celery_task_imports_inbound_adapter import api + +VALUE: str = api.VALUE diff --git a/tests/fixtures/architecture/celery_task_imports_outbound_adapter/__init__.py b/tests/fixtures/architecture/celery_task_imports_outbound_adapter/__init__.py new file mode 100644 index 00000000..235d848c --- /dev/null +++ b/tests/fixtures/architecture/celery_task_imports_outbound_adapter/__init__.py @@ -0,0 +1 @@ +"""Fixture package where a Celery task imports outbound storage.""" diff --git a/tests/fixtures/architecture/celery_task_imports_outbound_adapter/storage.py b/tests/fixtures/architecture/celery_task_imports_outbound_adapter/storage.py new file mode 100644 index 00000000..1986dd44 --- /dev/null +++ b/tests/fixtures/architecture/celery_task_imports_outbound_adapter/storage.py @@ -0,0 +1,3 @@ +"""Outbound storage fixture.""" + +VALUE = "storage" diff --git a/tests/fixtures/architecture/celery_task_imports_outbound_adapter/worker/__init__.py b/tests/fixtures/architecture/celery_task_imports_outbound_adapter/worker/__init__.py new file mode 100644 index 00000000..669a641b --- /dev/null +++ b/tests/fixtures/architecture/celery_task_imports_outbound_adapter/worker/__init__.py @@ -0,0 +1 @@ +"""Worker package for an outbound-adapter task violation fixture.""" diff --git a/tests/fixtures/architecture/celery_task_imports_outbound_adapter/worker/tasks.py b/tests/fixtures/architecture/celery_task_imports_outbound_adapter/worker/tasks.py new file mode 100644 index 00000000..eb22cd17 --- /dev/null +++ b/tests/fixtures/architecture/celery_task_imports_outbound_adapter/worker/tasks.py @@ -0,0 +1,5 @@ +"""Violating Celery task fixture.""" + +from tests.fixtures.architecture.celery_task_imports_outbound_adapter import storage + +VALUE: str = storage.VALUE diff --git a/tests/fixtures/architecture/checkpoint_payload_imports_application/__init__.py b/tests/fixtures/architecture/checkpoint_payload_imports_application/__init__.py new file mode 100644 index 00000000..83af2824 --- /dev/null +++ b/tests/fixtures/architecture/checkpoint_payload_imports_application/__init__.py @@ -0,0 +1 @@ +"""Fixture package where a checkpoint payload imports application code.""" diff --git a/tests/fixtures/architecture/checkpoint_payload_imports_application/domain.py b/tests/fixtures/architecture/checkpoint_payload_imports_application/domain.py new file mode 100644 index 00000000..4eecdc19 --- /dev/null +++ b/tests/fixtures/architecture/checkpoint_payload_imports_application/domain.py @@ -0,0 +1,3 @@ +"""Domain fixture for a checkpoint application dependency.""" + +VALUE = "domain" diff --git a/tests/fixtures/architecture/checkpoint_payload_imports_application/orchestration/__init__.py b/tests/fixtures/architecture/checkpoint_payload_imports_application/orchestration/__init__.py new file mode 100644 index 00000000..49c706e6 --- /dev/null +++ b/tests/fixtures/architecture/checkpoint_payload_imports_application/orchestration/__init__.py @@ -0,0 +1 @@ +"""Orchestration package for a checkpoint application violation fixture.""" diff --git a/tests/fixtures/architecture/checkpoint_payload_imports_application/orchestration/_checkpoint_payload.py b/tests/fixtures/architecture/checkpoint_payload_imports_application/orchestration/_checkpoint_payload.py new file mode 100644 index 00000000..1b2e8d58 --- /dev/null +++ b/tests/fixtures/architecture/checkpoint_payload_imports_application/orchestration/_checkpoint_payload.py @@ -0,0 +1,5 @@ +"""Violating checkpoint payload fixture.""" + +from tests.fixtures.architecture.checkpoint_payload_imports_application import service + +VALUE: str = service.VALUE diff --git a/tests/fixtures/architecture/checkpoint_payload_imports_application/service.py b/tests/fixtures/architecture/checkpoint_payload_imports_application/service.py new file mode 100644 index 00000000..b85358b2 --- /dev/null +++ b/tests/fixtures/architecture/checkpoint_payload_imports_application/service.py @@ -0,0 +1,5 @@ +"""Application-service fixture.""" + +from tests.fixtures.architecture.checkpoint_payload_imports_application import domain + +VALUE: str = domain.VALUE diff --git a/tests/fixtures/architecture/checkpoint_payload_imports_storage/__init__.py b/tests/fixtures/architecture/checkpoint_payload_imports_storage/__init__.py new file mode 100644 index 00000000..d4d7d3dc --- /dev/null +++ b/tests/fixtures/architecture/checkpoint_payload_imports_storage/__init__.py @@ -0,0 +1 @@ +"""Fixture package where a checkpoint payload imports storage.""" diff --git a/tests/fixtures/architecture/checkpoint_payload_imports_storage/orchestration/__init__.py b/tests/fixtures/architecture/checkpoint_payload_imports_storage/orchestration/__init__.py new file mode 100644 index 00000000..2779b031 --- /dev/null +++ b/tests/fixtures/architecture/checkpoint_payload_imports_storage/orchestration/__init__.py @@ -0,0 +1 @@ +"""Orchestration package for a checkpoint boundary violation fixture.""" diff --git a/tests/fixtures/architecture/checkpoint_payload_imports_storage/orchestration/_checkpoint_payload.py b/tests/fixtures/architecture/checkpoint_payload_imports_storage/orchestration/_checkpoint_payload.py new file mode 100644 index 00000000..b1ebb68b --- /dev/null +++ b/tests/fixtures/architecture/checkpoint_payload_imports_storage/orchestration/_checkpoint_payload.py @@ -0,0 +1,5 @@ +"""Violating checkpoint payload fixture.""" + +from tests.fixtures.architecture.checkpoint_payload_imports_storage import storage + +VALUE: str = storage.VALUE diff --git a/tests/fixtures/architecture/checkpoint_payload_imports_storage/storage.py b/tests/fixtures/architecture/checkpoint_payload_imports_storage/storage.py new file mode 100644 index 00000000..1986dd44 --- /dev/null +++ b/tests/fixtures/architecture/checkpoint_payload_imports_storage/storage.py @@ -0,0 +1,3 @@ +"""Outbound storage fixture.""" + +VALUE = "storage" diff --git a/tests/fixtures/architecture/orchestration_imports_domain_service/__init__.py b/tests/fixtures/architecture/orchestration_imports_domain_service/__init__.py new file mode 100644 index 00000000..d1d60224 --- /dev/null +++ b/tests/fixtures/architecture/orchestration_imports_domain_service/__init__.py @@ -0,0 +1 @@ +"""Fixture package where orchestration imports an application service.""" diff --git a/tests/fixtures/architecture/orchestration_imports_domain_service/domain.py b/tests/fixtures/architecture/orchestration_imports_domain_service/domain.py new file mode 100644 index 00000000..203d3971 --- /dev/null +++ b/tests/fixtures/architecture/orchestration_imports_domain_service/domain.py @@ -0,0 +1,3 @@ +"""Domain fixture for an orchestration dependency.""" + +VALUE: str = "domain" diff --git a/tests/fixtures/architecture/orchestration_imports_domain_service/orchestration/__init__.py b/tests/fixtures/architecture/orchestration_imports_domain_service/orchestration/__init__.py new file mode 100644 index 00000000..6f92e6b4 --- /dev/null +++ b/tests/fixtures/architecture/orchestration_imports_domain_service/orchestration/__init__.py @@ -0,0 +1 @@ +"""Orchestration package for an allowed application dependency.""" diff --git a/tests/fixtures/architecture/orchestration_imports_domain_service/orchestration/generation.py b/tests/fixtures/architecture/orchestration_imports_domain_service/orchestration/generation.py new file mode 100644 index 00000000..e517d956 --- /dev/null +++ b/tests/fixtures/architecture/orchestration_imports_domain_service/orchestration/generation.py @@ -0,0 +1,5 @@ +"""Allowed orchestration fixture.""" + +from tests.fixtures.architecture.orchestration_imports_domain_service import service + +VALUE: str = service.VALUE diff --git a/tests/fixtures/architecture/orchestration_imports_domain_service/service.py b/tests/fixtures/architecture/orchestration_imports_domain_service/service.py new file mode 100644 index 00000000..5f6f373e --- /dev/null +++ b/tests/fixtures/architecture/orchestration_imports_domain_service/service.py @@ -0,0 +1,5 @@ +"""Application-service fixture.""" + +from tests.fixtures.architecture.orchestration_imports_domain_service import domain + +VALUE: str = domain.VALUE diff --git a/tests/fixtures/architecture/orchestration_imports_inbound_adapter/__init__.py b/tests/fixtures/architecture/orchestration_imports_inbound_adapter/__init__.py new file mode 100644 index 00000000..b80de477 --- /dev/null +++ b/tests/fixtures/architecture/orchestration_imports_inbound_adapter/__init__.py @@ -0,0 +1 @@ +"""Fixture package where orchestration imports an inbound adapter.""" diff --git a/tests/fixtures/architecture/orchestration_imports_inbound_adapter/api.py b/tests/fixtures/architecture/orchestration_imports_inbound_adapter/api.py new file mode 100644 index 00000000..72ff6d26 --- /dev/null +++ b/tests/fixtures/architecture/orchestration_imports_inbound_adapter/api.py @@ -0,0 +1,3 @@ +"""Inbound-adapter fixture.""" + +VALUE: str = "api" diff --git a/tests/fixtures/architecture/orchestration_imports_inbound_adapter/orchestration/__init__.py b/tests/fixtures/architecture/orchestration_imports_inbound_adapter/orchestration/__init__.py new file mode 100644 index 00000000..a1109fbc --- /dev/null +++ b/tests/fixtures/architecture/orchestration_imports_inbound_adapter/orchestration/__init__.py @@ -0,0 +1 @@ +"""Orchestration package for an inbound-adapter violation.""" diff --git a/tests/fixtures/architecture/orchestration_imports_inbound_adapter/orchestration/generation.py b/tests/fixtures/architecture/orchestration_imports_inbound_adapter/orchestration/generation.py new file mode 100644 index 00000000..4974444f --- /dev/null +++ b/tests/fixtures/architecture/orchestration_imports_inbound_adapter/orchestration/generation.py @@ -0,0 +1,5 @@ +"""Violating orchestration fixture.""" + +from tests.fixtures.architecture.orchestration_imports_inbound_adapter import api + +VALUE: str = api.VALUE diff --git a/tests/fixtures/architecture/orchestration_node_imports_outbound_adapter/__init__.py b/tests/fixtures/architecture/orchestration_node_imports_outbound_adapter/__init__.py new file mode 100644 index 00000000..5cbb2f8b --- /dev/null +++ b/tests/fixtures/architecture/orchestration_node_imports_outbound_adapter/__init__.py @@ -0,0 +1 @@ +"""Fixture package where a LangGraph node imports outbound storage.""" diff --git a/tests/fixtures/architecture/orchestration_node_imports_outbound_adapter/orchestration/__init__.py b/tests/fixtures/architecture/orchestration_node_imports_outbound_adapter/orchestration/__init__.py new file mode 100644 index 00000000..f714db9c --- /dev/null +++ b/tests/fixtures/architecture/orchestration_node_imports_outbound_adapter/orchestration/__init__.py @@ -0,0 +1 @@ +"""Orchestration package for a node boundary violation fixture.""" diff --git a/tests/fixtures/architecture/orchestration_node_imports_outbound_adapter/orchestration/_graph_nodes.py b/tests/fixtures/architecture/orchestration_node_imports_outbound_adapter/orchestration/_graph_nodes.py new file mode 100644 index 00000000..0dc89e0a --- /dev/null +++ b/tests/fixtures/architecture/orchestration_node_imports_outbound_adapter/orchestration/_graph_nodes.py @@ -0,0 +1,7 @@ +"""Violating LangGraph node fixture.""" + +from tests.fixtures.architecture.orchestration_node_imports_outbound_adapter import ( + storage, +) + +VALUE: str = storage.VALUE diff --git a/tests/fixtures/architecture/orchestration_node_imports_outbound_adapter/storage.py b/tests/fixtures/architecture/orchestration_node_imports_outbound_adapter/storage.py new file mode 100644 index 00000000..1986dd44 --- /dev/null +++ b/tests/fixtures/architecture/orchestration_node_imports_outbound_adapter/storage.py @@ -0,0 +1,3 @@ +"""Outbound storage fixture.""" + +VALUE = "storage" diff --git a/tests/fixtures/architecture/orchestration_node_imports_port/__init__.py b/tests/fixtures/architecture/orchestration_node_imports_port/__init__.py new file mode 100644 index 00000000..0a53784f --- /dev/null +++ b/tests/fixtures/architecture/orchestration_node_imports_port/__init__.py @@ -0,0 +1 @@ +"""Fixture package where a LangGraph node imports a port.""" diff --git a/tests/fixtures/architecture/orchestration_node_imports_port/domain.py b/tests/fixtures/architecture/orchestration_node_imports_port/domain.py new file mode 100644 index 00000000..4ed5b83f --- /dev/null +++ b/tests/fixtures/architecture/orchestration_node_imports_port/domain.py @@ -0,0 +1,3 @@ +"""Domain-port fixture.""" + +VALUE: str = "domain" diff --git a/tests/fixtures/architecture/orchestration_node_imports_port/orchestration/__init__.py b/tests/fixtures/architecture/orchestration_node_imports_port/orchestration/__init__.py new file mode 100644 index 00000000..9c1440ac --- /dev/null +++ b/tests/fixtures/architecture/orchestration_node_imports_port/orchestration/__init__.py @@ -0,0 +1 @@ +"""Orchestration package for an allowed node fixture.""" diff --git a/tests/fixtures/architecture/orchestration_node_imports_port/orchestration/_graph_nodes.py b/tests/fixtures/architecture/orchestration_node_imports_port/orchestration/_graph_nodes.py new file mode 100644 index 00000000..55e75832 --- /dev/null +++ b/tests/fixtures/architecture/orchestration_node_imports_port/orchestration/_graph_nodes.py @@ -0,0 +1,5 @@ +"""Allowed LangGraph node fixture.""" + +from tests.fixtures.architecture.orchestration_node_imports_port import domain + +VALUE: str = domain.VALUE diff --git a/tests/fixtures/architecture/ungrouped_adapter_is_caught/__init__.py b/tests/fixtures/architecture/ungrouped_adapter_is_caught/__init__.py new file mode 100644 index 00000000..f55e20e9 --- /dev/null +++ b/tests/fixtures/architecture/ungrouped_adapter_is_caught/__init__.py @@ -0,0 +1 @@ +"""Fixture package proving reachable adapters are grouped.""" diff --git a/tests/fixtures/architecture/ungrouped_adapter_is_caught/adapter.py b/tests/fixtures/architecture/ungrouped_adapter_is_caught/adapter.py new file mode 100644 index 00000000..54540ccb --- /dev/null +++ b/tests/fixtures/architecture/ungrouped_adapter_is_caught/adapter.py @@ -0,0 +1,3 @@ +"""Outbound adapter fixture that must not be invisible to Hecate.""" + +VALUE = "adapter" diff --git a/tests/fixtures/architecture/ungrouped_adapter_is_caught/orchestration/__init__.py b/tests/fixtures/architecture/ungrouped_adapter_is_caught/orchestration/__init__.py new file mode 100644 index 00000000..ceb34bd6 --- /dev/null +++ b/tests/fixtures/architecture/ungrouped_adapter_is_caught/orchestration/__init__.py @@ -0,0 +1 @@ +"""Orchestration package for grouped-adapter guard fixture.""" diff --git a/tests/fixtures/architecture/ungrouped_adapter_is_caught/orchestration/generation.py b/tests/fixtures/architecture/ungrouped_adapter_is_caught/orchestration/generation.py new file mode 100644 index 00000000..5191d1fb --- /dev/null +++ b/tests/fixtures/architecture/ungrouped_adapter_is_caught/orchestration/generation.py @@ -0,0 +1,5 @@ +"""Violating orchestration fixture.""" + +from tests.fixtures.architecture.ungrouped_adapter_is_caught import adapter + +VALUE: str = adapter.VALUE diff --git a/tests/steps/test_architecture_enforcement_steps.py b/tests/steps/test_architecture_enforcement_steps.py index c9061d64..ad349521 100644 --- a/tests/steps/test_architecture_enforcement_steps.py +++ b/tests/steps/test_architecture_enforcement_steps.py @@ -58,6 +58,38 @@ def test_composition_root_wiring_is_accepted() -> None: """Run the composition-root acceptance scenario.""" +@scenario( + "../features/architecture_enforcement.feature", + "A clean orchestration fixture passes", +) +def test_clean_orchestration_fixture_is_accepted() -> None: + """Run the orchestration acceptance scenario.""" + + +@scenario( + "../features/architecture_enforcement.feature", + "A LangGraph node importing an adapter is rejected", +) +def test_langgraph_node_adapter_violation_is_rejected() -> None: + """Run the LangGraph-node violation scenario.""" + + +@scenario( + "../features/architecture_enforcement.feature", + "A Celery task importing an adapter is rejected", +) +def test_celery_task_adapter_violation_is_rejected() -> None: + """Run the Celery-task violation scenario.""" + + +@scenario( + "../features/architecture_enforcement.feature", + "A checkpoint payload importing storage is rejected", +) +def test_checkpoint_payload_storage_violation_is_rejected() -> None: + """Run the checkpoint-payload violation scenario.""" + + @given(parsers.parse('the architecture fixture package "{package_name}"')) def architecture_fixture_package( context: ArchitectureContext, diff --git a/tests/test_architecture_enforcement.py b/tests/test_architecture_enforcement.py index 8e818765..337b3615 100644 --- a/tests/test_architecture_enforcement.py +++ b/tests/test_architecture_enforcement.py @@ -8,7 +8,10 @@ coverage lives in `tests/test_architecture_hecate_config.py`. """ +import json +import tomllib import typing as typ +from pathlib import Path import pytest from architecture_hecate_config import ( @@ -19,11 +22,15 @@ if typ.TYPE_CHECKING: import subprocess # noqa: S404 # Type-only CompletedProcess reference. - from pathlib import Path from syrupy.assertion import SnapshotAssertion +def _fixture_module(package_name: str, module_name: str) -> str: + """Return a fully qualified architecture fixture module name.""" + return f"tests.fixtures.architecture.{package_name}.{module_name}" + + @pytest.mark.parametrize( ("package_name", "expected_message_parts"), [ @@ -83,6 +90,86 @@ "tests.fixtures.architecture.explicit_empty_all.storage", ), ), + ( + "orchestration_node_imports_outbound_adapter", + ( + "ARCH001", + _fixture_module( + "orchestration_node_imports_outbound_adapter", + "orchestration._graph_nodes", + ), + _fixture_module( + "orchestration_node_imports_outbound_adapter", + "storage", + ), + ), + ), + ( + "orchestration_imports_inbound_adapter", + ( + "ARCH001", + _fixture_module( + "orchestration_imports_inbound_adapter", + "orchestration.generation", + ), + _fixture_module("orchestration_imports_inbound_adapter", "api"), + ), + ), + ( + "celery_task_imports_inbound_adapter", + ( + "ARCH001", + _fixture_module( + "celery_task_imports_inbound_adapter", + "worker.tasks", + ), + _fixture_module("celery_task_imports_inbound_adapter", "api"), + ), + ), + ( + "celery_task_imports_outbound_adapter", + ( + "ARCH001", + _fixture_module( + "celery_task_imports_outbound_adapter", + "worker.tasks", + ), + _fixture_module("celery_task_imports_outbound_adapter", "storage"), + ), + ), + ( + "checkpoint_payload_imports_storage", + ( + "ARCH001", + _fixture_module( + "checkpoint_payload_imports_storage", + "orchestration._checkpoint_payload", + ), + _fixture_module("checkpoint_payload_imports_storage", "storage"), + ), + ), + ( + "checkpoint_payload_imports_application", + ( + "ARCH001", + _fixture_module( + "checkpoint_payload_imports_application", + "orchestration._checkpoint_payload", + ), + _fixture_module("checkpoint_payload_imports_application", "service"), + ), + ), + ( + "ungrouped_adapter_is_caught", + ( + "ARCH001", + _fixture_module( + "ungrouped_adapter_is_caught", + "orchestration.generation", + ), + _fixture_module("ungrouped_adapter_is_caught", "adapter"), + ), + ), ], ) def test_checker_reports_fixture_boundary_violations( @@ -113,14 +200,36 @@ def test_checker_diagnostic_output_matches_snapshot( completed_process = run_hecate_fixture_check(package_name, config_path) - assert completed_process.returncode == 1, ( - "Hecate must reject the forbidden architecture fixture" - ) + assert completed_process.returncode == 1, _render_process(completed_process) assert _render_process(completed_process) == snapshot, ( "rendered Hecate diagnostics must match the recorded snapshot" ) +def test_orchestration_json_diagnostics_match_snapshot( + tmp_path: Path, + snapshot: SnapshotAssertion, +) -> None: + """Orchestration boundary JSON diagnostics keep a stable shape.""" + package_names = ( + "orchestration_node_imports_outbound_adapter", + "celery_task_imports_inbound_adapter", + "checkpoint_payload_imports_storage", + ) + reports: dict[str, object] = {} + for package_name in package_names: + config_path = write_fixture_config(tmp_path, package_name) + completed_process = run_hecate_fixture_check( + package_name, + config_path, + output_format="json", + ) + assert completed_process.returncode == 1, _render_process(completed_process) + reports[package_name] = _normalise_hecate_json_report(completed_process) + + assert reports == snapshot, "normalized JSON diagnostics must match the snapshot" + + def test_checker_accepts_allowed_fixture_graph(tmp_path: Path) -> None: """Allowed fixture imports do not produce architecture violations.""" package_name = "allowed_case" @@ -143,6 +252,43 @@ def test_checker_accepts_composition_root_fixture_wiring(tmp_path: Path) -> None assert completed_process.returncode == 0, rendered +@pytest.mark.parametrize( + "package_name", + [ + "orchestration_node_imports_port", + "orchestration_imports_domain_service", + "celery_task_imports_domain_service", + ], +) +def test_checker_accepts_orchestration_fixture_graphs( + package_name: str, + tmp_path: Path, +) -> None: + """Allowed orchestration fixture imports do not produce violations.""" + config_path = write_fixture_config(tmp_path, package_name) + + completed_process = run_hecate_fixture_check(package_name, config_path) + + rendered = f"{completed_process.stdout}\n{completed_process.stderr}" + assert completed_process.returncode == 0, rendered + + +def test_production_config_declares_orchestration_groups() -> None: + """Production Hecate config names the orchestration enforcement groups.""" + pyproject_path = Path(__file__).resolve().parents[1] / "pyproject.toml" + config = tomllib.loads(pyproject_path.read_text(encoding="utf-8")) + tool_config = typ.cast("dict[str, object]", config["tool"]) + hecate_config = typ.cast("dict[str, object]", tool_config["hecate"]) + groups = typ.cast("list[dict[str, object]]", hecate_config["groups"]) + group_names = {typ.cast("str", group["name"]) for group in groups} + + assert { + "orchestration", + "orchestration_tasks", + "orchestration_checkpoint", + } <= group_names, "production Hecate config must declare orchestration groups" + + def test_production_checker_accepts_scoped_packages() -> None: """The scoped production package graph follows the enforced boundaries.""" completed_process = run_hecate_production_check() @@ -154,3 +300,15 @@ def test_production_checker_accepts_scoped_packages() -> None: def _render_process(completed_process: subprocess.CompletedProcess[str]) -> str: """Return captured Hecate output in assertion form.""" return f"stdout:\n{completed_process.stdout}\nstderr:\n{completed_process.stderr}" + + +def _normalise_hecate_json_report( + completed_process: subprocess.CompletedProcess[str], +) -> object: + """Return Hecate JSON output with workspace-specific paths normalised.""" + report = json.loads(completed_process.stdout) + for violation in report["violations"]: + violation["source_path"] = str( + Path(violation["source_path"]).relative_to(Path(__file__).parents[1]) + ) + return report diff --git a/tests/test_architecture_hecate_config.py b/tests/test_architecture_hecate_config.py index a4b06d88..0b4606e5 100644 --- a/tests/test_architecture_hecate_config.py +++ b/tests/test_architecture_hecate_config.py @@ -21,13 +21,9 @@ write_fixture_config, ) -if typ.TYPE_CHECKING: - from pathlib import Path - - from syrupy.assertion import SnapshotAssertion - if typ.TYPE_CHECKING: import collections.abc as cabc + from pathlib import Path class _ErrorCase(typ.NamedTuple): @@ -45,9 +41,10 @@ def test_fixture_config_normal_fixture_excludes_package_barrel( config = _read_fixture_config(tmp_path, package_name) - assert _group_prefixes(config, "outbound_adapter") == [f"{package}.storage"], ( - "outbound-adapter prefix must identify the fixture storage package" - ) + assert _group_prefixes(config, "outbound_adapter") == [ + f"{package}.storage", + f"{package}.adapter", + ], "outbound-adapter prefixes must identify fixture adapter modules" def test_fixture_config_barrel_fixture_includes_package_barrel( @@ -60,6 +57,7 @@ def test_fixture_config_barrel_fixture_includes_package_barrel( assert _group_prefixes(config, "outbound_adapter") == [ f"{package}.storage", + f"{package}.adapter", package, ], "barrel fixture outbound prefixes must include storage and package roots" @@ -71,6 +69,15 @@ def test_fixture_config_writes_expected_toml_shape(tmp_path: Path) -> None: config = _read_fixture_config(tmp_path, package_name) + _assert_expected_base_groups(config, package) + _assert_expected_orchestration_groups(config, package) + + +def _assert_expected_base_groups( + config: dict[str, object], + package: str, +) -> None: + """Assert the generated base Hecate groups.""" hecate_config = _hecate_config(config) assert hecate_config["root_packages"] == [package], ( "Hecate root_packages must contain the fixture package" @@ -78,32 +85,96 @@ def test_fixture_config_writes_expected_toml_shape(tmp_path: Path) -> None: assert hecate_config["default_rule_id"] == "ARCH001", ( "Hecate default_rule_id must be ARCH001" ) + assert _group_names(config) == [ + "composition_root", + "domain", + "orchestration_checkpoint", + "orchestration_nodes", + "orchestration_tasks", + "orchestration", + "application", + "inbound_adapter", + "outbound_adapter", + ], "Hecate groups must retain first-match policy order" assert _group_prefixes(config, "composition_root") == [f"{package}.runtime"], ( - "composition_root prefix must identify the runtime module" + "composition_root must match the runtime module" ) assert _group_allowed(config, "composition_root") == [ "application", "composition_root", "domain", "inbound_adapter", + "orchestration", + "orchestration_checkpoint", + "orchestration_nodes", + "orchestration_tasks", "outbound_adapter", ], "composition_root must allow every configured architecture group" + assert _group_prefixes(config, "domain") == [ + f"{package}.domain", + f"{package}.worker.workloads", + ], "domain must include domain and workload modules" assert _group_allowed(config, "domain") == ["domain"], ( "domain group must allow only domain imports" ) assert _group_allowed(config, "application") == ["application", "domain"], ( - "application group must allow application and domain imports" + "application must allow application and domain imports" ) + assert _group_prefixes(config, "inbound_adapter") == [ + f"{package}.api", + f"{package}.worker.topology", + ], "inbound_adapter must match API and worker topology modules" assert _group_allowed(config, "inbound_adapter") == [ "inbound_adapter", "application", "domain", - ], "inbound_adapter group must allow inbound, application, and domain imports" + ], "inbound_adapter must allow inbound, application, and domain imports" assert _group_allowed(config, "outbound_adapter") == [ "outbound_adapter", "application", "domain", - ], "outbound_adapter group must allow outbound, application, and domain imports" + ], "outbound_adapter must allow outbound, application, and domain imports" + + +def _assert_expected_orchestration_groups( + config: dict[str, object], + package: str, +) -> None: + """Assert the generated orchestration Hecate groups.""" + assert _group_prefixes(config, "orchestration_checkpoint") == [ + f"{package}.orchestration._checkpoint_payload", + f"{package}.orchestration._checkpoint_dto", + f"{package}.orchestration._payload_dto", + ], "checkpoint group must match checkpoint payload modules" + assert _group_allowed(config, "orchestration_checkpoint") == [ + "orchestration_checkpoint", + "domain", + ], "checkpoint group must allow checkpoint and domain imports" + assert _group_prefixes(config, "orchestration_nodes") == [ + f"{package}.orchestration._graph_nodes", + ], "node group must match only the graph node module" + assert _group_allowed(config, "orchestration_nodes") == [ + "orchestration_nodes", + "domain", + "orchestration_checkpoint", + ], "node group must allow node, domain, and checkpoint imports" + assert _group_prefixes(config, "orchestration_tasks") == [ + f"{package}.worker.tasks", + ], "task group must match worker tasks" + assert _group_allowed(config, "orchestration_tasks") == [ + "orchestration_tasks", + "application", + "domain", + ], "task group must allow task, application, and domain imports" + assert _group_prefixes(config, "orchestration") == [ + f"{package}.orchestration", + ], "orchestration group must match the orchestration package" + assert _group_allowed(config, "orchestration") == [ + "orchestration", + "application", + "domain", + "orchestration_checkpoint", + ], "orchestration must allow its declared internal dependencies" @pytest.mark.parametrize( @@ -208,10 +279,11 @@ def raising_run( ) +@pytest.mark.parametrize("output_format", ["text", "json"]) def test_fixture_check_uses_injected_python_and_explicit_arguments( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, - snapshot: SnapshotAssertion, + output_format: typ.Literal["text", "json"], ) -> None: """Fixture checks construct an isolated Hecate command.""" config_path = write_fixture_config(tmp_path, "allowed_case") @@ -237,15 +309,26 @@ def capture_run( "allowed_case", config_path, python_executable="/custom/python", + output_format=output_format, ) - fixture_root = REPO_ROOT / "tests/fixtures/architecture/allowed_case" - assert captured_command[5] == str(config_path), "config path must match" - assert captured_command[9] == str(fixture_root), "fixture root must match" - stable_command = [*captured_command] - stable_command[5] = "" - stable_command[9] = "" - assert stable_command == snapshot, "fixture Hecate command must match its snapshot" + expected_command = [ + "/custom/python", + "-m", + "hecate", + "check", + "--config", + str(config_path), + "--package", + "tests.fixtures.architecture.allowed_case", + "--root", + str(REPO_ROOT / "tests/fixtures/architecture/allowed_case"), + "--format", + output_format, + ] + assert captured_command == expected_command, ( + "fixture Hecate command must include the requested output format" + ) def test_production_check_uses_injected_python( @@ -299,6 +382,13 @@ def _group_prefixes(config: dict[str, object], group_name: str) -> list[str]: raise AssertionError(group_name) +def _group_names(config: dict[str, object]) -> list[str]: + """Return generated Hecate group names in matching order.""" + hecate_config = _hecate_config(config) + groups = typ.cast("list[dict[str, object]]", hecate_config["groups"]) + return [typ.cast("str", group["name"]) for group in groups] + + def _group_allowed(config: dict[str, object], group_name: str) -> list[str]: """Return allowed dependency groups for one generated Hecate group.""" hecate_config = _hecate_config(config) diff --git a/tests/test_checkpoint_payload_boundaries.py b/tests/test_checkpoint_payload_boundaries.py new file mode 100644 index 00000000..e98fb29a --- /dev/null +++ b/tests/test_checkpoint_payload_boundaries.py @@ -0,0 +1,263 @@ +"""Boundary tests for durable orchestration checkpoint payload DTOs.""" + +import collections.abc as cabc +import dataclasses as dc +import datetime as dt +import enum +import json +import types +import typing as typ + +import pytest +from hypothesis import given +from hypothesis import strategies as st + +from episodic.llm import LLMProviderOperation, LLMUsage, ProviderCallUsage +from episodic.orchestration._checkpoint_dto import ( + WorkflowCheckpoint, + WorkflowStepIdentity, +) +from episodic.orchestration._payload_dto import ( + ActionExecutionResult, + ExecutionPlan, + PlannedAction, + PlannerResult, +) + +_CHECKPOINT_PAYLOAD_DTOS: tuple[type[object], ...] = ( + ActionExecutionResult, + ExecutionPlan, + PlannedAction, + PlannerResult, + WorkflowCheckpoint, + WorkflowStepIdentity, +) +_ALLOWED_LEAF_TYPES: tuple[type[object], ...] = ( + str, + int, + float, + bool, + dt.datetime, + LLMUsage, + ProviderCallUsage, + LLMProviderOperation, +) +_RUNTIME_VALIDATED_JSON_FIELDS: frozenset[tuple[type[object], str]] = frozenset({ + (WorkflowCheckpoint, "payload"), +}) +_NON_PERSISTED_ATTACHMENT_FIELDS: frozenset[tuple[type[object], str]] = frozenset({ + (ActionExecutionResult, "guest_bios_result"), + (ActionExecutionResult, "show_notes_result"), +}) +_JSON_SCALAR_STRATEGY: st.SearchStrategy[object] = st.one_of( + st.none(), + st.booleans(), + st.integers(), + st.floats(allow_nan=False, allow_infinity=False), + st.text(), +) +_JSON_VALUE_STRATEGY: st.SearchStrategy[object] = st.recursive( + _JSON_SCALAR_STRATEGY, + lambda children: st.one_of( + st.lists(children, max_size=4), + st.dictionaries(st.text(), children, max_size=4), + ), + max_leaves=12, +) +type _OriginValidator = cabc.Callable[[tuple[object, ...]], bool] + + +def test_checkpoint_payload_dtos_use_provider_neutral_field_types() -> None: + """Checkpoint payload DTO fields stay within provider-neutral types.""" + rejected_fields: list[str] = [] + for dto_type in _CHECKPOINT_PAYLOAD_DTOS: + type_hints = typ.get_type_hints(dto_type) + for field in dc.fields(dto_type): + if (dto_type, field.name) in ( + _RUNTIME_VALIDATED_JSON_FIELDS | _NON_PERSISTED_ATTACHMENT_FIELDS + ): + continue + field_type = type_hints[field.name] + if not _is_provider_neutral_type(field_type): + rejected_fields.append( + f"{dto_type.__module__}.{dto_type.__qualname__}.{field.name}: " + f"{field_type!r}" + ) + + assert not rejected_fields, f"provider-specific DTO fields found: {rejected_fields}" + + +def test_workflow_checkpoint_rejects_non_json_payload_values() -> None: + """WorkflowCheckpoint rejects payload values that cannot be serialised.""" + with pytest.raises( + TypeError, + match="payload must be JSON-serializable", + ): + WorkflowCheckpoint( + checkpoint_id="checkpoint-1", + workflow_id="workflow-1", + workflow_type="generation_orchestration", + step_name="execute", + idempotency_key="workflow-1:generation_orchestration:execute:action-1:0", + payload={"bad": object()}, + ) + + +@pytest.mark.parametrize( + "payload", + [ + pytest.param({"value": (1, 2)}, id="tuple-value"), + pytest.param({1: "one"}, id="non-string-key"), + ], +) +def test_workflow_checkpoint_rejects_lossy_json_payloads( + payload: dict[object, object], +) -> None: + """WorkflowCheckpoint rejects payloads changed by a JSON round trip.""" + with pytest.raises( + TypeError, + match="payload must be JSON-serializable without data loss", + ): + WorkflowCheckpoint( + checkpoint_id="checkpoint-1", + workflow_id="workflow-1", + workflow_type="generation_orchestration", + step_name="execute", + idempotency_key="workflow-1:generation_orchestration:execute:action-1:0", + payload=typ.cast("dict[str, object]", payload), + ) + + +@given(payload=_JSON_VALUE_STRATEGY) +def test_workflow_checkpoint_payload_round_trips_through_json(payload: object) -> None: + """Any valid checkpoint payload value survives JSON serialisation unchanged.""" + checkpoint = WorkflowCheckpoint( + checkpoint_id="checkpoint-1", + workflow_id="workflow-1", + workflow_type="generation_orchestration", + step_name="execute", + idempotency_key="workflow-1:generation_orchestration:execute:action-1:0", + payload={"value": payload}, + ) + + encoded = json.dumps(checkpoint.payload, sort_keys=True) + + assert json.loads(encoded) == checkpoint.payload, ( + "checkpoint payload must survive JSON round-tripping" + ) + + +@pytest.mark.parametrize( + ("origin", "arguments", "expected"), + [ + pytest.param(typ.Union, (str, int), True, id="typing-union-valid"), + pytest.param( + types.UnionType, + (str, object), + False, + id="union-type-invalid", + ), + pytest.param( + typ.Literal, + ("execute", 1, None), + True, + id="literal", + ), + pytest.param(tuple, (str, int), True, id="fixed-tuple"), + pytest.param(tuple, (str, Ellipsis), True, id="variadic-tuple"), + pytest.param(list, (str,), True, id="list"), + pytest.param(cabc.Sequence, (str,), True, id="sequence"), + pytest.param(dict, (str, int), True, id="dict"), + pytest.param( + cabc.Mapping, + (int, str), + False, + id="mapping-non-string-key", + ), + pytest.param(set, (str,), False, id="unsupported-generic-origin"), + ], +) +def test_provider_neutral_origin_dispatch( + origin: object, + arguments: tuple[object, ...], + *, + expected: bool, +) -> None: + """Generic annotation origins dispatch to the expected validator.""" + assert _is_provider_neutral_origin(origin, arguments) is expected, ( + f"unexpected provider-neutral result for {origin!r} with {arguments!r}" + ) + + +def _is_provider_neutral_type(field_type: object) -> bool: + """Return whether a DTO field type can cross the checkpoint boundary.""" + if field_type is None or field_type is types.NoneType: + return True + if isinstance(field_type, type): + return _is_provider_neutral_leaf_type(field_type) + origin = typ.get_origin(field_type) + arguments = typ.get_args(field_type) + return _is_provider_neutral_origin(origin, arguments) + + +def _is_provider_neutral_leaf_type(field_type: type[object]) -> bool: + """Return whether a concrete type is allowed in checkpoint DTO fields.""" + return ( + field_type in _ALLOWED_LEAF_TYPES + or issubclass(field_type, enum.Enum) + or field_type in _CHECKPOINT_PAYLOAD_DTOS + ) + + +def _are_provider_neutral_types(arguments: tuple[object, ...]) -> bool: + """Return whether all annotation arguments are checkpoint-neutral.""" + return all(_is_provider_neutral_type(argument) for argument in arguments) + + +def _is_provider_neutral_mapping(arguments: tuple[object, ...]) -> bool: + """Return whether mapping arguments have string keys and neutral values.""" + key_type, value_type = arguments + return key_type is str and _is_provider_neutral_type(value_type) + + +def _is_provider_neutral_tuple(arguments: tuple[object, ...]) -> bool: + """Return whether tuple annotation arguments are checkpoint-neutral.""" + if arguments[-1:] == (Ellipsis,): + return _is_provider_neutral_type(arguments[0]) + return all(_is_provider_neutral_type(argument) for argument in arguments) + + +def _is_provider_neutral_literal(value: object) -> bool: + """Return whether a literal annotation value is provider-neutral.""" + return value is None or isinstance(value, (str, int, float, bool, enum.Enum)) + + +def _are_provider_neutral_literals(arguments: tuple[object, ...]) -> bool: + """Return whether all literal arguments are provider-neutral.""" + return all(_is_provider_neutral_literal(argument) for argument in arguments) + + +def _reject_unsupported_origin(_arguments: tuple[object, ...]) -> bool: + """Reject a generic annotation origin without a supported validator.""" + return False + + +_ORIGIN_VALIDATORS: dict[object, _OriginValidator] = { + typ.Union: _are_provider_neutral_types, + types.UnionType: _are_provider_neutral_types, + typ.Literal: _are_provider_neutral_literals, + tuple: _is_provider_neutral_tuple, + list: _are_provider_neutral_types, + cabc.Sequence: _are_provider_neutral_types, + dict: _is_provider_neutral_mapping, + cabc.Mapping: _is_provider_neutral_mapping, +} + + +def _is_provider_neutral_origin( + origin: object, + arguments: tuple[object, ...], +) -> bool: + """Return whether a generic annotation origin is checkpoint-neutral.""" + validator = _ORIGIN_VALIDATORS.get(origin, _reject_unsupported_origin) + return validator(arguments) diff --git a/tests/test_generation_orchestration_langgraph_vidaimock.py b/tests/test_generation_orchestration_langgraph_vidaimock.py new file mode 100644 index 00000000..f2f6b025 --- /dev/null +++ b/tests/test_generation_orchestration_langgraph_vidaimock.py @@ -0,0 +1,141 @@ +"""Vidai Mock behavioural coverage for the generation LangGraph path.""" + +import contextlib +import dataclasses as dc +import subprocess # noqa: S404 - required to manage the local Vidai Mock process +import typing as typ + +import pytest + +from episodic.llm.openai_adapter import ( + OpenAICompatibleLLMAdapter, + OpenAICompatibleLLMConfig, +) +from episodic.orchestration import ( + ActionKind, + GenerationGraphState, + GenerationOrchestrationConfig, + GenerationOrchestrationRequest, + ShowNotesToolExecutor, + StructuredGenerationPlanner, + build_generation_orchestration_graph, +) +from tests.steps.generation_orchestration_vidaimock import ( + find_free_port, + start_vidaimock_process, + write_provider_config, + write_response_template, +) + +if typ.TYPE_CHECKING: + import collections.abc as cabc + from pathlib import Path + + from episodic.llm.ports import LLMPort, LLMRequest, LLMResponse + + +@dc.dataclass(slots=True) +class _VidaiContext: + """Runtime state for one Vidai Mock-backed graph test.""" + + process: subprocess.Popen[str] | None = None + base_url: str = "" + + def stop(self) -> None: + """Terminate the local Vidai Mock process if it was started.""" + if self.process is None: + return + self.process.terminate() + try: + self.process.wait(timeout=5) + except subprocess.TimeoutExpired: + self.process.kill() + self.process.wait(timeout=5) + + +@dc.dataclass(slots=True) +class _RecordingLLMPort: + """Capture the actual `LLMRequest` values before delegating.""" + + wrapped: LLMPort + requests: list[LLMRequest] = dc.field(default_factory=list) + + async def generate(self, request: LLMRequest) -> LLMResponse: + """Record and forward the request.""" + self.requests.append(request) + return await self.wrapped.generate(request) + + +@pytest.fixture +def vidaimock_context(tmp_path: Path) -> cabc.Iterator[_VidaiContext]: + """Start the orchestration Vidai Mock server and stop it after the test.""" + provider_dir = tmp_path / "providers" + template_dir = tmp_path / "templates" / "orchestration" + provider_dir.mkdir(parents=True) + template_dir.mkdir(parents=True) + + write_provider_config(provider_dir) + write_response_template(template_dir) + context = _VidaiContext() + with contextlib.ExitStack() as stack: + stack.callback(context.stop) + start_vidaimock_process(context, tmp_path, port=find_free_port()) + yield context + + +@pytest.mark.asyncio +async def test_langgraph_plans_executes_and_finishes_with_vidai_mock( + vidaimock_context: _VidaiContext, +) -> None: + """The direct graph path completes against a Vidai Mock-backed LLM port.""" + request = GenerationOrchestrationRequest( + correlation_id="vidai-graph", + script_tei_xml=( + "

Welcome to episode 42.

" + "

We discuss the main topic.

" + ), + template_structure={"sections": ["intro", "discussion"]}, + ) + + async with OpenAICompatibleLLMAdapter( + config=OpenAICompatibleLLMConfig( + base_url=vidaimock_context.base_url, + api_key="test-key", + ), + ) as adapter: + recording_port = _RecordingLLMPort(wrapped=adapter) + config = GenerationOrchestrationConfig( + planning_model="gpt-4.1", + execution_model="gpt-4o-mini", + ) + graph = build_generation_orchestration_graph( + planner=StructuredGenerationPlanner( + llm=recording_port, + config=config, + ), + tool_executor=ShowNotesToolExecutor( + llm=recording_port, + config=config, + ), + ) + + state = await graph.ainvoke(GenerationGraphState(request=request)) + + result = state["orchestration_result"] + assert result.plan.steps[0].action_kind is ActionKind.GENERATE_SHOW_NOTES, ( + "planner must select show-note generation" + ) + assert result.action_results[0].show_notes_result is not None, ( + "show-note execution must return a result" + ) + topic = result.action_results[0].show_notes_result.entries[0].topic + assert topic == "Introduction", ( + "Vidai Mock response must populate the expected topic" + ) + assert result.total_usage.total_tokens == 81, ( + "planner and action usage must aggregate" + ) + assert [call.model for call in recording_port.requests] == [ + "gpt-4.1", + "gpt-4o-mini", + ], "orchestration must call the planning and execution models in order" diff --git a/tests/test_structured_logging.py b/tests/test_structured_logging.py new file mode 100644 index 00000000..19d14b77 --- /dev/null +++ b/tests/test_structured_logging.py @@ -0,0 +1,61 @@ +"""Tests for structured event logging.""" + +import datetime as dt +import enum +import json +import typing as typ +import uuid + +from episodic import logging as episodic_logging + +if typ.TYPE_CHECKING: + import pytest + + +class _EventSpyLogger: + """Collect structured messages emitted by `log_event`.""" + + def __init__(self) -> None: + """Initialize an empty message record.""" + self.messages: list[str] = [] + + def info(self, message: str, **kwargs: object) -> None: + """Record one INFO message and verify no logger kwargs were added.""" + assert not kwargs, "structured event fields must be encoded in the message" + self.messages.append(message) + + +class _EventState(enum.Enum): + """Representative non-string enum used in structured fields.""" + + READY = "ready" + + +def test_log_event_normalizes_non_json_structured_fields( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Structured logging normalizes common non-JSON field values.""" + logger = _EventSpyLogger() + monkeypatch.setattr(episodic_logging, "_event_log", logger) + event_id = uuid.UUID("12345678-1234-5678-1234-567812345678") + occurred_at = dt.datetime(2026, 8, 3, 12, 30, tzinfo=dt.UTC) + + episodic_logging.log_event( + "info", + "generation.failed", + event_id=event_id, + occurred_at=occurred_at, + state=_EventState.READY, + error=RuntimeError("provider unavailable"), + ) + + expected_payload = { + "error": "provider unavailable", + "event": "generation.failed", + "event_id": str(event_id), + "occurred_at": "2026-08-03T12:30:00+00:00", + "state": "ready", + } + assert json.loads(logger.messages[0]) == expected_payload, ( + "non-JSON fields must normalize to the expected event payload" + )