From 383ace3b400458d5ec0203ec250d4a627ff1add2 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 15 Jun 2026 23:01:26 +0200 Subject: [PATCH 01/24] Add execplan for orchestration architecture enforcement (2.4.5) Draft the ExecPlan for roadmap item 2.4.5, which extends Hecate architecture enforcement to LangGraph orchestration code, Celery tasks, and checkpoint payloads. The plan reconciles the roadmap's "ports only" wording with the system design's "domain services and ports only", and proposes three new first-match Hecate groups (orchestration, orchestration_tasks, orchestration_checkpoint) plus two boundary fixes surfaced during research: extracting WorkloadClass out of the kombu-coupled worker topology module, and decoupling checkpoint DTOs from the application-tier generation DTO barrel. It also resolves the existing 400-line limit breach in langgraph.py via a node/builder split. Enforcement is validated through synthetic architecture fixtures (positive and negative), a structural reflection test and a Hypothesis property test for checkpoint payloads, a syrupy snapshot of Hecate output, a pytest-bdd feature, and a vidai-mock-backed behavioural test of the generation graph. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...cture-enforcement-to-orchestration-code.md | 770 ++++++++++++++++++ 1 file changed, 770 insertions(+) create mode 100644 docs/execplans/2-4-5-extend-architecture-enforcement-to-orchestration-code.md 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..63d08179 --- /dev/null +++ b/docs/execplans/2-4-5-extend-architecture-enforcement-to-orchestration-code.md @@ -0,0 +1,770 @@ +# 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: DRAFT + +## 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 mis-classify 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 mis-ordering. + +- 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 + +- [ ] M0 Orientation and red harness (fixtures and failing tests, no + production changes). +- [ ] M1 Dedicated `orchestration` Hecate group and node/builder split. +- [ ] M2 Celery task enforcement and `WorkloadClass` extraction. +- [ ] M3 Checkpoint payload boundary audit (Hecate group plus structural and + property tests). +- [ ] M4 Behavioural tests, snapshots, documentation, and roadmap update. + +## 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). + +## 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. + +## Outcomes & retrospective + +To be completed at milestone boundaries and at completion. Compare the result +against the Purpose: a developer importing an adapter into a node, task, or +checkpoint payload must see `make lint` fail, and a non-neutral checkpoint +payload field must fail `make test`. + +## 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 normalisation 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 proceeds in five milestones. Each follows 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 and +re-run 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 normalisation 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 serialisation unchanged (proving payloads stay + JSON-shaped and free of non-serialisable 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 + +Goal: cover externally observable behaviour, lock output format, and document +the new boundaries. + +1. Add a `pytest-bdd` feature that specifies the enforcement workflow from a + maintainer's perspective. Embed the feature in this plan (see "BDD feature" + below) and place it under the project's feature directory (locate with + `leta files tests/` or the existing `*.feature` convention). Steps drive the + fixture harness: a clean orchestration fixture passes; a node importing an + adapter is rejected with an `ARCH001` violation naming the node module; a + Celery task importing an adapter is rejected; a checkpoint payload importing + storage is rejected. +2. Add a `syrupy` snapshot test capturing the `hecate check --format json` + output for one representative orchestration-violation fixture, so the + violation message shape for node, task, and checkpoint rules is regression + protected. Only snapshot fixture output (deterministic), never the + production tree. +3. Add a behavioural test that exercises `build_generation_orchestration_graph` + end to end against a simulated inference service using `vidai-mock` + (per the `vidai-mock` skill), asserting the graph still plans, executes, and + finishes after the node/builder split, with the mock standing in for the + `LLMPort` adapter. This proves the refactor preserved observable behaviour. +4. Documentation: + - Update `docs/episodic-podcast-generation-system-design.md` to mark the + orchestration enforcement slice as delivered and to describe the three new + groups and the checkpoint audit. + - Update `docs/developers-guide.md` "Architecture enforcement" with the new + groups, the first-match ordering for the new prefixes, the + `WorkloadClass` location, and how to add fixtures for orchestration + boundaries. + - Update `docs/langgraph-and-celery-in-hexagonal-architecture.md` (the + orchestration component architecture doc) with the node/builder split, the + ports-only node rule, the task rule, and the checkpoint payload rule. + - Add ADR-016 (verify the next free number with `ls docs/adr/`; ADR-015 is + already used) recording the orchestration-enforcement decisions (the + "ports only" interpretation, the node/builder split, and the + checkpoint audit mechanism), and cross-reference it from + `docs/adr/adr-014-hexagonal-architecture-enforcement.md` and the system + design. Follow the documentation style guide and the + `arch-decision-records` conventions. + - Update `docs/users-guide.md` only if a publicly consumable interface + changed. This slice is internal enforcement; if no public API changes, note + in the Decision Log that no users-guide change was required. + - Add the new ADR and any new component note to `docs/contents.md`. +5. Mark roadmap item 2.4.5 done in `docs/roadmap.md`. + +Validation: `make check-fmt`, `make typecheck`, `make lint`, `make test`, +`make markdownlint`, and `make nixie` all pass. The BDD scenarios fail before +their enforcement exists and pass after. + +## 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). Keep the specification synchronized with M4. + +```gherkin +Feature: Orchestration architecture enforcement + As a maintainer of Episodic + I want Hecate to reject adapter imports from orchestration code + So that LangGraph nodes, Celery tasks, and checkpoint payloads stay + behind ports + + Scenario: A clean orchestration fixture passes + Given the "orchestration_node_imports_port" fixture + When I run the architecture check on the fixture + Then the check passes with no violations + + Scenario: A LangGraph node importing an adapter is rejected + Given the "orchestration_node_imports_outbound_adapter" fixture + When I run the architecture check on the fixture + Then the check fails with an ARCH001 violation + And the violation names the node module as the importer + + Scenario: A Celery task importing an adapter is rejected + Given the "celery_task_imports_inbound_adapter" fixture + When I run the architecture check on the fixture + Then the check fails with an ARCH001 violation + + Scenario: A checkpoint payload importing storage is rejected + Given the "checkpoint_payload_imports_storage" fixture + When I run the architecture check on the fixture + Then the check fails with an ARCH001 violation +``` + +## 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 normalisation 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. From 89385e7f5f58957c39a1e44e174dc28bb7fc52ef Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 26 Jun 2026 19:37:30 +0200 Subject: [PATCH 02/24] Record orchestration enforcement plan start Mark the 2.4.5 ExecPlan as in progress and record the clean rebase, post-rebase validation, PR title update, and active Lody session reference. --- ...tend-architecture-enforcement-to-orchestration-code.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 index 63d08179..37b74554 100644 --- 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 @@ -5,7 +5,7 @@ This ExecPlan (execution plan) is a living document. The sections `Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work proceeds. -Status: DRAFT +Status: IN PROGRESS ## Purpose / big picture @@ -142,6 +142,12 @@ Adjust per milestone; stop and escalate when a threshold is breached. property tests). - [ ] 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. + ## Surprises & discoveries - Observation: Hecate counts imports inside `if TYPE_CHECKING:` blocks and From 4b9dc24942c0fc04958cd398511eddae180af55f Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 26 Jun 2026 19:46:14 +0200 Subject: [PATCH 03/24] Add orchestration architecture fixture harness Extend the synthetic Hecate fixture config with orchestration node, task, and checkpoint groups so upcoming production rules have red harness coverage before the real `pyproject.toml` policy changes. Add positive and negative fixture packages for LangGraph nodes, Celery tasks, checkpoint payloads, and grouped adapter guard cases. Record M0 completion and validation evidence in the ExecPlan. --- ...cture-enforcement-to-orchestration-code.md | 23 +++- tests/architecture_hecate_config.py | 62 ++++++++- .../__init__.py | 1 + .../domain.py | 3 + .../service.py | 5 + .../worker/__init__.py | 1 + .../worker/tasks.py | 8 ++ .../worker/workloads.py | 3 + .../__init__.py | 1 + .../api.py | 3 + .../worker/__init__.py | 1 + .../worker/tasks.py | 5 + .../__init__.py | 1 + .../storage.py | 3 + .../worker/__init__.py | 1 + .../worker/tasks.py | 5 + .../__init__.py | 1 + .../domain.py | 3 + .../orchestration/__init__.py | 1 + .../orchestration/_checkpoint_payload.py | 5 + .../service.py | 5 + .../__init__.py | 1 + .../orchestration/__init__.py | 1 + .../orchestration/_checkpoint_payload.py | 5 + .../storage.py | 3 + .../__init__.py | 1 + .../domain.py | 3 + .../orchestration/__init__.py | 1 + .../orchestration/generation.py | 5 + .../service.py | 5 + .../__init__.py | 1 + .../api.py | 3 + .../orchestration/__init__.py | 1 + .../orchestration/generation.py | 5 + .../__init__.py | 1 + .../orchestration/__init__.py | 1 + .../orchestration/_graph_nodes.py | 7 + .../storage.py | 3 + .../__init__.py | 1 + .../orchestration_node_imports_port/domain.py | 3 + .../orchestration/__init__.py | 1 + .../orchestration/_graph_nodes.py | 5 + .../ungrouped_adapter_is_caught/__init__.py | 1 + .../ungrouped_adapter_is_caught/adapter.py | 3 + .../orchestration/__init__.py | 1 + .../orchestration/generation.py | 5 + tests/test_architecture_enforcement.py | 126 +++++++++++++++++- tests/test_architecture_hecate_config.py | 76 ++++++++++- 48 files changed, 398 insertions(+), 12 deletions(-) create mode 100644 tests/fixtures/architecture/celery_task_imports_domain_service/__init__.py create mode 100644 tests/fixtures/architecture/celery_task_imports_domain_service/domain.py create mode 100644 tests/fixtures/architecture/celery_task_imports_domain_service/service.py create mode 100644 tests/fixtures/architecture/celery_task_imports_domain_service/worker/__init__.py create mode 100644 tests/fixtures/architecture/celery_task_imports_domain_service/worker/tasks.py create mode 100644 tests/fixtures/architecture/celery_task_imports_domain_service/worker/workloads.py create mode 100644 tests/fixtures/architecture/celery_task_imports_inbound_adapter/__init__.py create mode 100644 tests/fixtures/architecture/celery_task_imports_inbound_adapter/api.py create mode 100644 tests/fixtures/architecture/celery_task_imports_inbound_adapter/worker/__init__.py create mode 100644 tests/fixtures/architecture/celery_task_imports_inbound_adapter/worker/tasks.py create mode 100644 tests/fixtures/architecture/celery_task_imports_outbound_adapter/__init__.py create mode 100644 tests/fixtures/architecture/celery_task_imports_outbound_adapter/storage.py create mode 100644 tests/fixtures/architecture/celery_task_imports_outbound_adapter/worker/__init__.py create mode 100644 tests/fixtures/architecture/celery_task_imports_outbound_adapter/worker/tasks.py create mode 100644 tests/fixtures/architecture/checkpoint_payload_imports_application/__init__.py create mode 100644 tests/fixtures/architecture/checkpoint_payload_imports_application/domain.py create mode 100644 tests/fixtures/architecture/checkpoint_payload_imports_application/orchestration/__init__.py create mode 100644 tests/fixtures/architecture/checkpoint_payload_imports_application/orchestration/_checkpoint_payload.py create mode 100644 tests/fixtures/architecture/checkpoint_payload_imports_application/service.py create mode 100644 tests/fixtures/architecture/checkpoint_payload_imports_storage/__init__.py create mode 100644 tests/fixtures/architecture/checkpoint_payload_imports_storage/orchestration/__init__.py create mode 100644 tests/fixtures/architecture/checkpoint_payload_imports_storage/orchestration/_checkpoint_payload.py create mode 100644 tests/fixtures/architecture/checkpoint_payload_imports_storage/storage.py create mode 100644 tests/fixtures/architecture/orchestration_imports_domain_service/__init__.py create mode 100644 tests/fixtures/architecture/orchestration_imports_domain_service/domain.py create mode 100644 tests/fixtures/architecture/orchestration_imports_domain_service/orchestration/__init__.py create mode 100644 tests/fixtures/architecture/orchestration_imports_domain_service/orchestration/generation.py create mode 100644 tests/fixtures/architecture/orchestration_imports_domain_service/service.py create mode 100644 tests/fixtures/architecture/orchestration_imports_inbound_adapter/__init__.py create mode 100644 tests/fixtures/architecture/orchestration_imports_inbound_adapter/api.py create mode 100644 tests/fixtures/architecture/orchestration_imports_inbound_adapter/orchestration/__init__.py create mode 100644 tests/fixtures/architecture/orchestration_imports_inbound_adapter/orchestration/generation.py create mode 100644 tests/fixtures/architecture/orchestration_node_imports_outbound_adapter/__init__.py create mode 100644 tests/fixtures/architecture/orchestration_node_imports_outbound_adapter/orchestration/__init__.py create mode 100644 tests/fixtures/architecture/orchestration_node_imports_outbound_adapter/orchestration/_graph_nodes.py create mode 100644 tests/fixtures/architecture/orchestration_node_imports_outbound_adapter/storage.py create mode 100644 tests/fixtures/architecture/orchestration_node_imports_port/__init__.py create mode 100644 tests/fixtures/architecture/orchestration_node_imports_port/domain.py create mode 100644 tests/fixtures/architecture/orchestration_node_imports_port/orchestration/__init__.py create mode 100644 tests/fixtures/architecture/orchestration_node_imports_port/orchestration/_graph_nodes.py create mode 100644 tests/fixtures/architecture/ungrouped_adapter_is_caught/__init__.py create mode 100644 tests/fixtures/architecture/ungrouped_adapter_is_caught/adapter.py create mode 100644 tests/fixtures/architecture/ungrouped_adapter_is_caught/orchestration/__init__.py create mode 100644 tests/fixtures/architecture/ungrouped_adapter_is_caught/orchestration/generation.py 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 index 37b74554..d8e7c650 100644 --- 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 @@ -134,7 +134,7 @@ Adjust per milestone; stop and escalate when a threshold is breached. ## Progress -- [ ] M0 Orientation and red harness (fixtures and failing tests, no +- [x] M0 Orientation and red harness (fixtures and failing tests, no production changes). - [ ] M1 Dedicated `orchestration` Hecate group and node/builder split. - [ ] M2 Celery task enforcement and `WorkloadClass` extraction. @@ -148,6 +148,12 @@ conflicts. Post-rebase gates passed: `make check-fmt`, `make test`, 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`). + ## Surprises & discoveries - Observation: Hecate counts imports inside `if TYPE_CHECKING:` blocks and @@ -176,6 +182,14 @@ point at the active Lody session. 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. + ## Decision log - Decision: interpret "depend on ports only" as "depend on the application and @@ -213,6 +227,13 @@ point at the active Lody session. 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. + ## Outcomes & retrospective To be completed at milestone boundaries and at completion. Compare the result diff --git a/tests/architecture_hecate_config.py b/tests/architecture_hecate_config.py index e305a99e..3fb2d381 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", @@ -208,13 +232,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 +260,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 +294,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/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..85e79576 --- /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 = "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..91edd5be --- /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 = 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..af0cf73e --- /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 = (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..598071ae --- /dev/null +++ b/tests/fixtures/architecture/celery_task_imports_domain_service/worker/workloads.py @@ -0,0 +1,3 @@ +"""Domain-port workload contract fixture.""" + +VALUE = "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..76d5c0cc --- /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 = 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..5592e798 --- /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 = 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..f0937fc8 --- /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 = 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..adf1776f --- /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 = 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..8f9795a1 --- /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 = 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..dda2f11c --- /dev/null +++ b/tests/fixtures/architecture/orchestration_imports_domain_service/domain.py @@ -0,0 +1,3 @@ +"""Domain fixture for an orchestration dependency.""" + +VALUE = "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..1d0692f5 --- /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 = 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..0a13c6d0 --- /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 = 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..466d744c --- /dev/null +++ b/tests/fixtures/architecture/orchestration_imports_inbound_adapter/api.py @@ -0,0 +1,3 @@ +"""Inbound-adapter fixture.""" + +VALUE = "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..bbae6b8d --- /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 = 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..4feaf4c4 --- /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 = 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..8850f052 --- /dev/null +++ b/tests/fixtures/architecture/orchestration_node_imports_port/domain.py @@ -0,0 +1,3 @@ +"""Domain-port fixture.""" + +VALUE = "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..dbef69f0 --- /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 = 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..44e4135d --- /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 = adapter.VALUE diff --git a/tests/test_architecture_enforcement.py b/tests/test_architecture_enforcement.py index 8e818765..fd49ed67 100644 --- a/tests/test_architecture_enforcement.py +++ b/tests/test_architecture_enforcement.py @@ -8,7 +8,9 @@ coverage lives in `tests/test_architecture_hecate_config.py`. """ +import tomllib import typing as typ +from pathlib import Path import pytest from architecture_hecate_config import ( @@ -19,11 +21,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 +89,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( @@ -143,6 +229,44 @@ 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 + + +@pytest.mark.xfail(strict=True, reason="groups added in M1-M3") +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 + + def test_production_checker_accepts_scoped_packages() -> None: """The scoped production package graph follows the enforced boundaries.""" completed_process = run_hecate_production_check() diff --git a/tests/test_architecture_hecate_config.py b/tests/test_architecture_hecate_config.py index a4b06d88..329438c7 100644 --- a/tests/test_architecture_hecate_config.py +++ b/tests/test_architecture_hecate_config.py @@ -45,9 +45,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 +61,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" @@ -78,22 +80,77 @@ 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_prefixes(config, "composition_root") == [f"{package}.runtime"], ( - "composition_root prefix must identify the runtime module" - ) + 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"] 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", + ] assert _group_allowed(config, "domain") == ["domain"], ( "domain group must allow only domain imports" ) + assert _group_prefixes(config, "orchestration_checkpoint") == [ + f"{package}.orchestration._checkpoint_payload", + f"{package}.orchestration._checkpoint_dto", + f"{package}.orchestration._payload_dto", + ] + assert _group_allowed(config, "orchestration_checkpoint") == [ + "orchestration_checkpoint", + "domain", + ] + assert _group_prefixes(config, "orchestration_nodes") == [ + f"{package}.orchestration._graph_nodes", + ] + assert _group_allowed(config, "orchestration_nodes") == [ + "orchestration_nodes", + "domain", + "orchestration_checkpoint", + ] + assert _group_prefixes(config, "orchestration_tasks") == [ + f"{package}.worker.tasks", + ] + assert _group_allowed(config, "orchestration_tasks") == [ + "orchestration_tasks", + "application", + "domain", + ] + assert _group_prefixes(config, "orchestration") == [ + f"{package}.orchestration", + ] + assert _group_allowed(config, "orchestration") == [ + "orchestration", + "application", + "domain", + "orchestration_checkpoint", + ] assert _group_allowed(config, "application") == ["application", "domain"], ( "application group must allow application and domain imports" ) + assert _group_prefixes(config, "inbound_adapter") == [ + f"{package}.api", + f"{package}.worker.topology", + ] assert _group_allowed(config, "inbound_adapter") == [ "inbound_adapter", "application", @@ -299,6 +356,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) From 7e9ad80c855ecbcb357e80f6f1653a7a4d406927 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 26 Jun 2026 20:46:13 +0200 Subject: [PATCH 04/24] Record M0 review completion Update the 2.4.5 ExecPlan with the CodeRabbit result for the M0 fixture-harness milestone so the plan remains resumable. --- ...-4-5-extend-architecture-enforcement-to-orchestration-code.md | 1 + 1 file changed, 1 insertion(+) 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 index d8e7c650..7579bfc1 100644 --- 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 @@ -153,6 +153,7 @@ 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. ## Surprises & discoveries From 5732f8cd04c8e780e87e2be605d7162ea3de8a5f Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 26 Jun 2026 20:58:33 +0200 Subject: [PATCH 05/24] Split orchestration graph modules Move LangGraph node functions and graph assembly into focused modules so architecture enforcement can target graph orchestration separately. --- ...cture-enforcement-to-orchestration-code.md | 58 ++- .../canonical/adapters/generation_runs.py | 2 +- episodic/logging.py | 28 ++ episodic/orchestration/_graph_builder.py | 256 ++++++++++ episodic/orchestration/_graph_nodes.py | 186 +++++++ episodic/orchestration/_types.py | 32 +- episodic/orchestration/langgraph.py | 464 +----------------- 7 files changed, 552 insertions(+), 474 deletions(-) create mode 100644 episodic/orchestration/_graph_builder.py create mode 100644 episodic/orchestration/_graph_nodes.py 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 index 7579bfc1..ddd193df 100644 --- 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 @@ -136,7 +136,7 @@ Adjust per milestone; stop and escalate when a threshold is breached. - [x] M0 Orientation and red harness (fixtures and failing tests, no production changes). -- [ ] M1 Dedicated `orchestration` Hecate group and node/builder split. +- [x] M1 Dedicated `orchestration` Hecate group and node/builder split. - [ ] M2 Celery task enforcement and `WorkloadClass` extraction. - [ ] M3 Checkpoint payload boundary audit (Hecate group plus structural and property tests). @@ -155,6 +155,17 @@ 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 is +pending for this milestone. + ## Surprises & discoveries - Observation: Hecate counts imports inside `if TYPE_CHECKING:` blocks and @@ -191,6 +202,17 @@ CodeRabbit review completed with 0 findings. 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. + ## Decision log - Decision: interpret "depend on ports only" as "depend on the application and @@ -235,12 +257,38 @@ CodeRabbit review completed with 0 findings. 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. + ## Outcomes & retrospective -To be completed at milestone boundaries and at completion. Compare the result -against the Purpose: a developer importing an adapter into a node, task, or -checkpoint payload must see `make lint` fail, and a non-neutral checkpoint -payload field must fail `make test`. +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. + +Remaining outcomes to complete: a developer importing an adapter into a Celery +task or checkpoint payload must see `make lint` fail in production, and a +non-neutral checkpoint payload field must fail `make test`. ## Context and orientation 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..8b03a3db 100644 --- a/episodic/logging.py +++ b/episodic/logging.py @@ -13,6 +13,7 @@ """ import enum +import json import logging import typing as typ import warnings @@ -253,12 +254,39 @@ def log_error( ) +_event_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 only accept ``exc_info`` and ``stack_info`` + beside the message. Structured fields are serialized into one JSON message + when needed. + """ + 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, 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/_graph_builder.py b/episodic/orchestration/_graph_builder.py new file mode 100644 index 00000000..5b9ddb82 --- /dev/null +++ b/episodic/orchestration/_graph_builder.py @@ -0,0 +1,256 @@ +"""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 +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 + + +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 + _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``. + """ + 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. + """ + 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/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/_types.py b/episodic/orchestration/_types.py index b7f686fe..e57f0f97 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 + +_log_event = log_event class ActionKind(enum.StrEnum): 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() From 15ea317e748298271fa83f7fd6b65ee0316fb2bc Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 26 Jun 2026 21:06:17 +0200 Subject: [PATCH 06/24] Record M1 review completion Note the zero-finding CodeRabbit review for the orchestration graph split milestone. --- ...5-extend-architecture-enforcement-to-orchestration-code.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 index ddd193df..51c459a1 100644 --- 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 @@ -163,8 +163,8 @@ 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 is -pending for this milestone. +`make test` (`1020 passed, 3 skipped, 1 xfailed`). CodeRabbit review completed +with 0 findings. ## Surprises & discoveries From 9dc853e4f313e2897228a23d7b7422f7b023a452 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 26 Jun 2026 21:11:10 +0200 Subject: [PATCH 07/24] Extract worker workload contract Move WorkloadClass into a provider-neutral worker module so task architecture enforcement no longer depends on the Kombu topology module. --- ...cture-enforcement-to-orchestration-code.md | 41 +++++++++++++++++-- episodic/worker/__init__.py | 3 +- episodic/worker/runtime.py | 3 +- episodic/worker/tasks.py | 2 +- episodic/worker/topology.py | 10 +---- episodic/worker/workloads.py | 10 +++++ 6 files changed, 54 insertions(+), 15 deletions(-) create mode 100644 episodic/worker/workloads.py 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 index 51c459a1..959e0550 100644 --- 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 @@ -137,7 +137,7 @@ Adjust per milestone; stop and escalate when a threshold is breached. - [x] M0 Orientation and red harness (fixtures and failing tests, no production changes). - [x] M1 Dedicated `orchestration` Hecate group and node/builder split. -- [ ] M2 Celery task enforcement and `WorkloadClass` extraction. +- [x] M2 Celery task enforcement and `WorkloadClass` extraction. - [ ] M3 Checkpoint payload boundary audit (Hecate group plus structural and property tests). - [ ] M4 Behavioural tests, snapshots, documentation, and roadmap update. @@ -166,6 +166,17 @@ 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 is pending for this milestone. + ## Surprises & discoveries - Observation: Hecate counts imports inside `if TYPE_CHECKING:` blocks and @@ -213,6 +224,14 @@ with 0 findings. 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. + ## Decision log - Decision: interpret "depend on ports only" as "depend on the application and @@ -277,6 +296,14 @@ with 0 findings. 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. + ## Outcomes & retrospective M1 outcome: the LangGraph node functions now live in @@ -286,9 +313,15 @@ M1 outcome: the LangGraph node functions now live in The production architecture gate now groups those graph modules separately from `application`, while leaving checkpoint DTO grouping for M3. -Remaining outcomes to complete: a developer importing an adapter into a Celery -task or checkpoint payload must see `make lint` fail in production, and a -non-neutral checkpoint payload field must fail `make test`. +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. + +Remaining outcomes to complete: a developer importing an adapter into a +checkpoint payload must see `make lint` fail in production, and a non-neutral +checkpoint payload field must fail `make test`. ## Context and orientation 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..43ddf11a --- /dev/null +++ b/episodic/worker/workloads.py @@ -0,0 +1,10 @@ +"""Provider-neutral workload classifications for worker routing.""" + +import enum + + +class WorkloadClass(enum.StrEnum): + """Canonical workload classes for routed Celery tasks.""" + + IO_BOUND = "io_bound" + CPU_BOUND = "cpu_bound" From e84c6848754030f1ba34297c64486f098f7414e5 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 26 Jun 2026 21:17:52 +0200 Subject: [PATCH 08/24] Record M2 review completion Note the zero-finding CodeRabbit review for the worker workload contract milestone. --- ...4-5-extend-architecture-enforcement-to-orchestration-code.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 959e0550..eb67ecd3 100644 --- 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 @@ -175,7 +175,7 @@ config now classifies `episodic.worker.workloads` as `domain_ports` and `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 is pending for this milestone. +CodeRabbit review completed with 0 findings. ## Surprises & discoveries From e017f37ecb9e0bced889e8092ab26d83c0311232 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 26 Jun 2026 21:36:59 +0200 Subject: [PATCH 09/24] Enforce checkpoint payload boundaries Add a provider-neutral orchestration payload DTO core and retarget checkpoint serialization away from the application-coupled DTO barrel. Group checkpoint payload modules under Hecate, add JSON-shape and structural payload boundary tests, and record the M3 decisions in the ExecPlan. --- ...cture-enforcement-to-orchestration-code.md | 66 +++- episodic/orchestration/_action_result_dto.py | 79 +---- episodic/orchestration/_checkpoint_dto.py | 13 +- episodic/orchestration/_checkpoint_payload.py | 46 ++- episodic/orchestration/_dto.py | 245 ++----------- episodic/orchestration/_payload_dto.py | 331 ++++++++++++++++++ episodic/orchestration/_result_dto.py | 2 +- episodic/orchestration/checkpoints.py | 2 +- tests/test_architecture_enforcement.py | 1 - tests/test_checkpoint_payload_boundaries.py | 173 +++++++++ 10 files changed, 623 insertions(+), 335 deletions(-) create mode 100644 episodic/orchestration/_payload_dto.py create mode 100644 tests/test_checkpoint_payload_boundaries.py 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 index eb67ecd3..92cf97b1 100644 --- 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 @@ -138,7 +138,7 @@ Adjust per milestone; stop and escalate when a threshold is breached. production changes). - [x] M1 Dedicated `orchestration` Hecate group and node/builder split. - [x] M2 Celery task enforcement and `WorkloadClass` extraction. -- [ ] M3 Checkpoint payload boundary audit (Hecate group plus structural and +- [x] M3 Checkpoint payload boundary audit (Hecate group plus structural and property tests). - [ ] M4 Behavioural tests, snapshots, documentation, and roadmap update. @@ -177,6 +177,20 @@ 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 normalisation +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 is pending for this milestone. + ## Surprises & discoveries - Observation: Hecate counts imports inside `if TYPE_CHECKING:` blocks and @@ -232,6 +246,26 @@ CodeRabbit review completed with 0 findings. 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 + serialisation 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 @@ -304,6 +338,25 @@ CodeRabbit review completed with 0 findings. 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 serialisation 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. + ## Outcomes & retrospective M1 outcome: the LangGraph node functions now live in @@ -319,9 +372,14 @@ 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. -Remaining outcomes to complete: a developer importing an adapter into a -checkpoint payload must see `make lint` fail in production, and a non-neutral -checkpoint payload field must fail `make test`. +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 serialisation. + +Remaining outcomes to complete: the behavioural, snapshot, documentation, and +roadmap updates in M4 remain. ## Context and orientation 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..ef33886b 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,11 @@ def __post_init__(self) -> None: if not isinstance(self.payload, dict): msg = "payload must be a mapping object." raise TypeError(msg) + try: + json.dumps(self.payload, allow_nan=False) + except (TypeError, ValueError) as exc: + msg = "payload must be JSON-serializable." + raise TypeError(msg) from exc 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..2cf9e58e 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,27 @@ LLMTokenBudget, ) -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 -- 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 . import _payload_dto +from ._types import ActionKind + +ActionExecutionResult = _payload_dto.ActionExecutionResult +ExecutionPlan = _payload_dto.ExecutionPlan +PlannedAction = _payload_dto.PlannedAction +PlannerResult = _payload_dto.PlannerResult +_coerce_action_kind = _payload_dto._coerce_action_kind +_coerce_action_kinds = _payload_dto._coerce_action_kinds +_coerce_model_tier = _payload_dto._coerce_model_tier +_coerce_provider_operation = _payload_dto._coerce_provider_operation +_coerce_single_action_kind = _payload_dto._coerce_single_action_kind +_coerce_single_model_tier = _payload_dto._coerce_single_model_tier +_normalize_non_empty_text = _payload_dto._normalize_non_empty_text +_normalize_required_inputs = _payload_dto._normalize_required_inputs +_normalize_string_fields = _payload_dto._normalize_string_fields +_raise_required_inputs_value_error = _payload_dto._raise_required_inputs_value_error +_require_non_empty_string = _payload_dto._require_non_empty_string +_require_object = _payload_dto._require_object +_require_optional_string_list = _payload_dto._require_optional_string_list +_require_plan_step_list = _payload_dto._require_plan_step_list @dc.dataclass(frozen=True, slots=True) @@ -271,66 +128,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/_payload_dto.py b/episodic/orchestration/_payload_dto.py new file mode 100644 index 00000000..2f8f4ef3 --- /dev/null +++ b/episodic/orchestration/_payload_dto.py @@ -0,0 +1,331 @@ +"""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 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[typ.Any, ...]: + """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 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[typ.Any, ...]: + """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/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/tests/test_architecture_enforcement.py b/tests/test_architecture_enforcement.py index fd49ed67..f9dac381 100644 --- a/tests/test_architecture_enforcement.py +++ b/tests/test_architecture_enforcement.py @@ -250,7 +250,6 @@ def test_checker_accepts_orchestration_fixture_graphs( assert completed_process.returncode == 0, rendered -@pytest.mark.xfail(strict=True, reason="groups added in M1-M3") 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" diff --git a/tests/test_checkpoint_payload_boundaries.py b/tests/test_checkpoint_payload_boundaries.py new file mode 100644 index 00000000..4f18f017 --- /dev/null +++ b/tests/test_checkpoint_payload_boundaries.py @@ -0,0 +1,173 @@ +"""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, +) +_UNION_ORIGINS: frozenset[object] = frozenset({typ.Union, types.UnionType}) +_SEQUENCE_ORIGINS: frozenset[object] = frozenset({list, cabc.Sequence}) +_MAPPING_ORIGINS: frozenset[object] = frozenset({dict, cabc.Mapping}) + + +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 + + +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()}, + ) + + +@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 + + +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 _is_provider_neutral_origin( + origin: object, + arguments: tuple[object, ...], +) -> bool: + """Return whether a generic annotation origin is checkpoint-neutral.""" + if origin in _UNION_ORIGINS: + return all(_is_provider_neutral_type(argument) for argument in arguments) + if origin is typ.Literal: + return all(_is_provider_neutral_literal(argument) for argument in arguments) + if origin is tuple: + return _is_provider_neutral_tuple(arguments) + if origin in _SEQUENCE_ORIGINS: + return all(_is_provider_neutral_type(argument) for argument in arguments) + if origin in _MAPPING_ORIGINS: + key_type, value_type = arguments + return key_type is str and _is_provider_neutral_type(value_type) + return False + + +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)) From 5ada41232548ac5d8e858f83f859bf8d6450aa48 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 26 Jun 2026 21:42:01 +0200 Subject: [PATCH 10/24] Record M3 review completion Record the clean CodeRabbit review result for the checkpoint payload boundary milestone before starting the final behavioural and documentation work. --- ...4-5-extend-architecture-enforcement-to-orchestration-code.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 92cf97b1..433b270f 100644 --- 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 @@ -189,7 +189,7 @@ rejects non-JSON payload values, and 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 is pending for this milestone. +CodeRabbit review completed with 0 findings. ## Surprises & discoveries From c823750c0afc32ed87bd8e101f8b0ae6685be65b Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 26 Jun 2026 22:04:01 +0200 Subject: [PATCH 11/24] Document orchestration enforcement Add behavioural and snapshot coverage for orchestration architecture boundaries, including Vidai Mock coverage for the direct LangGraph path. Record ADR-016, update maintainer documentation, and mark roadmap item `2.4.5` complete. --- ...-014-hexagonal-architecture-enforcement.md | 15 +- ...-orchestration-architecture-enforcement.md | 95 ++++++++++++ docs/contents.md | 4 + docs/developers-guide.md | 45 ++++-- ...isodic-podcast-generation-system-design.md | 40 ++++-- ...cture-enforcement-to-orchestration-code.md | 49 ++++--- ...ph-and-celery-in-hexagonal-architecture.md | 35 +++++ docs/roadmap.md | 6 +- .../test_architecture_enforcement.ambr | 46 ++++++ tests/architecture_hecate_config.py | 6 + .../features/architecture_enforcement.feature | 26 ++++ .../test_architecture_enforcement_steps.py | 32 +++++ tests/test_architecture_enforcement.py | 37 +++++ ...ation_orchestration_langgraph_vidaimock.py | 136 ++++++++++++++++++ 14 files changed, 532 insertions(+), 40 deletions(-) create mode 100644 docs/adr/adr-016-orchestration-architecture-enforcement.md create mode 100644 tests/test_generation_orchestration_langgraph_vidaimock.py 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..17eb1d73 --- /dev/null +++ b/docs/adr/adr-016-orchestration-architecture-enforcement.md @@ -0,0 +1,95 @@ +# 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 orchestration DTOs and domain ports only. +- `orchestration` for graph builders, planning orchestration, and tool + execution policy, allowed to depend on application services and checkpoint + DTOs but not adapters. +- `orchestration_tasks` for `episodic.worker.tasks`, allowed to depend on + domain services, domain ports, and `episodic.worker.topology.WorkloadClass`. +- `orchestration_checkpoint` for checkpoint DTO and payload serialisation + modules, allowed to depend on itself and domain-port value types only. + +`episodic.worker.topology.WorkloadClass` is treated as a domain-port-like +worker contract so task modules can describe workload routing without importing +the Celery runtime. + +## 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. Specific orchestration prefixes must + stay before 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..539af941 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -788,13 +788,26 @@ 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_checkpoint`: provider-neutral checkpoint payload DTO and + serialisation modules. +- `orchestration`: LangGraph builders, graph state, planning orchestration, and + tool execution policy. +- `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_checkpoint` and +`orchestration_nodes` before the broad `orchestration` prefix, and +`orchestration_tasks` before worker adapter prefixes. + +`episodic.worker.topology.WorkloadClass` is a worker workload contract that +task modules may import without pulling in the Celery app or runtime wiring. +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 +818,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 normalising workspace +paths. + ### TEI payload compression Canonical TEI payload storage now supports transparent Zstandard compression @@ -1406,8 +1426,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 serialisation 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 +1446,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..bc4c0fc0 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 @@ -2354,6 +2365,17 @@ stateDiagram-v2 ## Core Workflows + +### Multi-source Ingestion and Prioritization + +1. Producer submits new sources through the API or scheduled connectors. +2. Ingestion service classifies documents, computes freshness and reliability + scores, and applies weighting heuristics defined per series. +3. Conflicts resolve using the weighting matrix; rejected content is retained + for audit. +4. Normalized TEI fragments merge into the canonical episode; provenance is + logged and downstream events trigger generation. + ### Multi-source Ingestion and Prioritization 1. Producer submits new sources through the API or scheduled connectors. 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 index 433b270f..459754ae 100644 --- 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 @@ -140,7 +140,7 @@ Adjust per milestone; stop and escalate when a threshold is breached. - [x] M2 Celery task enforcement and `WorkloadClass` extraction. - [x] M3 Checkpoint payload boundary audit (Hecate group plus structural and property tests). -- [ ] M4 Behavioural tests, snapshots, documentation, and roadmap update. +- [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`, @@ -191,6 +191,19 @@ plus a Hypothesis JSON round-trip property. Focused validation passed with `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 +normalised `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 is pending. + ## Surprises & discoveries - Observation: Hecate counts imports inside `if TYPE_CHECKING:` blocks and @@ -357,6 +370,12 @@ CodeRabbit review completed with 0 findings. 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 @@ -773,32 +792,30 @@ Place under the project's feature directory (confirm the path and step-module convention first). Keep the specification synchronized with M4. ```gherkin -Feature: Orchestration architecture enforcement - As a maintainer of Episodic - I want Hecate to reject adapter imports from orchestration code - So that LangGraph nodes, Celery tasks, and checkpoint payloads stay - behind ports +Feature: Architecture enforcement Scenario: A clean orchestration fixture passes - Given the "orchestration_node_imports_port" fixture - When I run the architecture check on the fixture - Then the check passes with no violations + 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 "orchestration_node_imports_outbound_adapter" fixture - When I run the architecture check on the fixture + 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 violation names the node module as the importer + And the architecture diagnostic mentions "orchestration._graph_nodes" Scenario: A Celery task importing an adapter is rejected - Given the "celery_task_imports_inbound_adapter" fixture - When I run the architecture check on the fixture + 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 "checkpoint_payload_imports_storage" fixture - When I run the architecture check on the fixture + 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 diff --git a/docs/langgraph-and-celery-in-hexagonal-architecture.md b/docs/langgraph-and-celery-in-hexagonal-architecture.md index 0a5f1469..070fda68 100644 --- a/docs/langgraph-and-celery-in-hexagonal-architecture.md +++ b/docs/langgraph-and-celery-in-hexagonal-architecture.md @@ -36,6 +36,41 @@ 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 classifies orchestration modules before +broader adapter prefixes so the first matching group is the strictest useful +boundary. + +- `orchestration_nodes` covers `episodic.orchestration._graph_nodes`. Node + functions may import orchestration DTOs and 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 and domain + ports, but still cannot import inbound or outbound adapters. +- `orchestration_tasks` covers `episodic.worker.tasks`. Tasks may import + `episodic.worker.topology.WorkloadClass`, domain services, and ports; the + worker runtime remains the composition root that wires Celery. +- `orchestration_checkpoint` covers checkpoint DTO and payload serialisation + 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 serialised 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/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/architecture_hecate_config.py b/tests/architecture_hecate_config.py index 3fb2d381..dcef2f97 100644 --- a/tests/architecture_hecate_config.py +++ b/tests/architecture_hecate_config.py @@ -125,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. @@ -137,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 ------- @@ -167,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. 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/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 f9dac381..ec37b00d 100644 --- a/tests/test_architecture_enforcement.py +++ b/tests/test_architecture_enforcement.py @@ -8,6 +8,7 @@ coverage lives in `tests/test_architecture_hecate_config.py`. """ +import json import tomllib import typing as typ from pathlib import Path @@ -207,6 +208,30 @@ def test_checker_diagnostic_output_matches_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 + reports[package_name] = _normalise_hecate_json_report(completed_process) + + assert reports == snapshot + + def test_checker_accepts_allowed_fixture_graph(tmp_path: Path) -> None: """Allowed fixture imports do not produce architecture violations.""" package_name = "allowed_case" @@ -277,3 +302,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_generation_orchestration_langgraph_vidaimock.py b/tests/test_generation_orchestration_langgraph_vidaimock.py new file mode 100644 index 00000000..0ec09859 --- /dev/null +++ b/tests/test_generation_orchestration_langgraph_vidaimock.py @@ -0,0 +1,136 @@ +"""Vidai Mock behavioural coverage for the generation LangGraph path.""" + +from __future__ import annotations + +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() + start_vidaimock_process( + typ.cast("typ.Any", context), tmp_path, port=find_free_port() + ) + try: + yield context + finally: + context.stop() + + +@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 + assert result.action_results[0].show_notes_result is not None + assert result.action_results[0].show_notes_result.entries[0].topic == "Introduction" + assert result.total_usage.total_tokens == 81 + assert [call.model for call in recording_port.requests] == [ + "gpt-4.1", + "gpt-4o-mini", + ] From 63ab75502ce59bfae114407a05a3d24ae2bf1620 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 26 Jun 2026 22:10:37 +0200 Subject: [PATCH 12/24] Record M4 review completion Update the orchestration enforcement execplan with the clean CodeRabbit review result after the M4 documentation and test coverage milestone. --- ...4-5-extend-architecture-enforcement-to-orchestration-code.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 459754ae..26bb3d51 100644 --- 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 @@ -202,7 +202,7 @@ ADR-016, the node/builder split, the `orchestration_nodes`, 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 is pending. +CodeRabbit review completed with 0 findings. ## Surprises & discoveries From e34aed53cab5e730e684e949a41cc7c79a2b3f11 Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 22 Jul 2026 00:01:43 +0200 Subject: [PATCH 13/24] Restore orchestration Hecate groups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reconstruct the first-match architecture group ordering after the rebase silently duplicated the outbound adapter table. Preserve main’s updated LLM adapter prefix while restoring the branch’s orchestration, checkpoint, and worker task boundaries. --- pyproject.toml | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index eacc3597..c0564a8a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -794,6 +794,9 @@ allowed = [ "composition_root", "domain_ports", "inbound_adapter", + "orchestration", + "orchestration_checkpoint", + "orchestration_tasks", "outbound_adapter", ] @@ -823,9 +826,28 @@ 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" +prefixes = [ + "episodic.orchestration._graph_builder", + "episodic.orchestration._graph_nodes", + "episodic.orchestration.langgraph", +] +allowed = ["orchestration", "application", "domain_ports", "orchestration_checkpoint"] + [[tool.hecate.groups]] name = "application" prefixes = [ @@ -838,15 +860,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 +889,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"] From 3e1c4a9c99527bcecc46c72c08d1f1038c39934d Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 22 Jul 2026 00:07:01 +0200 Subject: [PATCH 14/24] Fix orchestration documentation spelling Apply the Oxford spelling forms required by the new repository spelling gate, regenerate the typos configuration, and remove a duplicated ingestion workflow block exposed by Markdown lint. --- ...-orchestration-architecture-enforcement.md | 6 +- docs/developers-guide.md | 14 +- ...isodic-podcast-generation-system-design.md | 15 +- ...cture-enforcement-to-orchestration-code.md | 434 +++++++++--------- ...ph-and-celery-in-hexagonal-architecture.md | 12 +- 5 files changed, 223 insertions(+), 258 deletions(-) diff --git a/docs/adr/adr-016-orchestration-architecture-enforcement.md b/docs/adr/adr-016-orchestration-architecture-enforcement.md index 17eb1d73..15d70f7b 100644 --- a/docs/adr/adr-016-orchestration-architecture-enforcement.md +++ b/docs/adr/adr-016-orchestration-architecture-enforcement.md @@ -12,8 +12,8 @@ groups. ## 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 +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. @@ -53,7 +53,7 @@ The accepted groups are: DTOs but not adapters. - `orchestration_tasks` for `episodic.worker.tasks`, allowed to depend on domain services, domain ports, and `episodic.worker.topology.WorkloadClass`. -- `orchestration_checkpoint` for checkpoint DTO and payload serialisation +- `orchestration_checkpoint` for checkpoint DTO and payload serialization modules, allowed to depend on itself and domain-port value types only. `episodic.worker.topology.WorkloadClass` is treated as a domain-port-like diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 539af941..6857ea8a 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -789,7 +789,7 @@ The enforced groups are: OpenAI-compatible LLM adapters, including `episodic.llm.openai_adapter`, the `episodic.llm.openai_api` helper package, and `episodic.llm.openai_client`. - `orchestration_checkpoint`: provider-neutral checkpoint payload DTO and - serialisation modules. + serialization modules. - `orchestration`: LangGraph builders, graph state, planning orchestration, and tool execution policy. - `orchestration_tasks`: Celery task entrypoints. @@ -798,10 +798,10 @@ The enforced groups are: When adding a new port or adapter, update `[tool.hecate]` in `pyproject.toml` 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_checkpoint` and -`orchestration_nodes` before the broad `orchestration` prefix, and -`orchestration_tasks` before worker adapter prefixes. +prefixes because Hecate uses first-match group ordering: `composition_root` +before adapter prefixes, `orchestration_checkpoint` and `orchestration_nodes` +before the broad `orchestration` prefix, and `orchestration_tasks` before +worker adapter prefixes. `episodic.worker.topology.WorkloadClass` is a worker workload contract that task modules may import without pulling in the Celery app or runtime wiring. @@ -822,7 +822,7 @@ 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 normalising workspace +payload checks. Snapshot JSON diagnostics only after normalizing workspace paths. ### TEI payload compression @@ -1432,7 +1432,7 @@ Roadmap item `2.4.1` introduces a dedicated orchestration package in plan, and action-result payload DTOs. - `episodic/orchestration/_checkpoint_dto.py` and `episodic/orchestration/_checkpoint_payload.py` contain checkpoint state DTOs - and JSON payload serialisation helpers. + 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. diff --git a/docs/episodic-podcast-generation-system-design.md b/docs/episodic-podcast-generation-system-design.md index bc4c0fc0..772c940c 100644 --- a/docs/episodic-podcast-generation-system-design.md +++ b/docs/episodic-podcast-generation-system-design.md @@ -211,8 +211,8 @@ 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, explicit composition roots, LangGraph node modules, - Celery task modules, and orchestration checkpoint payload modules. + 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` delivered LangGraph-node-specific imports, Celery task @@ -2365,17 +2365,6 @@ stateDiagram-v2 ## Core Workflows - -### Multi-source Ingestion and Prioritization - -1. Producer submits new sources through the API or scheduled connectors. -2. Ingestion service classifies documents, computes freshness and reliability - scores, and applies weighting heuristics defined per series. -3. Conflicts resolve using the weighting matrix; rejected content is retained - for audit. -4. Normalized TEI fragments merge into the canonical episode; provenance is - logged and downstream events trigger generation. - ### Multi-source Ingestion and Prioritization 1. Producer submits new sources through the API or scheduled connectors. 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 index 26bb3d51..0297f6c9 100644 --- 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 @@ -1,9 +1,8 @@ # 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. +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: IN PROGRESS @@ -12,10 +11,10 @@ Status: IN PROGRESS 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 +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"). @@ -30,9 +29,9 @@ 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. +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 @@ -53,13 +52,13 @@ escalation, not a workaround. 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 + 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. +- 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. @@ -94,43 +93,38 @@ Adjust per milestone; stop and escalate when a threshold is breached. - 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. + 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. + 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`. + 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 mis-classify a module (for +- 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 mis-ordering. + 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. + `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 @@ -150,21 +144,21 @@ 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. +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. +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 @@ -172,12 +166,12 @@ 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. +`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 normalisation +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 @@ -187,111 +181,104 @@ 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. +`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 -normalised `hecate check --format json` snapshot covering representative +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. +`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. + 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. + 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). + (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. + 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. + 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. + 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. + `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 - serialisation 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. + 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. + 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 @@ -299,82 +286,73 @@ CodeRabbit review completed with 0 findings. 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. + 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. + 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. + 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. + `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. + 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. + `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. + `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. + 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 serialisation 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. + 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. + behaviour changed. Date/Author: 2026-06-26, implementation agent. ## Outcomes & retrospective @@ -382,20 +360,20 @@ 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. +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 +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 serialisation. +round-trip unchanged through JSON serialization. Remaining outcomes to complete: the behavioural, snapshot, documentation, and roadmap updates in M4 remain. @@ -453,10 +431,10 @@ 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. +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 @@ -465,11 +443,11 @@ 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` + `_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). @@ -488,12 +466,12 @@ Celery is a distributed task queue. The worker package: - `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 normalisation helpers from the `_dto` barrel; -the barrel transitively imports `episodic.generation` (application tier) through -`_action_result_dto`. +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) @@ -518,9 +496,9 @@ the barrel transitively imports `episodic.generation` (application tier) through 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-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 @@ -574,14 +552,14 @@ production or configuration change, and confirm the current baseline. 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. + `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 + 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; @@ -605,8 +583,8 @@ graph assembly. 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. + `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 @@ -622,13 +600,13 @@ graph assembly. 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. + (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). +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 @@ -643,9 +621,9 @@ Goal: make Celery task code ports-only by removing its dependence on the 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. + 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 @@ -671,7 +649,7 @@ canonical or ORM state. 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 normalisation helpers and imports only `episodic.llm` (ports) and + 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. @@ -681,9 +659,9 @@ canonical or ORM state. 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. + 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 @@ -693,8 +671,8 @@ canonical or ORM state. 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 serialisation unchanged (proving payloads stay - JSON-shaped and free of non-serialisable adapter or ORM objects). Follow the + 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. @@ -722,10 +700,10 @@ the new boundaries. protected. Only snapshot fixture output (deterministic), never the production tree. 3. Add a behavioural test that exercises `build_generation_orchestration_graph` - end to end against a simulated inference service using `vidai-mock` - (per the `vidai-mock` skill), asserting the graph still plans, executes, and - finishes after the node/builder split, with the mock standing in for the - `LLMPort` adapter. This proves the refactor preserved observable behaviour. + end to end against a simulated inference service using `vidai-mock` (per the + `vidai-mock` skill), asserting the graph still plans, executes, and finishes + after the node/builder split, with the mock standing in for the `LLMPort` + adapter. This proves the refactor preserved observable behaviour. 4. Documentation: - Update `docs/episodic-podcast-generation-system-design.md` to mark the orchestration enforcement slice as delivered and to describe the three new @@ -775,11 +753,11 @@ to match the generated group prefixes the helper emits. Suggested fixtures 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. +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. + 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 @@ -936,7 +914,7 @@ the Decision Log when chosen): - `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 normalisation helpers. + 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`), @@ -944,8 +922,8 @@ the Decision Log when chosen): 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. + 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. diff --git a/docs/langgraph-and-celery-in-hexagonal-architecture.md b/docs/langgraph-and-celery-in-hexagonal-architecture.md index 070fda68..57321ca9 100644 --- a/docs/langgraph-and-celery-in-hexagonal-architecture.md +++ b/docs/langgraph-and-celery-in-hexagonal-architecture.md @@ -36,13 +36,11 @@ 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 classifies orchestration modules before -broader adapter prefixes so the first matching group is the strictest useful -boundary. +Roadmap item `2.4.5` makes those orchestration rules executable through Hecate. +The production configuration classifies orchestration modules before broader +adapter prefixes so the first matching group is the strictest useful boundary. - `orchestration_nodes` covers `episodic.orchestration._graph_nodes`. Node functions may import orchestration DTOs and ports, but not Falcon, Celery, @@ -53,7 +51,7 @@ boundary. - `orchestration_tasks` covers `episodic.worker.tasks`. Tasks may import `episodic.worker.topology.WorkloadClass`, domain services, and ports; the worker runtime remains the composition root that wires Celery. -- `orchestration_checkpoint` covers checkpoint DTO and payload serialisation +- `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. @@ -69,7 +67,7 @@ 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 serialised as JSON. +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 From 3bc41d66585d069fc6e437020bd226029e589c07 Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 29 Jul 2026 00:08:46 +0200 Subject: [PATCH 15/24] Refactor checkpoint origin validation Replace conditional generic-origin validation with a validator dispatch table while preserving recursive payload-boundary semantics. Add direct parametrized coverage for every supported origin category and unsupported generics. --- tests/test_checkpoint_payload_boundaries.py | 101 ++++++++++++++++---- 1 file changed, 81 insertions(+), 20 deletions(-) diff --git a/tests/test_checkpoint_payload_boundaries.py b/tests/test_checkpoint_payload_boundaries.py index 4f18f017..ec7b41d4 100644 --- a/tests/test_checkpoint_payload_boundaries.py +++ b/tests/test_checkpoint_payload_boundaries.py @@ -64,9 +64,7 @@ ), max_leaves=12, ) -_UNION_ORIGINS: frozenset[object] = frozenset({typ.Union, types.UnionType}) -_SEQUENCE_ORIGINS: frozenset[object] = frozenset({list, cabc.Sequence}) -_MAPPING_ORIGINS: frozenset[object] = frozenset({dict, cabc.Mapping}) +type _OriginValidator = cabc.Callable[[tuple[object, ...]], bool] def test_checkpoint_payload_dtos_use_provider_neutral_field_types() -> None: @@ -122,6 +120,46 @@ def test_workflow_checkpoint_payload_round_trips_through_json(payload: object) - assert json.loads(encoded) == checkpoint.payload +@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 + + 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: @@ -142,23 +180,15 @@ def _is_provider_neutral_leaf_type(field_type: type[object]) -> bool: ) -def _is_provider_neutral_origin( - origin: object, - arguments: tuple[object, ...], -) -> bool: - """Return whether a generic annotation origin is checkpoint-neutral.""" - if origin in _UNION_ORIGINS: - return all(_is_provider_neutral_type(argument) for argument in arguments) - if origin is typ.Literal: - return all(_is_provider_neutral_literal(argument) for argument in arguments) - if origin is tuple: - return _is_provider_neutral_tuple(arguments) - if origin in _SEQUENCE_ORIGINS: - return all(_is_provider_neutral_type(argument) for argument in arguments) - if origin in _MAPPING_ORIGINS: - key_type, value_type = arguments - return key_type is str and _is_provider_neutral_type(value_type) - return False +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: @@ -171,3 +201,34 @@ def _is_provider_neutral_tuple(arguments: tuple[object, ...]) -> bool: 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) From 8a1b94236d4de7bc5dd562ed14478aba82effbbf Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 29 Jul 2026 00:13:57 +0200 Subject: [PATCH 16/24] Extract Hecate config assertions Separate base architecture expectations from orchestration-specific group expectations while keeping the fixture config test as the orchestration point. --- tests/test_architecture_hecate_config.py | 48 +++++++++++++++--------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/tests/test_architecture_hecate_config.py b/tests/test_architecture_hecate_config.py index 329438c7..40a4f8f1 100644 --- a/tests/test_architecture_hecate_config.py +++ b/tests/test_architecture_hecate_config.py @@ -73,6 +73,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" @@ -110,6 +119,28 @@ def test_fixture_config_writes_expected_toml_shape(tmp_path: Path) -> None: assert _group_allowed(config, "domain") == ["domain"], ( "domain group must allow only domain imports" ) + assert _group_allowed(config, "application") == ["application", "domain"] + assert _group_prefixes(config, "inbound_adapter") == [ + f"{package}.api", + f"{package}.worker.topology", + ] + assert _group_allowed(config, "inbound_adapter") == [ + "inbound_adapter", + "application", + "domain", + ] + assert _group_allowed(config, "outbound_adapter") == [ + "outbound_adapter", + "application", + "domain", + ] + + +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", @@ -144,23 +175,6 @@ def test_fixture_config_writes_expected_toml_shape(tmp_path: Path) -> None: "domain", "orchestration_checkpoint", ] - assert _group_allowed(config, "application") == ["application", "domain"], ( - "application group must allow application and domain imports" - ) - assert _group_prefixes(config, "inbound_adapter") == [ - f"{package}.api", - f"{package}.worker.topology", - ] - assert _group_allowed(config, "inbound_adapter") == [ - "inbound_adapter", - "application", - "domain", - ], "inbound_adapter group 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" @pytest.mark.parametrize( From 0c0bff5754e5974cebbaaa957f8cc559b48515f1 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 3 Aug 2026 00:53:40 +0200 Subject: [PATCH 17/24] Mark architecture enforcement plan complete Set the ExecPlan status to `COMPLETE` and remove the stale M4 outstanding-work statement while preserving the completed milestone and gate records. --- ...-extend-architecture-enforcement-to-orchestration-code.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) 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 index 0297f6c9..53f314e7 100644 --- 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 @@ -4,7 +4,7 @@ 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: IN PROGRESS +Status: COMPLETE ## Purpose / big picture @@ -375,9 +375,6 @@ 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. -Remaining outcomes to complete: the behavioural, snapshot, documentation, and -roadmap updates in M4 remain. - ## Context and orientation This section assumes no prior knowledge of the repository. From 7b9106a12df9b01b5217eb9ef57f06bed31a6835 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 3 Aug 2026 00:54:02 +0200 Subject: [PATCH 18/24] Document workload class examples Show the IO-bound workload member and its serialized value in both the module and class docstrings. --- episodic/worker/workloads.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/episodic/worker/workloads.py b/episodic/worker/workloads.py index 43ddf11a..8e545e23 100644 --- a/episodic/worker/workloads.py +++ b/episodic/worker/workloads.py @@ -1,10 +1,24 @@ -"""Provider-neutral workload classifications for worker routing.""" +"""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.""" + """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" From 4e430d78eb22d7f4dfcb780cdafa782102867aa9 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 3 Aug 2026 00:55:18 +0200 Subject: [PATCH 19/24] Align orchestration architecture documentation Document the dedicated `orchestration_nodes` boundary and its checkpoint DTO and domain-port dependencies. Use `episodic.worker.workloads.WorkloadClass` as the canonical task contract, while identifying the topology path as a compatibility alias only. --- ...-orchestration-architecture-enforcement.md | 17 ++++++++------- docs/developers-guide.md | 21 ++++++++++++------- ...ph-and-celery-in-hexagonal-architecture.md | 12 ++++++----- 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/docs/adr/adr-016-orchestration-architecture-enforcement.md b/docs/adr/adr-016-orchestration-architecture-enforcement.md index 15d70f7b..281afb0b 100644 --- a/docs/adr/adr-016-orchestration-architecture-enforcement.md +++ b/docs/adr/adr-016-orchestration-architecture-enforcement.md @@ -47,18 +47,20 @@ deterministic import-boundary enforcement, accepting a more detailed The accepted groups are: - `orchestration_nodes` for `episodic.orchestration._graph_nodes`, allowed to - depend on orchestration DTOs and domain ports only. + 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 and checkpoint DTOs but not adapters. - `orchestration_tasks` for `episodic.worker.tasks`, allowed to depend on - domain services, domain ports, and `episodic.worker.topology.WorkloadClass`. + 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.topology.WorkloadClass` is treated as a domain-port-like -worker contract so task modules can describe workload routing without importing -the Celery runtime. +`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 @@ -72,8 +74,9 @@ the Celery runtime. ### Negative -- Hecate group ordering now matters more. Specific orchestration prefixes must - stay before broader orchestration and adapter prefixes. +- 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. diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 6857ea8a..7a71a557 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -788,10 +788,14 @@ 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. + tool execution policy, excluding the dedicated node group. - `orchestration_tasks`: Celery task entrypoints. - `composition_root`: modules that wire concrete adapters, currently `episodic.api.runtime` and `episodic.worker.runtime`. @@ -799,13 +803,14 @@ The enforced groups are: When adding a new port or adapter, update `[tool.hecate]` in `pyproject.toml` 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_checkpoint` and `orchestration_nodes` -before the broad `orchestration` prefix, and `orchestration_tasks` before -worker adapter prefixes. - -`episodic.worker.topology.WorkloadClass` is a worker workload contract that -task modules may import without pulling in the Celery app or runtime wiring. -Worker runtime modules own concrete Celery configuration. +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: diff --git a/docs/langgraph-and-celery-in-hexagonal-architecture.md b/docs/langgraph-and-celery-in-hexagonal-architecture.md index 57321ca9..51b1707a 100644 --- a/docs/langgraph-and-celery-in-hexagonal-architecture.md +++ b/docs/langgraph-and-celery-in-hexagonal-architecture.md @@ -39,17 +39,19 @@ 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 classifies orchestration modules before broader -adapter prefixes so the first matching group is the strictest useful boundary. +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 orchestration DTOs and ports, but not Falcon, Celery, - SQLAlchemy, OpenAI adapters, or other concrete infrastructure. + 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 and domain ports, but still cannot import inbound or outbound adapters. - `orchestration_tasks` covers `episodic.worker.tasks`. Tasks may import - `episodic.worker.topology.WorkloadClass`, domain services, and ports; the + `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 From 30e33175c47e43285f3f5c8717254c00be221feb Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 3 Aug 2026 00:56:12 +0200 Subject: [PATCH 20/24] Complete orchestration architecture ExecPlan Mark the ExecPlan complete and replace stale M4 implementation claims with a completed outcome while retaining the milestone and final-gate evidence. --- ...cture-enforcement-to-orchestration-code.md | 74 +++++-------------- 1 file changed, 19 insertions(+), 55 deletions(-) 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 index 53f314e7..5c4c5b9f 100644 --- 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 @@ -528,10 +528,11 @@ Read these documents (signposts): ## Plan of work -The work proceeds in five milestones. Each follows 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 and -re-run the gates. Architecture rules are validated through the fixture harness +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. @@ -678,56 +679,13 @@ 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 - -Goal: cover externally observable behaviour, lock output format, and document -the new boundaries. - -1. Add a `pytest-bdd` feature that specifies the enforcement workflow from a - maintainer's perspective. Embed the feature in this plan (see "BDD feature" - below) and place it under the project's feature directory (locate with - `leta files tests/` or the existing `*.feature` convention). Steps drive the - fixture harness: a clean orchestration fixture passes; a node importing an - adapter is rejected with an `ARCH001` violation naming the node module; a - Celery task importing an adapter is rejected; a checkpoint payload importing - storage is rejected. -2. Add a `syrupy` snapshot test capturing the `hecate check --format json` - output for one representative orchestration-violation fixture, so the - violation message shape for node, task, and checkpoint rules is regression - protected. Only snapshot fixture output (deterministic), never the - production tree. -3. Add a behavioural test that exercises `build_generation_orchestration_graph` - end to end against a simulated inference service using `vidai-mock` (per the - `vidai-mock` skill), asserting the graph still plans, executes, and finishes - after the node/builder split, with the mock standing in for the `LLMPort` - adapter. This proves the refactor preserved observable behaviour. -4. Documentation: - - Update `docs/episodic-podcast-generation-system-design.md` to mark the - orchestration enforcement slice as delivered and to describe the three new - groups and the checkpoint audit. - - Update `docs/developers-guide.md` "Architecture enforcement" with the new - groups, the first-match ordering for the new prefixes, the - `WorkloadClass` location, and how to add fixtures for orchestration - boundaries. - - Update `docs/langgraph-and-celery-in-hexagonal-architecture.md` (the - orchestration component architecture doc) with the node/builder split, the - ports-only node rule, the task rule, and the checkpoint payload rule. - - Add ADR-016 (verify the next free number with `ls docs/adr/`; ADR-015 is - already used) recording the orchestration-enforcement decisions (the - "ports only" interpretation, the node/builder split, and the - checkpoint audit mechanism), and cross-reference it from - `docs/adr/adr-014-hexagonal-architecture-enforcement.md` and the system - design. Follow the documentation style guide and the - `arch-decision-records` conventions. - - Update `docs/users-guide.md` only if a publicly consumable interface - changed. This slice is internal enforcement; if no public API changes, note - in the Decision Log that no users-guide change was required. - - Add the new ADR and any new component note to `docs/contents.md`. -5. Mark roadmap item 2.4.5 done in `docs/roadmap.md`. - -Validation: `make check-fmt`, `make typecheck`, `make lint`, `make test`, -`make markdownlint`, and `make nixie` all pass. The BDD scenarios fail before -their enforcement exists and pass after. +### 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 @@ -764,7 +722,7 @@ checkpoint groups for these synthetic packages. ## BDD feature Place under the project's feature directory (confirm the path and step-module -convention first). Keep the specification synchronized with M4. +convention first). This specification records the completed M4 scenarios. ```gherkin Feature: Architecture enforcement @@ -927,3 +885,9 @@ the Decision Log when chosen): 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. From c3c01ac695f047ebd2e01fb15187fae4bd1a416e Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 3 Aug 2026 01:15:53 +0200 Subject: [PATCH 21/24] Enforce orchestration node boundary Give graph nodes their own strict Hecate group while allowing broader orchestration modules to assemble them. Align architecture documentation, fixture typing, diagnostics, and command-format coverage with the production policy. --- ...-orchestration-architecture-enforcement.md | 7 ++--- docs/developers-guide.md | 4 ++- ...cture-enforcement-to-orchestration-code.md | 13 ++++---- ...ph-and-celery-in-hexagonal-architecture.md | 11 +++---- pyproject.toml | 15 ++++++++-- .../domain.py | 2 +- .../service.py | 2 +- .../worker/tasks.py | 2 +- .../worker/workloads.py | 2 +- .../domain.py | 2 +- .../orchestration/generation.py | 2 +- .../service.py | 2 +- .../api.py | 2 +- .../orchestration/generation.py | 2 +- .../orchestration_node_imports_port/domain.py | 2 +- tests/test_architecture_enforcement.py | 4 +-- tests/test_architecture_hecate_config.py | 30 ++++++++++++++----- 17 files changed, 64 insertions(+), 40 deletions(-) diff --git a/docs/adr/adr-016-orchestration-architecture-enforcement.md b/docs/adr/adr-016-orchestration-architecture-enforcement.md index 281afb0b..cfe38ee3 100644 --- a/docs/adr/adr-016-orchestration-architecture-enforcement.md +++ b/docs/adr/adr-016-orchestration-architecture-enforcement.md @@ -49,11 +49,10 @@ 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 and checkpoint - DTOs but not adapters. + 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`. + 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. diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 7a71a557..d4e0ab81 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -795,7 +795,9 @@ The enforced groups are: - `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. + 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`. 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 index 5c4c5b9f..955b5e28 100644 --- 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 @@ -528,13 +528,12 @@ Read these documents (signposts): ## 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. +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) diff --git a/docs/langgraph-and-celery-in-hexagonal-architecture.md b/docs/langgraph-and-celery-in-hexagonal-architecture.md index 51b1707a..25c0a68e 100644 --- a/docs/langgraph-and-celery-in-hexagonal-architecture.md +++ b/docs/langgraph-and-celery-in-hexagonal-architecture.md @@ -39,17 +39,18 @@ 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. +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 and domain - ports, but still cannot import inbound or outbound adapters. + 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. diff --git a/pyproject.toml b/pyproject.toml index c0564a8a..09f03e31 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -796,6 +796,7 @@ allowed = [ "inbound_adapter", "orchestration", "orchestration_checkpoint", + "orchestration_nodes", "orchestration_tasks", "outbound_adapter", ] @@ -839,14 +840,24 @@ prefixes = [ ] 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._graph_nodes", "episodic.orchestration.langgraph", ] -allowed = ["orchestration", "application", "domain_ports", "orchestration_checkpoint"] +allowed = [ + "orchestration", + "application", + "domain_ports", + "orchestration_checkpoint", + "orchestration_nodes", +] [[tool.hecate.groups]] name = "application" diff --git a/tests/fixtures/architecture/celery_task_imports_domain_service/domain.py b/tests/fixtures/architecture/celery_task_imports_domain_service/domain.py index 85e79576..7b8f1f36 100644 --- a/tests/fixtures/architecture/celery_task_imports_domain_service/domain.py +++ b/tests/fixtures/architecture/celery_task_imports_domain_service/domain.py @@ -1,3 +1,3 @@ """Domain fixture for an allowed task dependency.""" -VALUE = "domain" +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 index 91edd5be..48b24b8b 100644 --- a/tests/fixtures/architecture/celery_task_imports_domain_service/service.py +++ b/tests/fixtures/architecture/celery_task_imports_domain_service/service.py @@ -2,4 +2,4 @@ from tests.fixtures.architecture.celery_task_imports_domain_service import domain -VALUE = domain.VALUE +VALUE: str = domain.VALUE 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 index af0cf73e..02fbaed8 100644 --- a/tests/fixtures/architecture/celery_task_imports_domain_service/worker/tasks.py +++ b/tests/fixtures/architecture/celery_task_imports_domain_service/worker/tasks.py @@ -5,4 +5,4 @@ workloads, ) -VALUE = (service.VALUE, workloads.VALUE) +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 index 598071ae..5db5296f 100644 --- a/tests/fixtures/architecture/celery_task_imports_domain_service/worker/workloads.py +++ b/tests/fixtures/architecture/celery_task_imports_domain_service/worker/workloads.py @@ -1,3 +1,3 @@ """Domain-port workload contract fixture.""" -VALUE = "workload" +VALUE: str = "workload" diff --git a/tests/fixtures/architecture/orchestration_imports_domain_service/domain.py b/tests/fixtures/architecture/orchestration_imports_domain_service/domain.py index dda2f11c..203d3971 100644 --- a/tests/fixtures/architecture/orchestration_imports_domain_service/domain.py +++ b/tests/fixtures/architecture/orchestration_imports_domain_service/domain.py @@ -1,3 +1,3 @@ """Domain fixture for an orchestration dependency.""" -VALUE = "domain" +VALUE: str = "domain" diff --git a/tests/fixtures/architecture/orchestration_imports_domain_service/orchestration/generation.py b/tests/fixtures/architecture/orchestration_imports_domain_service/orchestration/generation.py index 1d0692f5..e517d956 100644 --- a/tests/fixtures/architecture/orchestration_imports_domain_service/orchestration/generation.py +++ b/tests/fixtures/architecture/orchestration_imports_domain_service/orchestration/generation.py @@ -2,4 +2,4 @@ from tests.fixtures.architecture.orchestration_imports_domain_service import service -VALUE = service.VALUE +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 index 0a13c6d0..5f6f373e 100644 --- a/tests/fixtures/architecture/orchestration_imports_domain_service/service.py +++ b/tests/fixtures/architecture/orchestration_imports_domain_service/service.py @@ -2,4 +2,4 @@ from tests.fixtures.architecture.orchestration_imports_domain_service import domain -VALUE = domain.VALUE +VALUE: str = domain.VALUE diff --git a/tests/fixtures/architecture/orchestration_imports_inbound_adapter/api.py b/tests/fixtures/architecture/orchestration_imports_inbound_adapter/api.py index 466d744c..72ff6d26 100644 --- a/tests/fixtures/architecture/orchestration_imports_inbound_adapter/api.py +++ b/tests/fixtures/architecture/orchestration_imports_inbound_adapter/api.py @@ -1,3 +1,3 @@ """Inbound-adapter fixture.""" -VALUE = "api" +VALUE: str = "api" diff --git a/tests/fixtures/architecture/orchestration_imports_inbound_adapter/orchestration/generation.py b/tests/fixtures/architecture/orchestration_imports_inbound_adapter/orchestration/generation.py index bbae6b8d..4974444f 100644 --- a/tests/fixtures/architecture/orchestration_imports_inbound_adapter/orchestration/generation.py +++ b/tests/fixtures/architecture/orchestration_imports_inbound_adapter/orchestration/generation.py @@ -2,4 +2,4 @@ from tests.fixtures.architecture.orchestration_imports_inbound_adapter import api -VALUE = api.VALUE +VALUE: str = api.VALUE diff --git a/tests/fixtures/architecture/orchestration_node_imports_port/domain.py b/tests/fixtures/architecture/orchestration_node_imports_port/domain.py index 8850f052..4ed5b83f 100644 --- a/tests/fixtures/architecture/orchestration_node_imports_port/domain.py +++ b/tests/fixtures/architecture/orchestration_node_imports_port/domain.py @@ -1,3 +1,3 @@ """Domain-port fixture.""" -VALUE = "domain" +VALUE: str = "domain" diff --git a/tests/test_architecture_enforcement.py b/tests/test_architecture_enforcement.py index ec37b00d..c9c9ba9b 100644 --- a/tests/test_architecture_enforcement.py +++ b/tests/test_architecture_enforcement.py @@ -200,9 +200,7 @@ 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" ) diff --git a/tests/test_architecture_hecate_config.py b/tests/test_architecture_hecate_config.py index 40a4f8f1..980012de 100644 --- a/tests/test_architecture_hecate_config.py +++ b/tests/test_architecture_hecate_config.py @@ -279,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") @@ -308,15 +309,28 @@ 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" + assert captured_command == [ + "/custom/python", + "-m", + "hecate", + "check", + "--config", + str(config_path), + "--package", + "tests.fixtures.architecture.allowed_case", + "--root", + str( + Path(__file__).resolve().parent + / "fixtures" + / "architecture" + / "allowed_case", + ), + "--format", + output_format, + ] def test_production_check_uses_injected_python( From 957f683c7b3665f3d0f56885e94c182f68d97e65 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 3 Aug 2026 01:15:58 +0200 Subject: [PATCH 22/24] Harden structured payload serialization Normalize non-JSON structured log fields and require checkpoint payloads to survive JSON round trips without data loss. Cover representative logging values and lossy checkpoint shapes. --- episodic/logging.py | 23 +++++++- episodic/orchestration/_checkpoint_dto.py | 5 +- tests/test_checkpoint_payload_boundaries.py | 25 +++++++++ tests/test_structured_logging.py | 58 +++++++++++++++++++++ 4 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 tests/test_structured_logging.py diff --git a/episodic/logging.py b/episodic/logging.py index 8b03a3db..3a7a4a57 100644 --- a/episodic/logging.py +++ b/episodic/logging.py @@ -12,10 +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 @@ -257,12 +259,28 @@ def log_error( _event_log = getLogger(__name__) +def _serialize_log_field(value: object) -> object: + """Return a stable JSON-compatible representation for a log field.""" + if isinstance(value, enum.Enum): + return value.value + if isinstance(value, dt.date | dt.time): + return value.isoformat() + if isinstance(value, uuid.UUID | BaseException): + return str(value) + 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 = { @@ -271,7 +289,10 @@ def log_event(level: str, message: str, **fields: object) -> None: 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) + log_method( + json.dumps(payload, default=_serialize_log_field, sort_keys=True), + **allowed_kwargs, + ) return try: log_method(message, **allowed_kwargs) diff --git a/episodic/orchestration/_checkpoint_dto.py b/episodic/orchestration/_checkpoint_dto.py index ef33886b..7d758611 100644 --- a/episodic/orchestration/_checkpoint_dto.py +++ b/episodic/orchestration/_checkpoint_dto.py @@ -48,10 +48,13 @@ def __post_init__(self) -> None: msg = "payload must be a mapping object." raise TypeError(msg) try: - json.dumps(self.payload, allow_nan=False) + 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/tests/test_checkpoint_payload_boundaries.py b/tests/test_checkpoint_payload_boundaries.py index ec7b41d4..7b8e54ba 100644 --- a/tests/test_checkpoint_payload_boundaries.py +++ b/tests/test_checkpoint_payload_boundaries.py @@ -103,6 +103,31 @@ def test_workflow_checkpoint_rejects_non_json_payload_values() -> None: ) +@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.""" diff --git a/tests/test_structured_logging.py b/tests/test_structured_logging.py new file mode 100644 index 00000000..0ba96596 --- /dev/null +++ b/tests/test_structured_logging.py @@ -0,0 +1,58 @@ +"""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 + 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"), + ) + + assert json.loads(logger.messages[0]) == { + "error": "provider unavailable", + "event": "generation.failed", + "event_id": str(event_id), + "occurred_at": "2026-08-03T12:30:00+00:00", + "state": "ready", + } From c00716cbd6f32dd82f45cb0ffec02d0a20b22905 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 3 Aug 2026 01:16:05 +0200 Subject: [PATCH 23/24] Tighten orchestration helper contracts Use slotted graph extensions, pass finish callbacks their DTO directly, and replace payload-boundary Any types with provider-neutral protocols. Guard Vidai Mock cleanup before process startup. --- episodic/orchestration/_graph_builder.py | 22 +++++++++++---- episodic/orchestration/_payload_dto.py | 27 +++++++++++++++++-- ...ation_orchestration_langgraph_vidaimock.py | 12 +++------ 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/episodic/orchestration/_graph_builder.py b/episodic/orchestration/_graph_builder.py index 5b9ddb82..050030c6 100644 --- a/episodic/orchestration/_graph_builder.py +++ b/episodic/orchestration/_graph_builder.py @@ -35,9 +35,19 @@ protocols = importlib.import_module("episodic.orchestration._protocols") -@dc.dataclass +@dc.dataclass(slots=True) class GenerationGraphExtensions: - """Optional collaborators for the generation orchestration graph.""" + """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 = ( @@ -48,7 +58,7 @@ class GenerationGraphExtensions: def _invoke_finish_callback( finish_callback: cabc.Callable[[dto.GenerationOrchestrationResult], None], - result: dict[str, dto.GenerationOrchestrationResult], + result: dto.GenerationOrchestrationResult, correlation_id: str | None, ) -> None: """Invoke *finish_callback* with the aggregated domain result. @@ -60,7 +70,7 @@ def _invoke_finish_callback( invocations must provide their own synchronization. """ try: - finish_callback(result["orchestration_result"]) + finish_callback(result) _log_event( "debug", "generation_graph.finish_node.callback.finish", @@ -238,7 +248,9 @@ async def _run_finish_node( state.request.correlation_id if state.request is not None else None ) _invoke_finish_callback( - graph_extensions.finish_callback, result, correlation_id + graph_extensions.finish_callback, + result["orchestration_result"], + correlation_id, ) return result diff --git a/episodic/orchestration/_payload_dto.py b/episodic/orchestration/_payload_dto.py index 2f8f4ef3..10e951ed 100644 --- a/episodic/orchestration/_payload_dto.py +++ b/episodic/orchestration/_payload_dto.py @@ -243,6 +243,20 @@ class PlannerResult: ) +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.""" @@ -252,7 +266,7 @@ def usage(self) -> LLMUsage: raise NotImplementedError @property - def entries(self) -> tuple[typ.Any, ...]: + def entries(self) -> tuple[ShowNotesEntryAttachment, ...]: """Return show-note entries without importing generation DTOs.""" raise NotImplementedError @@ -266,6 +280,15 @@ def model(self) -> str: 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.""" @@ -275,7 +298,7 @@ def generation_result(self) -> GenerationResultAttachment: raise NotImplementedError @property - def sources(self) -> tuple[typ.Any, ...]: + def sources(self) -> tuple[GuestBioSourceAttachment, ...]: """Return source attachments without importing canonical DTOs.""" raise NotImplementedError diff --git a/tests/test_generation_orchestration_langgraph_vidaimock.py b/tests/test_generation_orchestration_langgraph_vidaimock.py index 0ec09859..1154bbc8 100644 --- a/tests/test_generation_orchestration_langgraph_vidaimock.py +++ b/tests/test_generation_orchestration_langgraph_vidaimock.py @@ -1,7 +1,6 @@ """Vidai Mock behavioural coverage for the generation LangGraph path.""" -from __future__ import annotations - +import contextlib import dataclasses as dc import subprocess # noqa: S404 - required to manage the local Vidai Mock process import typing as typ @@ -78,13 +77,10 @@ def vidaimock_context(tmp_path: Path) -> cabc.Iterator[_VidaiContext]: write_provider_config(provider_dir) write_response_template(template_dir) context = _VidaiContext() - start_vidaimock_process( - typ.cast("typ.Any", context), tmp_path, port=find_free_port() - ) - try: + with contextlib.ExitStack() as stack: + stack.callback(context.stop) + start_vidaimock_process(context, tmp_path, port=find_free_port()) yield context - finally: - context.stop() @pytest.mark.asyncio From 5b1e8aa5bf2f4dd8f7a3238c1e311714e81f9156 Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 25 Aug 2026 12:45:54 +0200 Subject: [PATCH 24/24] Adapt orchestration changes to current checks Preserve architecture fixture semantics while satisfying the stricter lint rules from `main`. Remove the obsolete command snapshot after parametrizing its text and JSON formats, and restore documentation required by current public-function checks. --- episodic/logging.py | 16 ++-- episodic/orchestration/_dto.py | 74 ++++++++++++++----- episodic/orchestration/_graph_builder.py | 10 ++- episodic/orchestration/_types.py | 6 +- .../test_architecture_hecate_config.ambr | 15 ---- .../worker/tasks.py | 2 +- .../worker/tasks.py | 2 +- .../orchestration/_checkpoint_payload.py | 2 +- .../service.py | 2 +- .../orchestration/_checkpoint_payload.py | 2 +- .../orchestration/_graph_nodes.py | 2 +- .../orchestration/_graph_nodes.py | 2 +- .../orchestration/generation.py | 2 +- tests/test_architecture_enforcement.py | 6 +- tests/test_architecture_hecate_config.py | 50 ++++++------- tests/test_checkpoint_payload_boundaries.py | 10 ++- ...ation_orchestration_langgraph_vidaimock.py | 19 +++-- tests/test_structured_logging.py | 7 +- 18 files changed, 136 insertions(+), 93 deletions(-) delete mode 100644 tests/__snapshots__/test_architecture_hecate_config.ambr diff --git a/episodic/logging.py b/episodic/logging.py index 3a7a4a57..7c1652c7 100644 --- a/episodic/logging.py +++ b/episodic/logging.py @@ -261,13 +261,15 @@ def log_error( def _serialize_log_field(value: object) -> object: """Return a stable JSON-compatible representation for a log field.""" - if isinstance(value, enum.Enum): - return value.value - if isinstance(value, dt.date | dt.time): - return value.isoformat() - if isinstance(value, uuid.UUID | BaseException): - return str(value) - return str(value) + 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: diff --git a/episodic/orchestration/_dto.py b/episodic/orchestration/_dto.py index 2cf9e58e..afc4dc10 100644 --- a/episodic/orchestration/_dto.py +++ b/episodic/orchestration/_dto.py @@ -8,28 +8,62 @@ LLMTokenBudget, ) -from . import _payload_dto +from ._payload_dto import ( + ActionExecutionResult as ActionExecutionResult, +) +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 -ActionExecutionResult = _payload_dto.ActionExecutionResult -ExecutionPlan = _payload_dto.ExecutionPlan -PlannedAction = _payload_dto.PlannedAction -PlannerResult = _payload_dto.PlannerResult -_coerce_action_kind = _payload_dto._coerce_action_kind -_coerce_action_kinds = _payload_dto._coerce_action_kinds -_coerce_model_tier = _payload_dto._coerce_model_tier -_coerce_provider_operation = _payload_dto._coerce_provider_operation -_coerce_single_action_kind = _payload_dto._coerce_single_action_kind -_coerce_single_model_tier = _payload_dto._coerce_single_model_tier -_normalize_non_empty_text = _payload_dto._normalize_non_empty_text -_normalize_required_inputs = _payload_dto._normalize_required_inputs -_normalize_string_fields = _payload_dto._normalize_string_fields -_raise_required_inputs_value_error = _payload_dto._raise_required_inputs_value_error -_require_non_empty_string = _payload_dto._require_non_empty_string -_require_object = _payload_dto._require_object -_require_optional_string_list = _payload_dto._require_optional_string_list -_require_plan_step_list = _payload_dto._require_plan_step_list - @dc.dataclass(frozen=True, slots=True) class GenerationOrchestrationRequest: diff --git a/episodic/orchestration/_graph_builder.py b/episodic/orchestration/_graph_builder.py index 050030c6..46471f87 100644 --- a/episodic/orchestration/_graph_builder.py +++ b/episodic/orchestration/_graph_builder.py @@ -76,7 +76,7 @@ def _invoke_finish_callback( "generation_graph.finish_node.callback.finish", correlation_id=correlation_id, ) - except Exception as exc: # noqa: BLE001 + except Exception as exc: # noqa: BLE001 - Callback failures must not replace a completed graph result. _log_event( "error", "generation_graph.finish_node.callback.error", @@ -178,6 +178,10 @@ def _build_execute_node( 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: @@ -225,6 +229,10 @@ def build_generation_orchestration_graph( 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) diff --git a/episodic/orchestration/_types.py b/episodic/orchestration/_types.py index e57f0f97..05af7c26 100644 --- a/episodic/orchestration/_types.py +++ b/episodic/orchestration/_types.py @@ -2,9 +2,9 @@ import enum -from episodic.logging import log_event - -_log_event = log_event +from episodic.logging import ( + log_event as _log_event, # noqa: F401 - Compatibility re-export for orchestration callers. +) class ActionKind(enum.StrEnum): 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/fixtures/architecture/celery_task_imports_inbound_adapter/worker/tasks.py b/tests/fixtures/architecture/celery_task_imports_inbound_adapter/worker/tasks.py index 76d5c0cc..c5179f2c 100644 --- a/tests/fixtures/architecture/celery_task_imports_inbound_adapter/worker/tasks.py +++ b/tests/fixtures/architecture/celery_task_imports_inbound_adapter/worker/tasks.py @@ -2,4 +2,4 @@ from tests.fixtures.architecture.celery_task_imports_inbound_adapter import api -VALUE = api.VALUE +VALUE: str = api.VALUE 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 index 5592e798..eb22cd17 100644 --- a/tests/fixtures/architecture/celery_task_imports_outbound_adapter/worker/tasks.py +++ b/tests/fixtures/architecture/celery_task_imports_outbound_adapter/worker/tasks.py @@ -2,4 +2,4 @@ from tests.fixtures.architecture.celery_task_imports_outbound_adapter import storage -VALUE = storage.VALUE +VALUE: str = storage.VALUE 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 index f0937fc8..1b2e8d58 100644 --- a/tests/fixtures/architecture/checkpoint_payload_imports_application/orchestration/_checkpoint_payload.py +++ b/tests/fixtures/architecture/checkpoint_payload_imports_application/orchestration/_checkpoint_payload.py @@ -2,4 +2,4 @@ from tests.fixtures.architecture.checkpoint_payload_imports_application import service -VALUE = service.VALUE +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 index adf1776f..b85358b2 100644 --- a/tests/fixtures/architecture/checkpoint_payload_imports_application/service.py +++ b/tests/fixtures/architecture/checkpoint_payload_imports_application/service.py @@ -2,4 +2,4 @@ from tests.fixtures.architecture.checkpoint_payload_imports_application import domain -VALUE = domain.VALUE +VALUE: str = domain.VALUE 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 index 8f9795a1..b1ebb68b 100644 --- a/tests/fixtures/architecture/checkpoint_payload_imports_storage/orchestration/_checkpoint_payload.py +++ b/tests/fixtures/architecture/checkpoint_payload_imports_storage/orchestration/_checkpoint_payload.py @@ -2,4 +2,4 @@ from tests.fixtures.architecture.checkpoint_payload_imports_storage import storage -VALUE = storage.VALUE +VALUE: str = storage.VALUE 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 index 4feaf4c4..0dc89e0a 100644 --- 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 @@ -4,4 +4,4 @@ storage, ) -VALUE = storage.VALUE +VALUE: str = storage.VALUE 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 index dbef69f0..55e75832 100644 --- a/tests/fixtures/architecture/orchestration_node_imports_port/orchestration/_graph_nodes.py +++ b/tests/fixtures/architecture/orchestration_node_imports_port/orchestration/_graph_nodes.py @@ -2,4 +2,4 @@ from tests.fixtures.architecture.orchestration_node_imports_port import domain -VALUE = domain.VALUE +VALUE: str = domain.VALUE diff --git a/tests/fixtures/architecture/ungrouped_adapter_is_caught/orchestration/generation.py b/tests/fixtures/architecture/ungrouped_adapter_is_caught/orchestration/generation.py index 44e4135d..5191d1fb 100644 --- a/tests/fixtures/architecture/ungrouped_adapter_is_caught/orchestration/generation.py +++ b/tests/fixtures/architecture/ungrouped_adapter_is_caught/orchestration/generation.py @@ -2,4 +2,4 @@ from tests.fixtures.architecture.ungrouped_adapter_is_caught import adapter -VALUE = adapter.VALUE +VALUE: str = adapter.VALUE diff --git a/tests/test_architecture_enforcement.py b/tests/test_architecture_enforcement.py index c9c9ba9b..337b3615 100644 --- a/tests/test_architecture_enforcement.py +++ b/tests/test_architecture_enforcement.py @@ -224,10 +224,10 @@ def test_orchestration_json_diagnostics_match_snapshot( config_path, output_format="json", ) - assert completed_process.returncode == 1 + assert completed_process.returncode == 1, _render_process(completed_process) reports[package_name] = _normalise_hecate_json_report(completed_process) - assert reports == snapshot + assert reports == snapshot, "normalized JSON diagnostics must match the snapshot" def test_checker_accepts_allowed_fixture_graph(tmp_path: Path) -> None: @@ -286,7 +286,7 @@ def test_production_config_declares_orchestration_groups() -> None: "orchestration", "orchestration_tasks", "orchestration_checkpoint", - } <= group_names + } <= group_names, "production Hecate config must declare orchestration groups" def test_production_checker_accepts_scoped_packages() -> None: diff --git a/tests/test_architecture_hecate_config.py b/tests/test_architecture_hecate_config.py index 980012de..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): @@ -100,7 +96,9 @@ def _assert_expected_base_groups( "inbound_adapter", "outbound_adapter", ], "Hecate groups must retain first-match policy order" - assert _group_prefixes(config, "composition_root") == [f"{package}.runtime"] + assert _group_prefixes(config, "composition_root") == [f"{package}.runtime"], ( + "composition_root must match the runtime module" + ) assert _group_allowed(config, "composition_root") == [ "application", "composition_root", @@ -115,25 +113,27 @@ def _assert_expected_base_groups( 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"] + assert _group_allowed(config, "application") == ["application", "domain"], ( + "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 must allow inbound, application, and domain imports" assert _group_allowed(config, "outbound_adapter") == [ "outbound_adapter", "application", "domain", - ] + ], "outbound_adapter must allow outbound, application, and domain imports" def _assert_expected_orchestration_groups( @@ -145,36 +145,36 @@ def _assert_expected_orchestration_groups( 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( @@ -312,7 +312,7 @@ def capture_run( output_format=output_format, ) - assert captured_command == [ + expected_command = [ "/custom/python", "-m", "hecate", @@ -322,15 +322,13 @@ def capture_run( "--package", "tests.fixtures.architecture.allowed_case", "--root", - str( - Path(__file__).resolve().parent - / "fixtures" - / "architecture" - / "allowed_case", - ), + 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( diff --git a/tests/test_checkpoint_payload_boundaries.py b/tests/test_checkpoint_payload_boundaries.py index 7b8e54ba..e98fb29a 100644 --- a/tests/test_checkpoint_payload_boundaries.py +++ b/tests/test_checkpoint_payload_boundaries.py @@ -84,7 +84,7 @@ def test_checkpoint_payload_dtos_use_provider_neutral_field_types() -> None: f"{field_type!r}" ) - assert not rejected_fields + assert not rejected_fields, f"provider-specific DTO fields found: {rejected_fields}" def test_workflow_checkpoint_rejects_non_json_payload_values() -> None: @@ -142,7 +142,9 @@ def test_workflow_checkpoint_payload_round_trips_through_json(payload: object) - encoded = json.dumps(checkpoint.payload, sort_keys=True) - assert json.loads(encoded) == checkpoint.payload + assert json.loads(encoded) == checkpoint.payload, ( + "checkpoint payload must survive JSON round-tripping" + ) @pytest.mark.parametrize( @@ -182,7 +184,9 @@ def test_provider_neutral_origin_dispatch( expected: bool, ) -> None: """Generic annotation origins dispatch to the expected validator.""" - assert _is_provider_neutral_origin(origin, arguments) is expected + 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: diff --git a/tests/test_generation_orchestration_langgraph_vidaimock.py b/tests/test_generation_orchestration_langgraph_vidaimock.py index 1154bbc8..f2f6b025 100644 --- a/tests/test_generation_orchestration_langgraph_vidaimock.py +++ b/tests/test_generation_orchestration_langgraph_vidaimock.py @@ -122,11 +122,20 @@ async def test_langgraph_plans_executes_and_finishes_with_vidai_mock( state = await graph.ainvoke(GenerationGraphState(request=request)) result = state["orchestration_result"] - assert result.plan.steps[0].action_kind is ActionKind.GENERATE_SHOW_NOTES - assert result.action_results[0].show_notes_result is not None - assert result.action_results[0].show_notes_result.entries[0].topic == "Introduction" - assert result.total_usage.total_tokens == 81 + 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 index 0ba96596..19d14b77 100644 --- a/tests/test_structured_logging.py +++ b/tests/test_structured_logging.py @@ -21,7 +21,7 @@ def __init__(self) -> None: def info(self, message: str, **kwargs: object) -> None: """Record one INFO message and verify no logger kwargs were added.""" - assert not kwargs + assert not kwargs, "structured event fields must be encoded in the message" self.messages.append(message) @@ -49,10 +49,13 @@ def test_log_event_normalizes_non_json_structured_fields( error=RuntimeError("provider unavailable"), ) - assert json.loads(logger.messages[0]) == { + 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" + )