Skip to content

feat(delivery): S1 — one delivery path; refuse an incomplete launch config - #159

Merged
Polichinel merged 1 commit into
developmentfrom
feat/s1-delete-legacy-delivery-path
Jul 31, 2026
Merged

feat(delivery): S1 — one delivery path; refuse an incomplete launch config#159
Polichinel merged 1 commit into
developmentfrom
feat/s1-delete-legacy-delivery-path

Conversation

@Polichinel

Copy link
Copy Markdown
Collaborator

Closes #149 · Closes #145 · Epic #148 · Register C-63 closed, C-29 resolved

What this is

Not new work — a removal that was four days overdue. The frame-native contract path replaced the pandas path and won (run-0, 2026-07-27, FAO served since). The replaced path was kept behind a config fork "until run 0 proves the contract path live" (C-40) and never removed. D-11 warned exactly this: "If it lingers, three-way duplication becomes the permanent shape."

The defect it closes

Two independent config axes gave four delivery modes; production used one. Omitting either key silently selected the retired half.get() returning None because a launcher never mentioned the key is indistinguishable from a deliberate False. That is the inference ADR-003 forbids, in the repository that authored it.

before after
unfao/managers/unfao.py 636 lines 406
config forks 14 0, plus one refusal
pandas / PGMDataset in the manager yes gone

Deleted

The legacy _read_forecast_data body · _read_historical_data's pandas leg · _append_metadata · _delivery_description · _clip_observed_history · the legacy _save body (whose AppwriteConfig duplicated _unfao_appwrite_config verbatim) · LEGACY_FORECAST_FILTERS.

_prod_forecasts_datastore also loses its name_scoped parameter — it existed solely for the caller that no longer exists (ISP).

Added

unfao/launch_config.py — the launch declarations, asserted not inferred. Sibling of appwrite_env (that validates the environment, this the delivery mode), same fail-loud shape, dependency-light. A launcher omitting a key is refused by name and told there is no fallback — so nobody "fixes" it by setting False.

#145 closes with it. The silent uploads were in the legacy _save; the surviving path uploads only through _ContractStorePort, which already raises on a failed result — the guard written after run-0 stranded an invisible orphan (PR #132).

Tests

test_selection_guard.py rewritten to pin the retirement, not a golden string for deleted code. The guarantee moved rather than lapsed: the retired filter values now live in ADR-013's post-adoption record where they stay checkable, and a new test asserts §11.4's Hop-B clause still binds — retiring our reader must not be read as relaxing the consumer's obligation.

test_append_metadata.py deleted — it characterised a deleted method and its own docstring said "keep in lockstep with _append_metadata". The enricher behaviour it exercised is covered by test_enrichment.py.

test_launch_config.py added. Docstring maps in test_input_integrity_e2e.py corrected to the post-#149 tree.

Review findings, addressed in-commit

/review-diff returned HOLD. Fixed before pushing:

  1. launch_config raised without logging — ADR-008:48 requires both. New code violating a standing ADR in the exact dimension the story is about. Fixed, with tests asserting the log, not just the raise.
  2. _transform annotated -> list while returning None.
  3. The declaration check ran after _initialize_data_loader() — a refused config paid for loader construction. Reordered.

Registered C-71 rather than widening scope: appwrite_env has the identical ADR-008 gap (pre-existing, #134). launch_config was written to mirror it and inherited the flaw; fixing only the new one leaves the pair inconsistent, which is worth tracking explicitly.

Contract preservation

No file under wire/, delivery/ or fixtures/ appears in this diff — the ADR-013 byte-parity surface is untouched by construction, not merely by test result.

ruff clean · 303 passed, 41 xfailed · the 5 failures are the known local pyarrow 23.0.1-vs-pinned-16.1.0 byte-parity tests; CI authoritative.

🤖 Generated with Claude Code

…onfig

Closes #149 and #145; closes register C-63; resolves C-29. Epic #148.

The frame-native contract path replaced the pandas path and won — run-0 delivered
global-land 2026-07-27, FAO served since. The replaced path was kept behind a
config fork "until run 0 proves the contract path live" (C-40) and never removed.
D-11 warned that if it lingered the duplication would become permanent. This is
the removal, not new work.

Two independent axes (wire_contract, data_format) gave four delivery modes;
production used one; omitting either key silently selected the retired half,
because .get() returning None is indistinguishable from a deliberate False. That
is the inference ADR-003 forbids, in the repo that authored it.

  unfao/managers/unfao.py   636 -> 406 lines
  14 config forks           -> 0, plus one refusal
  pandas / PGMDataset       -> gone from the manager

Deleted: the legacy _read_forecast_data body, _read_historical_data's pandas leg,
_append_metadata, _delivery_description, _clip_observed_history, the legacy _save
body (whose AppwriteConfig duplicated _unfao_appwrite_config verbatim), and
LEGACY_FORECAST_FILTERS. _prod_forecasts_datastore lost its name_scoped parameter
— it existed only for the caller that no longer exists (ISP).

Added: unfao/launch_config.py — the launch declarations, asserted not inferred.
Sibling of appwrite_env (that one validates the environment, this one the
delivery mode), same fail-loud shape, dependency-light. A launcher that omits a
key is refused BY NAME and told there is no fallback, so nobody "fixes" it by
setting False.

#145's silent uploads go with the legacy _save: the surviving path uploads only
through _ContractStorePort, which already raises on a failed result — the guard
written after run-0 stranded an invisible orphan (PR #132).

Tests: test_selection_guard.py rewritten to pin the RETIREMENT rather than a
golden string for deleted code, re-pointed at ADR-013's post-adoption record so
the disjointness fact stays checkable, and asserting the Hop-B clause still binds
— retiring our reader must not read as relaxing the consumer's obligation.
test_append_metadata.py deleted (it characterised a deleted method; its docstring
said "keep in lockstep with _append_metadata"). test_launch_config.py added.
Docstring maps in test_input_integrity_e2e.py corrected to the post-#149 tree.

ADR-013: dated post-adoption entry recording the Hop-A retirement, the retired
filter values, and that §11.4's Hop-B clause is untouched.

Review findings addressed in-commit (/review-diff verdict was HOLD):
- launch_config raised without logging — ADR-008:48 requires both. Fixed, with
  tests asserting the log, not just the raise.
- _transform annotated -> list while returning None. Fixed.
- the declaration check ran after _initialize_data_loader; a refused config paid
  for loader construction. Reordered.
Registered C-71: appwrite_env has the identical ADR-008 gap (pre-existing, #134).
Left for scope discipline — the module this one mirrors is now inconsistent with
it, which is worth tracking rather than silently widening S1.

Wire contract untouched: no file under wire/, delivery/ or fixtures/ in the diff.
ruff clean; 303 passed, 41 xfailed, 5 known local pyarrow byte-parity failures
(CI authoritative).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Polichinel
Polichinel merged commit b2a4f99 into development Jul 31, 2026
4 checks passed
@Polichinel
Polichinel deleted the feat/s1-delete-legacy-delivery-path branch July 31, 2026 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant