mirror of
https://github.com/azaion/gps-denied-onboard.git
synced 2026-06-21 08:41:12 +00:00
chore: cumulative review batches 40-42 (PASS_WITH_WARNINGS)
5 findings: F1 (Medium / Maintainability) - _iso_now copies grew to 8 across c11 + c13 + c7, AZ-508 hygiene PBI no longer matches reality; F2-F5 (Low) - triplicated atomic-write JSON helpers, 4x duplicated SectorClassification enum (acknowledged by ADR-009), recurring "outcome=failure" prose vs typed-exception drift across the C11 trio, and an NFR-perf-cold-start near-miss that prompted PEP 562 lazy-import discipline in c12. None block the implement loop. Updated _autodev_state.md last_cumulative_review to batches_40-42. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -0,0 +1,319 @@
|
||||
# Cumulative Code Review — Batches 40–42 / Cycle 1
|
||||
|
||||
**Date**: 2026-05-13
|
||||
**Mode**: Cumulative (all 7 phases, emphasis on Phases 6 + 7)
|
||||
**Batches covered**: 40, 41, 42
|
||||
**Tasks covered**: AZ-316 (C11 HttpTileDownloader), AZ-320 (C11 IdempotentRetryTileUploader + c6 `increment_upload_attempts` extension + 0003 migration + `VotingStatus.UPLOAD_GIVEUP`), AZ-326 (C12 operator-tool CLI + SectorClassificationStore + freshness-table + EXIT_* constants), AZ-327 (C12 CompanionBringup + ParamikoSshSessionFactory + RemoteSidecarVerifier + error families)
|
||||
**Changed files in scope**: 26 production + 11 tests + 6 docs / migration / config (full list in §Scope below)
|
||||
**Verdict**: **PASS_WITH_WARNINGS**
|
||||
|
||||
## Scope
|
||||
|
||||
| Domain | Files (changed since `cumulative_review_batches_37-39_cycle1_report.md`) |
|
||||
|--------|--------------------------------------------------------------------------|
|
||||
| c11_tile_manager (production) | `_types.py` (added `SectorClassification`, `DownloadOutcome`, `TileSummary`, `DownloadRequest`, `DownloadBatchReport`); `errors.py` (added `ResolutionRejectionError`, `CacheBudgetExceededError`); `config.py` (download fields + `C11RetryConfig` + `disable_retry_decorator`); `interface.py` (real `TileDownloader` Protocol shape); `tile_downloader.py` (new — `HttpTileDownloader`, `request_hash`, journal); `idempotent_retry.py` (new — `IdempotentRetryTileUploader`); `__init__.py` (re-exports) |
|
||||
| c12_operator_tooling (production) | `_types.py` (new — `SectorClassification`, `CompanionAddress`, `ReadinessReport`, `ReadinessOutcome`, `CompanionUnreachableReason`, `AreaIdentifier`); `exit_codes.py` (new); `freshness_table.py` (new); `config.py` (new — `C12Config`, `C12CompanionConfig`, `HostKeyPolicy`); `errors.py` (new — `CompanionUnreachableError`, `ContentHashMismatchError`); `ssh_session.py` (new — `SshSession`/`SshSessionFactory` Protocols); `paramiko_ssh_session.py` (new — concrete adapter); `remote_sidecar_verifier.py` (new); `companion_bringup.py` (new); `cli.py` (new — Click app + 6 subcommands); `__main__.py` (new); `sector_classification_store.py` (new); `__init__.py` (PEP 562 lazy re-exports for heavy adapters); `flights_api/__init__.py` (PEP 562 lazy re-exports for httpx + bbox helpers) |
|
||||
| c6_tile_cache (production) | `_types.py` (added `VotingStatus.UPLOAD_GIVEUP`); `interface.py` (added `TileMetadataStore.increment_upload_attempts` w/ `NotImplementedError` default); `postgres_filesystem_store.py` (added `increment_upload_attempts` SQL, extended `_ALLOWED_VOTING_TRANSITIONS`, tightened `pending_uploads` filter) |
|
||||
| runtime_root (composition root) | `c11_factory.py` (added `build_tile_downloader` + `_C6DownloadAdapter`; `build_tile_uploader` now wraps with retry decorator by default + `clock` keyword); `c12_factory.py` (added `build_sector_classification_store`, `build_companion_bringup`, expanded `build_operator_tool` aggregator) |
|
||||
| FDR | `fdr_client/records.py` (registered `c11.upload.giveup`) |
|
||||
| Migrations | `db/migrations/versions/0003_c11_upload_attempts.py` (new — additive column + widened CHECK constraint, reversible) |
|
||||
| Tests | `c11_tile_manager/test_tile_downloader.py` (new, 14 tests); `c11_tile_manager/test_idempotent_retry.py` (new, 13 tests); `c11_tile_manager/test_protocol_conformance.py` (extended); `c12_operator_tooling/test_freshness_table.py`, `test_exit_codes.py`, `test_sector_classification_store.py`, `test_cli_help_and_logging.py`, `test_cli_build_cache.py`, `test_cli_console_script.py`, `test_companion_bringup.py`, `test_paramiko_factory_smoke.py` (all new); `c6_tile_cache/test_protocol_conformance.py` (extended `UPLOAD_GIVEUP` + new method on fakes); `test_ac5_alembic.py` (head-revision bumped to `0003_c11_upload_attempts`); `test_az272_fdr_record_schema.py` (added `c11.upload.giveup` payload) |
|
||||
| Docs / configuration | `_docs/02_document/contracts/c6_tile_cache/tile_metadata_store.md` (v1.3.0 — `increment_upload_attempts` method + I-8 update); `_docs/02_tasks/done/AZ-316_*.md`, `AZ-320_*.md`, `AZ-326_*.md`, `AZ-327_*.md`; `_docs/03_implementation/batch_40_cycle1_report.md`, `batch_41_cycle1_report.md`, `batch_42_cycle1_report.md`; `_docs/03_implementation/reviews/batch_40_review.md`, `batch_41_review.md`; `_docs/_autodev_state.md`; `_docs/_process_leftovers/2026-05-13_ruff_format_az489_pending.md`; `pyproject.toml` (added `paramiko>=3.4,<4.0` + `operator-tool` console script entry) |
|
||||
|
||||
## Summary
|
||||
|
||||
Three batches, four feature tasks, zero blocking findings. The C11 contract surface
|
||||
is now complete (downloader + uploader + retry decorator + signing key + flight gate
|
||||
all wired and tested), the C12 epic landed its first runnable end-to-end CLI shell
|
||||
plus the companion pre-flight verification path, and the c6 contract gained a clean
|
||||
forward-only `UPLOAD_GIVEUP` transition with an additive migration. The full
|
||||
unit-test suite reached **1494 passed / 80 environment-skipped** with zero
|
||||
failures across the changed surface; pre-existing perf microbenches (C8 covariance
|
||||
projector, C10 batcher overhead, C11 signing-key sign-p99) remain flaky on this dev
|
||||
host but were green or untouched in the batches under review.
|
||||
|
||||
The dominant architectural achievement of this window is the **maturation of the
|
||||
PEP 562 lazy-import discipline** for operator-tooling cold start. AZ-326 calls
|
||||
out a hard NFR (`operator-tool --help` ≤ 500 ms p99 on a developer laptop) plus an
|
||||
explicit constraint forbidding eager imports of `paramiko`, `httpx`, or
|
||||
`pymavlink` at CLI module load time. The implementation went well beyond the
|
||||
named libraries: the package `__init__.py` for both `c12_operator_tooling` and its
|
||||
nested `flights_api` package was converted to PEP 562 `__getattr__` lazy
|
||||
re-exports, deferring `HttpxFlightsApiClient` (httpx), `ParamikoSshSession*`
|
||||
(paramiko + cryptography), and `bbox_from_waypoints` / `takeoff_origin_from_flight`
|
||||
(numpy + pyproj). The `cli.py` module was switched to import from leaf
|
||||
`flights_api.errors` / `flights_api.interface` rather than the package, avoiding
|
||||
even the lazy machinery in the hot path. `python -X importtime` confirms zero
|
||||
heavy-dependency imports at CLI cold start; cold-start dropped from ~870 ms to
|
||||
<200 ms typical, <500 ms p99. The pattern is now an established primitive for
|
||||
any future operator-side CLI surface.
|
||||
|
||||
The recurring **Clock-vs-sleep injection** deviation (flagged in cumulative
|
||||
reports for batches 31–33 and 34–36, then noted in 37–39 as still-open for the
|
||||
C11 trio) is now **CLOSED**:
|
||||
|
||||
- Batch 41 lands the full `Clock` Protocol injection in
|
||||
`IdempotentRetryTileUploader` (uses both `monotonic_ns` and `time_ns`).
|
||||
- Batch 40 still injects a bare `Callable[[float], None]` sleep in
|
||||
`HttpTileDownloader` because the downloader genuinely never needs
|
||||
`monotonic_ns` / `time_ns` — this is now the documented exception, not the
|
||||
drift, and routes through `WallClock.sleep_until_ns` to honour the AZ-398
|
||||
invariant.
|
||||
- Batch 42's C12 components do not need a Clock at all (CLI subcommands use
|
||||
Click's exit machinery; CompanionBringup's only timing dependency is the
|
||||
paramiko connect-timeout).
|
||||
|
||||
Phase 6 (Cross-Task Consistency) verifies:
|
||||
|
||||
- **AZ-316 ↔ c6 freshness contract** — `HttpTileDownloader` recognises c6's
|
||||
`FreshnessRejectionError` by class-name match (`_is_freshness_rejection`)
|
||||
rather than importing it; the AZ-507 lint stays green and the spec's
|
||||
freshness-counter behaviour is exercised end-to-end via the
|
||||
`_C6DownloadAdapter` in `runtime_root/c11_factory.py`.
|
||||
- **AZ-320 ↔ c6 voting-state contract** — `IdempotentRetryTileUploader` reaches
|
||||
`UPLOAD_GIVEUP` via a string constant rather than the c6 enum;
|
||||
`PostgresFilesystemStore.update_voting_status` coerces strings via
|
||||
`VotingStatus(status)`; the new transitions
|
||||
(`PENDING → UPLOAD_GIVEUP`, `TRUSTED → UPLOAD_GIVEUP`) are added to
|
||||
`_ALLOWED_VOTING_TRANSITIONS` and the contract Invariant I-8 was updated in
|
||||
the same batch (v1.3.0). `pending_uploads` SQL excludes the new state so a
|
||||
re-run does not re-pick give-up tiles.
|
||||
- **AZ-326 ↔ AZ-327 sector classification + freshness table** — `cli.py`'s
|
||||
`set-sector` subcommand drives `SectorClassificationStore` (AZ-326);
|
||||
`freshness_threshold_months(SectorClassification)` is reused by the
|
||||
`build-cache` subcommand (AC-NEW-6); both AC IDs surface in `--help`. No
|
||||
duplication of either helper across the C12 internals.
|
||||
- **AZ-326 ↔ AZ-489 flights-api contract** — the build-cache subcommand
|
||||
resolves the `FlightDto` either online (`fetch_flight`) or offline
|
||||
(`load_flight_file`) and forwards to the orchestrator (sibling AZ-328 not yet
|
||||
shipped); the consumer-side seam matches what AZ-489 froze in its v1.0.0
|
||||
contract. Switching `cli.py` to leaf-module imports left the AZ-489
|
||||
unit-test surface intact (28 tests pass against the lazy `__getattr__` package).
|
||||
- **AZ-327 ↔ AZ-280 sidecar contract** — `RemoteSidecarVerifier` runs
|
||||
`sha256sum` + `cat` over SSH to compare against the `.sha256` sidecar shape
|
||||
AZ-280 documents. Engine bytes never reach the operator workstation.
|
||||
- **Migration head + Alembic test consistency** — Batch 41's 0003 migration
|
||||
bumps the head revision; `tests/unit/test_ac5_alembic.py` was updated in
|
||||
the same batch to assert `0003_c11_upload_attempts`. No drift between
|
||||
migration files and test expectations.
|
||||
- **FDR record-kind registry** — Batch 41's `c11.upload.giveup` is the only
|
||||
new payload kind across all three batches; registered in
|
||||
`fdr_client/records.py::KNOWN_PAYLOAD_KEYS` and matched by a mock payload
|
||||
in `tests/unit/test_az272_fdr_record_schema.py`.
|
||||
|
||||
Phase 7 (Architecture Compliance):
|
||||
|
||||
- **Layer direction**: every changed `components/*.py` file imports only from
|
||||
`_types/*`, `helpers/*`, `config`, `logging`, `clock`, `fdr_client`, or
|
||||
third-party libs (httpx, paramiko, click) — all Layer 1 or lower. No
|
||||
upward imports.
|
||||
- **Public API respect**: zero `from gps_denied_onboard.components.<other>`
|
||||
imports inside any `components/*.py` file changed in this window. The
|
||||
AZ-507 / ADR-009 cross-component-contract-surface rule is honoured. The
|
||||
AZ-270 lint
|
||||
(`tests/unit/test_az270_compose_root.test_ac6_only_compose_root_imports_concrete_strategies`)
|
||||
passes.
|
||||
- **No new cyclic module dependencies**: verified via `python -c "import …"`
|
||||
+ `importlib.import_module` round-trip across all 57 public names exposed
|
||||
by `c12_operator_tooling/__init__.py`. The PEP 562 lazy `__getattr__` does
|
||||
not introduce a cycle because heavy modules are resolved at first
|
||||
attribute access, not at package import.
|
||||
- **Duplicate symbols across components**: see Findings F1 + F2 + F3
|
||||
below — `_iso_now`-style helpers, atomic-write helpers, and the
|
||||
`SectorClassification` enum each grew their copy count this window. F3 is
|
||||
acknowledged-by-design (consumer-side cut per ADR-009); F1 + F2 are real
|
||||
hygiene drift that the existing AZ-508 PBI no longer fully covers.
|
||||
- **Cross-cutting concerns**: lazy-import discipline is the only
|
||||
cross-cutting primitive added this window; lives in
|
||||
`c12_operator_tooling/__init__.py` + `flights_api/__init__.py` only.
|
||||
Future operator-side CLI surfaces should adopt the same pattern;
|
||||
documented in the batch report and in this review's Summary.
|
||||
|
||||
## Findings
|
||||
|
||||
| # | Severity | Category | File:Line | Title |
|
||||
|---|----------|----------|-----------|-------|
|
||||
| 1 | Medium | Maintainability | `c11_tile_manager/{tile_downloader.py:883, idempotent_retry.py:69, tile_uploader.py:739}`, `c13_fdr/{writer.py:70, cap_policy.py:35}`, `c7_inference/{onnx_trt_ep_runtime.py:169, thermal_publisher.py:343}` (vs. `c6_tile_cache/_timestamp.py:19` — the consolidated helper) | `_iso_now`-style helpers grew to 8 copies; AZ-508 hygiene PBI no longer matches reality |
|
||||
| 2 | Low | Maintainability | `c11_tile_manager/tile_downloader.py:219`, `c12_operator_tooling/sector_classification_store.py:156`, `c10_provisioning/manifest_builder.py:546+561` | Atomic-write JSON helpers triplicated; no shared `helpers/atomic_write.py` |
|
||||
| 3 | Low | Architecture (acknowledged-by-design per ADR-009) | `c6_tile_cache/_types.py:86`, `c10_provisioning/interface.py:59`, `c11_tile_manager/_types.py:183`, `c12_operator_tooling/_types.py:33` | `SectorClassification` enum duplicated to 4 components; intended per AZ-507 consumer-side cut, but the count is worth tracking |
|
||||
| 4 | Low | Maintainability | `_docs/02_tasks/done/AZ-316_*.md`, `AZ-319_*.md`, `AZ-320_*.md` (specs say "outcome = failure"); `c11_tile_manager/{tile_downloader.py, tile_uploader.py, idempotent_retry.py}` (impls raise) | Recurring "spec prose says `outcome=failure`, implementation raises typed exception" deviation across the C11 trio |
|
||||
| 5 | Low | Performance | `c12_operator_tooling/__init__.py`, `c12_operator_tooling/flights_api/__init__.py`, `c12_operator_tooling/cli.py` | NFR-perf-cold-start was a near-miss (~870 ms → <500 ms p99 only after PEP 562 conversion); operator-side discipline now established but not yet codified in `coderule.mdc` |
|
||||
|
||||
### Finding Details
|
||||
|
||||
**F1 — `_iso_now` helpers grew to 8 copies (Medium / Maintainability)**
|
||||
|
||||
- Locations:
|
||||
- `c11_tile_manager/tile_downloader.py:883` (Batch 40 — new)
|
||||
- `c11_tile_manager/idempotent_retry.py:69` (Batch 41 — new)
|
||||
- `c11_tile_manager/tile_uploader.py:739` (pre-existing)
|
||||
- `c13_fdr/writer.py:70` (pre-existing)
|
||||
- `c13_fdr/cap_policy.py:35` (pre-existing)
|
||||
- `c7_inference/onnx_trt_ep_runtime.py:169` (pre-existing)
|
||||
- `c7_inference/thermal_publisher.py:343` (pre-existing)
|
||||
- vs. the consolidated `c6_tile_cache/_timestamp.py:19::iso_ts_now` (the
|
||||
only canonical helper today)
|
||||
- Description: at the last cumulative review (batches 34–36), the count was
|
||||
"down to 2 active copies in c7" with c6 already consolidated. This window
|
||||
added two more copies via the new C11 download + retry tasks. AZ-508 in
|
||||
`_docs/02_tasks/todo/` exists as the project-wide hygiene PBI for this
|
||||
consolidation but has not been picked up; its task spec also no longer
|
||||
matches reality (it lists modules that no longer carry the helper, and is
|
||||
silent on the new C11 sites).
|
||||
- Suggestion: refresh the AZ-508 spec to enumerate all 7 current
|
||||
duplicate-helper sites + the canonical `c6_tile_cache/_timestamp.iso_ts_now`
|
||||
target, raise its priority to land before any further C11/C12/C13 task
|
||||
introduces another copy, and consider promoting the canonical helper
|
||||
from `c6_tile_cache/_timestamp.py` to a cross-component
|
||||
`helpers/timestamps.py` so consumer-side cuts can reach it without
|
||||
importing from c6.
|
||||
- Severity rationale: Medium because the count is now 8 (above the
|
||||
"manageable drift" threshold) and at least three more in-flight tasks in
|
||||
the C11 / C12 / C13 epics will likely add copies if AZ-508 stays unpicked.
|
||||
|
||||
**F2 — Atomic-write JSON helpers triplicated (Low / Maintainability)**
|
||||
|
||||
- Locations:
|
||||
- `c10_provisioning/manifest_builder.py:546` (`_atomic_write_manifest`)
|
||||
- `c10_provisioning/manifest_builder.py:561` (`_atomic_write_signature`)
|
||||
- `c11_tile_manager/tile_downloader.py:219` (`_atomic_write_json`)
|
||||
- `c12_operator_tooling/sector_classification_store.py:156` (`_atomic_write`)
|
||||
- Description: every site implements the same write-temp +
|
||||
`os.replace` + `fsync` + parent-directory `fsync` pattern. Each was
|
||||
written from scratch citing "we already use this pattern in module X";
|
||||
none of them imports from a shared helper. Promoting the helper to
|
||||
`src/gps_denied_onboard/helpers/atomic_write.py` would let consumer-side
|
||||
cuts reach it without crossing component boundaries (helpers/ is at
|
||||
Layer 1 alongside `_types/`).
|
||||
- Suggestion: open a hygiene PBI to extract `helpers/atomic_write.py` and
|
||||
migrate the four call sites in one PR. Tests at the helper level can
|
||||
then own the AC-5 atomic-under-crash invariant; the call sites collapse
|
||||
to a one-line `helpers.atomic_write_bytes(path, payload)`.
|
||||
- Severity rationale: Low because each existing copy is correct in
|
||||
isolation; the drift is repeated boilerplate, not divergent semantics.
|
||||
|
||||
**F3 — `SectorClassification` enum duplicated 4× (Low / Architecture, by-design)**
|
||||
|
||||
- Locations:
|
||||
- `c6_tile_cache/_types.py:86` — original / canonical
|
||||
- `c10_provisioning/interface.py:59` (Batch 35 — AZ-325 mirror)
|
||||
- `c11_tile_manager/_types.py:183` (Batch 40 — AZ-316 mirror)
|
||||
- `c12_operator_tooling/_types.py:33` (Batch 42 — AZ-326 mirror)
|
||||
- Description: every consumer that reasons about the operator-set
|
||||
classification declares its own `class SectorClassification(str, Enum)`
|
||||
with the same two values (`active_conflict`, `stable_rear`). Because all
|
||||
copies inherit `str`, value equality holds at boundaries and `c6`'s
|
||||
enum coerces incoming strings via the standard `Enum(value)` ctor.
|
||||
This is the AZ-507 / ADR-009 consumer-side cut by design — flagged
|
||||
here only because the count keeps climbing (every new component touching
|
||||
sector classification adds the 5th, 6th, … mirror).
|
||||
- Suggestion: consider promoting `SectorClassification` to a shared
|
||||
`_types/sector_classification.py` so consumers can `from
|
||||
gps_denied_onboard._types.sector_classification import …` instead of
|
||||
hand-mirroring. ADR-009 already permits `_types/*` as a cross-component
|
||||
contract surface; this is the precedent. Tracker-only suggestion (no
|
||||
code change required this cycle).
|
||||
- Severity rationale: Low because the duplication is intentional and
|
||||
equivalence is preserved by the `str` Enum design.
|
||||
|
||||
**F4 — "outcome=failure" spec drift across C11 trio (Low / Maintainability)**
|
||||
|
||||
- Locations:
|
||||
- `_docs/02_tasks/done/AZ-316_c11_tile_downloader.md` (spec: "outcome =
|
||||
failure"; impl: `raise CacheBudgetExceededError | SatelliteProviderError
|
||||
| RateLimitedError`)
|
||||
- `_docs/02_tasks/done/AZ-319_c11_tile_uploader.md` (spec: "outcome =
|
||||
failure"; impl raises typed exceptions)
|
||||
- `_docs/02_tasks/done/AZ-320_c11_idempotent_retry.md` (decorator passes
|
||||
the underlying exception through)
|
||||
- Description: the spec prose for the C11 download + upload + retry trio
|
||||
consistently uses "outcome = failure" as a return value, but the
|
||||
implementations raise typed exceptions per the contract's exception
|
||||
matrix. The behavioural delta is documented in each batch's per-batch
|
||||
review (Batch 39 F1, Batch 40 F1, Batch 41 F2 cluster). The exception
|
||||
path still flushes the journal / increments the counters / writes the
|
||||
partial report, so downstream auditors can distinguish failure shapes
|
||||
without reading the exception trace — the only artefact that drifted
|
||||
is the prose in the original task specs.
|
||||
- Suggestion: open a single hygiene PBI to refresh the three task specs
|
||||
+ the `DownloadOutcome` / `UploadOutcome` enum docstrings to align on
|
||||
"exceptions for hard failure; `partial` for tile-level rejection".
|
||||
This closes the recurring deviation in one PR rather than re-flagging
|
||||
it on every C11 batch review.
|
||||
- Severity rationale: Low because the contract is stable (exception
|
||||
matrix is the source of truth and the tests assert against it);
|
||||
drift is documentation-only.
|
||||
|
||||
**F5 — NFR-perf-cold-start near-miss + lazy-import discipline not codified (Low / Performance)**
|
||||
|
||||
- Locations:
|
||||
- `c12_operator_tooling/__init__.py` (PEP 562 `__getattr__`)
|
||||
- `c12_operator_tooling/flights_api/__init__.py` (PEP 562 `__getattr__`)
|
||||
- `c12_operator_tooling/cli.py` (leaf-module imports for hot path)
|
||||
- Description: the AZ-326 NFR ≤500 ms p99 was at ~870 ms with a naive
|
||||
package layout (eager re-exports of `HttpxFlightsApiClient`,
|
||||
`ParamikoSshSession*`, and the `bbox` helpers transitively pulled in
|
||||
paramiko + httpx + cryptography + numpy + pyproj + cv2 + gtsam). The
|
||||
Batch 42 fix went well beyond the spec's named libraries (which only
|
||||
call out paramiko / httpx / pymavlink) — numpy / pyproj / cv2 / gtsam
|
||||
were just as severe, dragged in by `flights_api.bbox` →
|
||||
`helpers.wgs_converter`. The fix lands in three places and depends on
|
||||
PEP 562 awareness from anyone who touches the package init in future.
|
||||
- Suggestion: add a one-paragraph rule in `coderule.mdc` (or a new
|
||||
focused `cli-cold-start.mdc`) that any operator-tooling package
|
||||
`__init__.py` MUST keep heavy adapters behind PEP 562
|
||||
`__getattr__`, and that CLI entry-point modules MUST import via leaf
|
||||
modules rather than package init. Future C12 sibling tasks (AZ-328,
|
||||
AZ-329, AZ-330) will each add a service that could regress this NFR
|
||||
if the rule is not codified.
|
||||
- Severity rationale: Low because the NFR currently passes; the risk
|
||||
is recurrence on future task additions if the discipline is not
|
||||
written down.
|
||||
|
||||
## Verdict & Action Map
|
||||
|
||||
**PASS_WITH_WARNINGS** — no Critical or High findings; per the implement
|
||||
skill's auto-fix matrix, all five findings escalate to the user as Medium
|
||||
(F1) or Low (F2-F5) Maintainability / Performance / Architecture entries
|
||||
that do NOT block batch close-out. Recommended actions in priority order:
|
||||
|
||||
1. **F1 (Medium)** — refresh AZ-508 spec and pull it into the next
|
||||
ready batch before any further C11 / C12 / C13 task lands. The drift
|
||||
is now visible across two consecutive cumulative reviews and grew
|
||||
this window.
|
||||
2. **F4 (Low)** — open a single doc-only PBI to align the three C11 spec
|
||||
prose blocks with their typed-exception contracts (one PR, no code
|
||||
change). Closes a recurring per-batch finding noise.
|
||||
3. **F5 (Low)** — codify the lazy-import discipline as a rule before
|
||||
AZ-328 / AZ-329 / AZ-330 are picked.
|
||||
4. **F2 (Low)** — extract `helpers/atomic_write.py` (one PR, four
|
||||
call-site migrations + a helper-level test for the under-crash
|
||||
invariant).
|
||||
5. **F3 (Low)** — tracker-only suggestion to consolidate
|
||||
`SectorClassification` into `_types/`; no code change unless a 5th
|
||||
component would otherwise need it.
|
||||
|
||||
No `_docs/02_document/architecture_compliance_baseline.md` exists, so
|
||||
no Baseline Delta section is emitted.
|
||||
|
||||
## Test posture
|
||||
|
||||
- Full unit suite: **1494 passed / 80 skipped** (skips are all
|
||||
pre-existing environment gates: Docker compose for c6 Postgres tests,
|
||||
CUDA / TensorRT / Tier-2 Jetson hardware for c7, `actionlint` for one
|
||||
GHA-lint test).
|
||||
- Pre-existing perf microbench failures on this dev host (C8 covariance
|
||||
projector, C10 batcher overhead, C11 signing-key sign-p99) are
|
||||
**outside the changed surface** and are documented in batches 40 + 41
|
||||
per-batch reviews. They were not re-investigated in this cumulative
|
||||
pass; the recommendation in those reviews stands (re-bench on
|
||||
reference hardware before re-classifying).
|
||||
|
||||
## Next batch
|
||||
|
||||
Batch 43 candidates (per `_docs/02_tasks/_dependencies_table.md` after
|
||||
AZ-326 + AZ-327 closed): AZ-328 (C12 build-cache orchestrator) is the
|
||||
natural next consumer — it depends on AZ-326 + AZ-327 + AZ-489 (all
|
||||
ready) plus AZ-321 / AZ-322 / AZ-323 (already done). The implement
|
||||
skill should compute the next batch from the dependencies table at the
|
||||
start of the next loop.
|
||||
@@ -13,4 +13,4 @@ retry_count: 0
|
||||
cycle: 1
|
||||
tracker: jira
|
||||
last_completed_batch: 42
|
||||
last_cumulative_review: batches_37-39
|
||||
last_cumulative_review: batches_40-42
|
||||
|
||||
Reference in New Issue
Block a user