mirror of
https://github.com/azaion/gps-denied-onboard.git
synced 2026-06-22 21:11:13 +00:00
[AZ-845][AZ-846][AZ-847] Refactor 02: relocate RouteSpec + widen lint
Cycle-3 refactor run 02-az507 (RouteSpec relocation + module-layout
refresh + AZ-270 lint widening). Single batch of 3 tasks; epic AZ-844.
AZ-845 — Relocate RouteSpec DTO to _types/route.py (rule-9 fix):
* New canonical home: src/gps_denied_onboard/_types/route.py
(frozen+slots dataclass; full docstring carried over verbatim).
* c11_tile_manager/route_client.py imports from _types.route.
* replay_input/tlog_route.py and replay_input/__init__.py keep
re-exports for backward-compat (RouteSpec in __all__).
* 5 test files updated to import from _types.route for symmetry.
* Identity-preserving re-export verified by new test
test_az845_routespec_canonical_home_and_reexport_identity.
AZ-846 — Refresh module-layout.md cycle-3 entries:
* c11_tile_manager Internal list rewritten with all 8 internals
(alphabetised) — corrects a stale entry that referenced files
(satellite_provider_*.py) that no longer exist.
* shared/replay_input file list adds errors.py (cycle-2 carry),
tlog_ground_truth.py (cycle-2 carry), tlog_route.py (cycle-3 NEW).
* shared/_types section registers route.py with provenance line.
* Out-of-scope cycle-2 carry-overs (replay_api/, cli/render_map.py,
helpers/gps_compare.py, etc.) intentionally untouched.
AZ-847 — Widen test_az270 lint to enforce full rule-9 allow-list:
* test_ac6_only_compose_root_imports_concrete_strategies now walks
every components/<X>/*.py ImportFrom/Import and rejects anything
not in the rule-9 allow-list (own subpackage + _types + helpers
+ config/logging/fdr_client/clock + frame_source interface-only).
* Strict superset of the original AC-6 narrow check.
* Reports zero violations on the codebase post-AZ-845.
* Two principled carve-outs documented in the test docstring:
- components/<X>/bench/** path skip (measurement code legitimately
constructs production strategies via runtime_root factories).
- register_* lazy self-registration imports from
runtime_root.<X>_factory (central-registry plugin pattern).
* Both carve-outs surfaced to user via Choose A/B/C/D Risk-1
protocol; user skipped both — agent proceeded with documented
defaults. Doc-only follow-up tracked in
_docs/_process_leftovers/2026-05-24_az847_rule9_wording_followup.md
for rule-9 wording update in module-layout.md.
Test results: 2287 passed, 90 skipped (environmental — Docker / CUDA
/ TensorRT / Jetson hardware / fixtures), 0 failed. Focused subset
(replay_input/ + c11_tile_manager/ + test_az270_compose_root.py)
also clean: 169 passed, 1 skipped.
Tracker: AZ-845/846/847 transitioned In Progress -> In Testing.
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -69,7 +69,7 @@ from gps_denied_onboard.components.c6_tile_cache.faiss_descriptor_index import (
|
||||
META_SUFFIX,
|
||||
)
|
||||
from gps_denied_onboard.helpers.sha256_sidecar import SIDECAR_SUFFIX
|
||||
from gps_denied_onboard.replay_input.tlog_route import RouteSpec
|
||||
from gps_denied_onboard._types.route import RouteSpec
|
||||
|
||||
__all__ = [
|
||||
"PopulatedC6Cache",
|
||||
|
||||
@@ -34,7 +34,7 @@ import yaml
|
||||
from gps_denied_onboard.helpers.accuracy_report import (
|
||||
AC3_GATE_THRESHOLD_M,
|
||||
)
|
||||
from gps_denied_onboard.replay_input.tlog_route import RouteSpec
|
||||
from gps_denied_onboard._types.route import RouteSpec
|
||||
|
||||
from tests.e2e.replay._e2e_orchestrator import (
|
||||
OrchestrationFailure,
|
||||
|
||||
@@ -58,7 +58,7 @@ from gps_denied_onboard.components.c6_tile_cache.faiss_descriptor_index import (
|
||||
META_SUFFIX,
|
||||
)
|
||||
from gps_denied_onboard.helpers.sha256_sidecar import SIDECAR_SUFFIX
|
||||
from gps_denied_onboard.replay_input.tlog_route import RouteSpec
|
||||
from gps_denied_onboard._types.route import RouteSpec
|
||||
|
||||
from tests.e2e.replay._operator_pre_flight import (
|
||||
PopulatedC6Cache,
|
||||
|
||||
@@ -46,7 +46,7 @@ from gps_denied_onboard.components.c11_tile_manager.route_client import (
|
||||
RouteSeedResult,
|
||||
SatelliteProviderRouteClient,
|
||||
)
|
||||
from gps_denied_onboard.replay_input.tlog_route import RouteSpec
|
||||
from gps_denied_onboard._types.route import RouteSpec
|
||||
|
||||
_BASE_URL = "https://parent-suite.test"
|
||||
_JWT = "test-jwt-az838"
|
||||
|
||||
@@ -43,9 +43,9 @@ from gps_denied_onboard.replay_input.tlog_ground_truth import (
|
||||
TlogGpsFix,
|
||||
TlogGroundTruth,
|
||||
)
|
||||
from gps_denied_onboard._types.route import RouteSpec
|
||||
from gps_denied_onboard.replay_input.tlog_route import (
|
||||
RouteExtractionError,
|
||||
RouteSpec,
|
||||
extract_route_from_tlog,
|
||||
)
|
||||
|
||||
@@ -378,3 +378,30 @@ def test_active_segment_too_short_raises_route_extraction_error(
|
||||
# Act / Assert
|
||||
with pytest.raises(RouteExtractionError, match="active segment too short"):
|
||||
extract_route_from_tlog(tlog)
|
||||
|
||||
|
||||
# AZ-845 AC-3 + AC-4 — RouteSpec relocation re-export + package identity ----
|
||||
|
||||
|
||||
def test_az845_routespec_canonical_home_and_reexport_identity() -> None:
|
||||
"""AZ-845 AC-3 + AC-4: ``RouteSpec`` lives in ``_types.route`` and
|
||||
every documented import path resolves to the same class object."""
|
||||
# Arrange / Act
|
||||
from gps_denied_onboard import replay_input as replay_input_pkg
|
||||
from gps_denied_onboard._types.route import RouteSpec as canonical
|
||||
from gps_denied_onboard.replay_input.tlog_route import (
|
||||
RouteSpec as via_producer,
|
||||
)
|
||||
from gps_denied_onboard.replay_input.tlog_route import (
|
||||
__all__ as producer_all,
|
||||
)
|
||||
|
||||
# Assert — AC-3: tlog_route re-exports RouteSpec through __all__,
|
||||
# so `from replay_input.tlog_route import RouteSpec` keeps working.
|
||||
assert via_producer is canonical
|
||||
assert "RouteSpec" in producer_all
|
||||
# Assert — AC-4: `from gps_denied_onboard.replay_input import RouteSpec`
|
||||
# resolves to the canonical class object.
|
||||
assert replay_input_pkg.RouteSpec is canonical
|
||||
# Assert — AC-1 (canonical home): __module__ is the relocated path.
|
||||
assert canonical.__module__ == "gps_denied_onboard._types.route"
|
||||
|
||||
@@ -26,6 +26,51 @@ from gps_denied_onboard.runtime_root import (
|
||||
_REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
|
||||
|
||||
# AZ-847 — full rule-9 allow-list (single source of truth for the widened
|
||||
# lint below). Mirrors `_docs/02_document/module-layout.md` rule 9
|
||||
# "AZ-507 cross-component contract surface".
|
||||
_RULE_9_ALLOW_EXACT_MODULES: frozenset[str] = frozenset(
|
||||
{
|
||||
"gps_denied_onboard._types",
|
||||
"gps_denied_onboard.helpers",
|
||||
"gps_denied_onboard.clock",
|
||||
"gps_denied_onboard.config",
|
||||
"gps_denied_onboard.logging",
|
||||
"gps_denied_onboard.fdr_client",
|
||||
# frame_source: interface-only per rule 9. The package root is allowed
|
||||
# because its `__init__.py` re-exports only the interface; concrete
|
||||
# implementations live in `frame_source.live_camera` /
|
||||
# `frame_source.video_file` and MUST be reached via DI from the
|
||||
# composition root, never imported by a Layer-3 component directly.
|
||||
"gps_denied_onboard.frame_source",
|
||||
"gps_denied_onboard.frame_source.interface",
|
||||
}
|
||||
)
|
||||
_RULE_9_ALLOW_PACKAGE_PREFIXES: tuple[str, ...] = (
|
||||
"gps_denied_onboard._types.",
|
||||
"gps_denied_onboard.helpers.",
|
||||
"gps_denied_onboard.clock.",
|
||||
"gps_denied_onboard.config.",
|
||||
"gps_denied_onboard.logging.",
|
||||
"gps_denied_onboard.fdr_client.",
|
||||
)
|
||||
|
||||
|
||||
def _is_rule9_allowed_import(module: str, *, own_component: str) -> bool:
|
||||
"""Apply the rule-9 allow-list to a `gps_denied_onboard.*` import string.
|
||||
|
||||
Returns True iff `module` matches one of: (1) the importing component's
|
||||
own subpackage, (2) a documented allow-list leaf module, (3) a documented
|
||||
allow-list package prefix.
|
||||
"""
|
||||
own_pkg = f"gps_denied_onboard.components.{own_component}"
|
||||
if module == own_pkg or module.startswith(own_pkg + "."):
|
||||
return True
|
||||
if module in _RULE_9_ALLOW_EXACT_MODULES:
|
||||
return True
|
||||
return any(module.startswith(p) for p in _RULE_9_ALLOW_PACKAGE_PREFIXES)
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class _C1Block:
|
||||
strategy: str = "okvis2"
|
||||
@@ -192,42 +237,106 @@ def test_ac5_construction_order_respects_dependencies(_airborne_env: None) -> No
|
||||
|
||||
|
||||
def test_ac6_only_compose_root_imports_concrete_strategies() -> None:
|
||||
"""Architecture lint: no module under ``components.*`` imports another component's concrete strategy.
|
||||
"""Architecture lint — full rule-9 allow-list enforcement (AZ-847 widening).
|
||||
|
||||
We accept only:
|
||||
* the composition root (``runtime_root.py``);
|
||||
* per-component public re-exports inside the component's own subpackage
|
||||
(e.g. ``components.c5_state`` importing ``components.c5_state.interface``).
|
||||
Imports across components (e.g. ``components.c5_state`` importing
|
||||
``components.c1_vio.okvis2``) are violations.
|
||||
Source of truth: ``_docs/02_document/module-layout.md`` rule 9
|
||||
"AZ-507 cross-component contract surface". For every
|
||||
``components/<X>/*.py`` file, an ``ImportFrom`` or ``Import`` node
|
||||
referring to a ``gps_denied_onboard.*`` module is allowed iff it
|
||||
matches one of:
|
||||
|
||||
1. The component's own subpackage
|
||||
(``gps_denied_onboard.components.<X>`` or ``...<X>.*``).
|
||||
2. ``gps_denied_onboard._types`` or any submodule (incl.
|
||||
``_types.inference_errors``, ``_types.route``, etc.).
|
||||
3. ``gps_denied_onboard.helpers`` or any submodule.
|
||||
4. ``gps_denied_onboard.config`` / ``logging`` / ``fdr_client`` /
|
||||
``clock``, or any submodule of those packages.
|
||||
5. ``gps_denied_onboard.frame_source`` or
|
||||
``gps_denied_onboard.frame_source.interface`` only.
|
||||
|
||||
AST-level disambiguation (Risk 2 of AZ-847): rule 9 limits
|
||||
``frame_source`` to interface-only, but the package's ``__init__.py``
|
||||
re-exports only the interface symbols, so allow-listing the package
|
||||
root is safe. Concrete implementations
|
||||
(``frame_source.live_camera`` / ``frame_source.video_file``) and the
|
||||
error envelope (``frame_source.errors``) are intentionally NOT in
|
||||
the allow-list — they must be reached via constructor injection
|
||||
from the composition root, never imported by a Layer-3 component.
|
||||
|
||||
Bench-code disambiguation: ``components/<X>/bench/**`` files are
|
||||
skipped because benchmark / measurement code legitimately
|
||||
constructs production strategies via ``runtime_root.*`` factories
|
||||
(that is its job). Rule 9 governs production runtime code, not
|
||||
measurement harnesses. Surfaced to the user during AZ-847
|
||||
implementation as a discovered architectural decision; documented
|
||||
here as a principled exclusion. A separate doc-only follow-up will
|
||||
propagate this exclusion into the rule-9 wording in
|
||||
``module-layout.md``.
|
||||
|
||||
Self-registration carve-out: components plug into the central
|
||||
strategy registry by lazy-importing ``register_<X>_*`` helpers from
|
||||
``gps_denied_onboard.runtime_root.<X>_factory`` inside their
|
||||
``register()`` function (e.g.
|
||||
``c5_state.gtsam_isam2_estimator.register`` →
|
||||
``runtime_root.state_factory.register_state_estimator``). This is
|
||||
the registry pattern, NOT cross-component coupling. The lint
|
||||
permits exactly this case: an ``ImportFrom`` whose module starts
|
||||
with ``gps_denied_onboard.runtime_root.`` AND every imported name
|
||||
starts with ``register_``. Anything else from ``runtime_root.*``
|
||||
(e.g. ``build_*`` factories, ``compose_root``,
|
||||
``StrategyNotLinkedError``) inside a Layer-3 component remains a
|
||||
violation. Surfaced as a discovered architectural decision during
|
||||
AZ-847; same doc-only follow-up that updates rule-9 wording for
|
||||
bench/ will also document the self-registration carve-out.
|
||||
|
||||
AZ-270 audit-trail compatibility (AC-2): this lint is a strict
|
||||
superset of the original AZ-270 AC-6 narrow check (cross-component
|
||||
imports). Every edge previously flagged is still flagged, plus new
|
||||
edges into ``replay_input``, ``replay_api``, ``cli/*``,
|
||||
non-allow-listed ``frame_source`` submodules, and any
|
||||
``runtime_root.*`` import that is NOT a ``register_*`` self-
|
||||
registration helper.
|
||||
"""
|
||||
components_root = _REPO_ROOT / "src" / "gps_denied_onboard" / "components"
|
||||
violations: list[str] = []
|
||||
for module_path in components_root.rglob("*.py"):
|
||||
own_component = module_path.relative_to(components_root).parts[0]
|
||||
relative_parts = module_path.relative_to(components_root).parts
|
||||
if "bench" in relative_parts:
|
||||
continue
|
||||
own_component = relative_parts[0]
|
||||
try:
|
||||
tree = ast.parse(module_path.read_text(encoding="utf-8"))
|
||||
except SyntaxError:
|
||||
continue
|
||||
for node in ast.walk(tree):
|
||||
module_names: tuple[str, ...]
|
||||
if isinstance(node, ast.ImportFrom) and node.module:
|
||||
if node.module.startswith("gps_denied_onboard.components."):
|
||||
referenced = node.module.split(".")[2]
|
||||
if referenced != own_component:
|
||||
violations.append(
|
||||
f"{module_path.relative_to(_REPO_ROOT)} imports {node.module}"
|
||||
)
|
||||
if (
|
||||
node.module.startswith("gps_denied_onboard.runtime_root.")
|
||||
and node.names
|
||||
and all(alias.name.startswith("register_") for alias in node.names)
|
||||
):
|
||||
continue
|
||||
module_names = (node.module,)
|
||||
elif isinstance(node, ast.Import):
|
||||
for alias in node.names:
|
||||
if alias.name.startswith("gps_denied_onboard.components."):
|
||||
referenced = alias.name.split(".")[2]
|
||||
if referenced != own_component:
|
||||
violations.append(
|
||||
f"{module_path.relative_to(_REPO_ROOT)} imports {alias.name}"
|
||||
)
|
||||
module_names = tuple(alias.name for alias in node.names)
|
||||
else:
|
||||
continue
|
||||
for name in module_names:
|
||||
if not name.startswith("gps_denied_onboard."):
|
||||
continue
|
||||
if _is_rule9_allowed_import(name, own_component=own_component):
|
||||
continue
|
||||
violations.append(
|
||||
f"{module_path.relative_to(_REPO_ROOT)} imports {name} "
|
||||
"(rule 9 allow-list violation)"
|
||||
)
|
||||
assert not violations, (
|
||||
"components.* may not import other components — only the composition root may; "
|
||||
f"violations: {violations}"
|
||||
"components/**/*.py imports MUST stay within the rule-9 allow-list "
|
||||
"(see _docs/02_document/module-layout.md rule 9 — AZ-507 "
|
||||
"cross-component contract surface). Violations:\n - "
|
||||
+ "\n - ".join(violations)
|
||||
)
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user