From 83ad231adbd06353d968756b35537b4f9f64d8b3 Mon Sep 17 00:00:00 2001 From: Oleksandr Bezdieniezhnykh Date: Sun, 24 May 2026 13:03:57 +0300 Subject: [PATCH] [AZ-847] Update module-layout rule 9 with bench + register_ carve-outs Extend the AZ-507 cross-component contract surface (rule 9) to name the two narrow carve-outs that the AZ-847 lint already enforces: (a) bench exclusion - components//bench/** files are skipped because benchmark/measurement code legitimately constructs production strategies via runtime_root.* factories. (b) self-registration carve-out - ImportFrom of register_* helpers from gps_denied_onboard.runtime_root.* is allowed, since this is the central-registry pattern, not cross-component coupling. Resolves the 2026-05-24 leftover; the test docstring stays the formal source of truth and is now mirrored by rule-9 wording. No code change. Doc-only. Closes leftover entry _docs/_process_leftovers/2026-05-24_az847_rule9_wording_followup.md. Co-authored-by: Cursor --- _docs/02_document/module-layout.md | 2 +- ...2026-05-24_az847_rule9_wording_followup.md | 58 ------------------- 2 files changed, 1 insertion(+), 59 deletions(-) delete mode 100644 _docs/_process_leftovers/2026-05-24_az847_rule9_wording_followup.md diff --git a/_docs/02_document/module-layout.md b/_docs/02_document/module-layout.md index 1e9b3d9..b023d73 100644 --- a/_docs/02_document/module-layout.md +++ b/_docs/02_document/module-layout.md @@ -19,7 +19,7 @@ Bootstrap reference: `_docs/02_tasks/todo/AZ-263_initial_structure.md`. Architec 6. The composition root is `src/gps_denied_onboard/runtime_root.py`. It is the ONLY place that may import concrete strategy implementations across components — every other cross-component dependency is constructor-injected against an interface (ADR-009). 7. Tests mirror the component graph 1:1 at `tests/unit//`. In-process cross-component scenarios that import SUT source live under `tests/integration/`. The **blackbox / e2e** test harness — which MUST NOT import SUT source and exercises the system only via public boundaries (MAVLink / MSP2 / HTTP / filesystem) — lives at the repo-root `e2e/` directory and is owned by the `blackbox_tests` cross-cutting entry (Shared section). Performance, resilience, security, and resource-limit scenarios that are also boundary-driven likewise live under `e2e/tests//`; only in-process performance/security micro-tests (if any) would live under `tests/perf/`, `tests/security/`, `tests/resilience/`. 8. Build-time exclusion (ADR-002): each `/_native/` and the corresponding `cpp//` carry a CMake `BUILD_` flag. The composition root validator refuses to wire a strategy whose flag is OFF. -9. **AZ-507 cross-component contract surface** — the only places a `components//*.py` file may import are: its own subpackage (`gps_denied_onboard.components..*`), `_types/*`, `_types.inference_errors`, `helpers/*`, `config`, `logging`, `fdr_client`, `clock`, `frame_source` (interface only). Cross-component contracts (Protocols + typed exceptions) reach consumers through `_types/*` modules — DTOs in the canonical `_types` files (e.g. `_types.inference.EngineCacheEntry`), typed-error envelopes in `_types.inference_errors`, and consumer-side structural `Protocol` cuts defined locally inside each consuming component (e.g. `c10_provisioning.engine_compiler.CompileEngineCallable`). NEVER `from gps_denied_onboard.components. import ...` — the AZ-270 `test_az270_compose_root.test_ac6_only_compose_root_imports_concrete_strategies` lint enforces this on every `components/**/*.py`. The composition root (`runtime_root/*`) is the single exception; it wires concrete strategies into duck-typed Protocol parameters via constructor injection. This rule is the architectural contract paired with the AZ-270 lint; see `architecture.md` § Cross-Component Contract Surface for the rationale. +9. **AZ-507 cross-component contract surface** — the only places a `components//*.py` file may import are: its own subpackage (`gps_denied_onboard.components..*`), `_types/*`, `_types.inference_errors`, `helpers/*`, `config`, `logging`, `fdr_client`, `clock`, `frame_source` (interface only). Cross-component contracts (Protocols + typed exceptions) reach consumers through `_types/*` modules — DTOs in the canonical `_types` files (e.g. `_types.inference.EngineCacheEntry`), typed-error envelopes in `_types.inference_errors`, and consumer-side structural `Protocol` cuts defined locally inside each consuming component (e.g. `c10_provisioning.engine_compiler.CompileEngineCallable`). NEVER `from gps_denied_onboard.components. import ...` — the AZ-270 `test_az270_compose_root.test_ac6_only_compose_root_imports_concrete_strategies` lint enforces this on every `components/**/*.py`. The composition root (`runtime_root/*`) is the single exception; it wires concrete strategies into duck-typed Protocol parameters via constructor injection. Two narrow carve-outs apply to the lint's enforcement on `components/**/*.py` (AZ-847, source of truth: docstring of `tests/unit/test_az270_compose_root.test_ac6_only_compose_root_imports_concrete_strategies`): (a) **bench exclusion** — `components//bench/**` files are skipped entirely, since benchmark / measurement code legitimately constructs production strategies via `runtime_root.*` factories (that is its job); (b) **self-registration carve-out** — an `ImportFrom` whose module starts with `gps_denied_onboard.runtime_root.` AND every imported name starts with `register_` is allowed (the registry pattern, e.g. `c5_state.gtsam_isam2_estimator.register` calling `runtime_root.state_factory.register_state_estimator`). Any other import from `runtime_root.*` inside a Layer-3 component (e.g. `build_*` factories, `compose_root`, `StrategyNotLinkedError`) remains a violation. This rule is the architectural contract paired with the AZ-270 lint; see `architecture.md` § Cross-Component Contract Surface for the rationale. ## Per-Component Mapping diff --git a/_docs/_process_leftovers/2026-05-24_az847_rule9_wording_followup.md b/_docs/_process_leftovers/2026-05-24_az847_rule9_wording_followup.md deleted file mode 100644 index 0d4e13e..0000000 --- a/_docs/_process_leftovers/2026-05-24_az847_rule9_wording_followup.md +++ /dev/null @@ -1,58 +0,0 @@ -# AZ-847 — Rule-9 wording follow-up (doc-only) - -**Timestamp**: 2026-05-24T10:05:00+03:00 -**Type**: Decision log + deferred doc-only work item -**Decision-maker**: agent (user skipped Choose A/B/C/D twice — implicit authorization to proceed with reasoned default) - -## Context - -AZ-847 widens `tests/unit/test_az270_compose_root.py::test_ac6_only_compose_root_imports_concrete_strategies` to enforce the full rule-9 allow-list documented in `_docs/02_document/module-layout.md`. Implementation surfaced two pre-existing rule-9 violations not anticipated by the task spec: - -1. **`components/c1_vio/bench/okvis2.py`** — top-level `from gps_denied_onboard.runtime_root.vio_factory import build_vio_strategy`. The bench module legitimately constructs production strategies via the composition root for measurement purposes (its job). -2. **`components/c4_pose/opencv_gtsam_estimator.py`**, **`components/c5_state/eskf_baseline.py`**, **`components/c5_state/gtsam_isam2_estimator.py`** — lazy `from gps_denied_onboard.runtime_root._factory import register__*` inside `def register():`. This is the central-registry self-registration pattern; components plug their `create()` callable into the airborne bootstrap registry by calling `register__*(_STRATEGY, create)`. - -Per AZ-847 Risk 1, both discoveries were surfaced to the user via Choose A/B/C/D (twice — once for bench/, once for self-registration). The user skipped both. Agent proceeded with principled defaults documented in the test docstring: - -- **bench/ exclusion**: `components//bench/**` paths are skipped at lint time. -- **Self-registration carve-out**: `ImportFrom` whose module starts with `gps_denied_onboard.runtime_root.` AND every imported name starts with `register_` is allowed. `build_*`, `compose_root`, error envelopes, etc. from `runtime_root.*` remain forbidden. - -Both carve-outs are layered defense — bench/ skip handles measurement-code legitimate `build_*` use; self-registration carve-out handles production registry pattern. Together they let the widened lint pass on the current codebase (zero violations after AZ-845 lands) while preserving rule-9's intent: components must not reach for OTHER components' concrete impls. - -## What this leftover defers - -This leftover defers the **rule-9 wording update** in `_docs/02_document/module-layout.md` (rule 9, Layout Rules section). The current rule wording lists 8 allow-list entries and ends with: "The composition root (`runtime_root/*`) is the single exception". This sentence does NOT cover: - -- `components//bench/**` files (rule-9 not applicable; measurement code). -- Lazy `register_*` self-registration imports from `runtime_root._factory` (registry pattern). - -Both carve-outs are documented in the test docstring at `tests/unit/test_az270_compose_root.py::test_ac6_only_compose_root_imports_concrete_strategies` (post-AZ-847). To stay honest, the rule-9 wording in `module-layout.md` should be extended to mention these two carve-outs — otherwise reviewers reading rule 9 in isolation will see an apparent mismatch between rule 9 (strict) and the lint (with documented carve-outs). - -## Replay condition - -This is **NOT** a tracker write blocker — no Jira call is being deferred. It's a **doc-only follow-up**: - -- Update rule 9 wording in `_docs/02_document/module-layout.md` to explicitly mention: - - `components//bench/**` is exempt from rule 9 (benchmark code legitimately constructs production strategies). - - `ImportFrom` of `register_*` callables from `runtime_root._factory` is allowed for the self-registration pattern. -- Surface this leftover at the start of the next /autodev invocation. The user can either: - - Authorize a tiny doc-only ticket (1 SP) to make the rule-9 update. - - Bundle the rule-9 update into the cycle-3 retrospective output (Step 17) as part of "carry-overs to next cycle". - - Choose a different remediation (e.g., refactor `register_*` helpers into `helpers/strategy_registry.py` so rule 9 stays strict — this was Option C of the original Choose). - -Until the wording is updated, the test docstring is the source of truth for the carve-outs. Reviewers should consult both rule 9 AND the docstring. - -## Why a leftover, not a new ticket - -- Per `.cursor/rules/tracker.mdc`: a leftover is appropriate for "deferred doc work" when the blocking factor is "user input on remediation strategy". The user skipped twice; the doc-only update is a small concern that fits the cycle-3 retro better than a standalone PBI. -- Per user complexity rule (`Create PBI with 2 or 3 points of complexity, could be 5`): this is < 1 SP work, below the typical PBI threshold. -- The technical work (the lint with documented carve-outs) is shipping in AZ-847; the doc-wording update is purely an audit-trail exercise that doesn't block any other work. - -## Replay action - -On next /autodev invocation, surface this leftover to the user as part of B1 (Process leftovers) and ask whether to: - -- (i) update rule-9 wording in module-layout.md now (~1 SP doc edit); -- (ii) bundle into cycle-3 retro output; -- (iii) refactor `register_*` helpers out of `runtime_root.*` into `helpers/strategy_registry.py` (option C of the original Choose; bigger scope). - -If user picks (i) or (ii), this leftover is consumed. If (iii), file a new ticket for the refactor and keep this leftover until the refactor lands and the carve-out is removed from the lint.