Skip to content

Add principle 14: couple through data at rest, not run status - #452

Closed
JackKelly wants to merge 2 commits into
intervention-logfrom
coupling-through-data-at-rest
Closed

Add principle 14: couple through data at rest, not run status#452
JackKelly wants to merge 2 commits into
intervention-logfrom
coupling-through-data-at-rest

Conversation

@JackKelly

@JackKelly JackKelly commented Aug 7, 2026

Copy link
Copy Markdown
Member

Stacked on #451 — targets intervention-log, so review that one first. The diff shown here is just this change.

The idea

Our production jobs don't really depend on each other. live_forecasts doesn't care whether ecmwf_ens succeeded in the last 24 hours, or whether this hour's telemetry pull ran — it selects the freshest NWP run genuinely present as of its own init time and stamps the row with how stale that was. A failed ingest makes the 06:00 forecast slightly staler. It cannot make it late, and it cannot make it absent.

The common alternative is a chain of scheduled jobs — A at 06:00, B at 06:15, C at 06:30 — where B's real input is the event of A having run. That design can't distinguish "A failed" from "A is still running" from "A had nothing to do", so one bad morning takes out everything downstream for the rest of the day.

Why this needed writing down

The property was already implemented, and already argued for in one place — f001982 dropped the promoted_model lineage edge from live_forecasts for precisely this reason. But the docs never stated the general rule, and two places framed it as a defect awaiting a fix:

  • The power_time_series_and_metadata_schedule docstring called the :55 offset "a cheap mitigation, not a guarantee" and carried a TODO proposing a run-status sensor that would fire live_forecasts_job only once the ingest had succeeded.
  • why-dagster-not-airflow.md listed "Sensors / run-status coordination" against "ingest → forecast ordering" as planned work.

Both describe a gate, and a gate turns one failed upstream run into a missing forecast — the exact failure mode Inherent Stability exists to prevent. An implementer following that TODO would have removed a load-bearing stability property while believing they were hardening it.

live_forecasts wants to run at 00/06/12/18 regardless, so there is no acceleration to be gained there anyway. Sensors are still wanted on the ingest assets, where they avoid booting a Fargate task just to discover there's nothing new to fetch — which is what #324 is actually for. The comparison-table row now says that instead.

Changes

  • New principle 14 in design-principles.md, appended rather than inserted so no existing "principle N" citation moves.
  • New rule 11 in inherent-stability.md, marked as restating a principle; preamble updated from "three of the ten" to "four of the eleven", and principle 1's "ten rules" link with it.
  • schedules.py — TODO removed; both docstrings now say the decoupling is deliberate and why.
  • operations.md and why-dagster-not-airflow.md — reworded off the "not a guarantee / fix still to explore" framing.

The asymmetry worth keeping in view

The lineage edge stays. Dagster still knows live_forecasts depends on ecmwf_ens and power_time_series_and_metadata, which is what lets a developer materialise one asset on a laptop and have its inputs built for them, and what keeps the graph legible in the UI. What we decline is letting that edge become a runtime precondition in production. Both the principle and the rule call this out explicitly, since it's the part most likely to be "tidied up" by someone later.

Verification

  • uv run pytest — 395 passed, 1 skipped
  • uv run ruff check / ruff format --check / uv run --all-packages ty check — clean
  • uv run pymarkdown scan and uv run mkdocs build --strict — clean

@JackKelly JackKelly added documentation Improvements or additions to documentation inherent-stability Degrade gracefully rather than fail: the inherent-stability workstream labels Aug 7, 2026
@JackKelly JackKelly self-assigned this Aug 7, 2026
@JackKelly
JackKelly force-pushed the coupling-through-data-at-rest branch from cc01bcc to 9ad9f60 Compare August 7, 2026 08:01
JackKelly and others added 2 commits August 7, 2026 09:02
Production jobs read whatever is on disk when they run and record how
stale it was. None of them asks whether the job producing its input
succeeded, or ran at all. That is why a failed ingest makes the 06:00
forecast slightly staler rather than absent.

The property was already implemented and already argued for in one place
(dropping the promoted_model lineage edge), but the docs never stated it,
and two places framed it as a defect awaiting a fix: the TODO on
power_time_series_and_metadata_schedule proposed a run-status sensor that
would only fire live_forecasts once the ingest had succeeded, and the
Airflow comparison table listed "run-status coordination" as planned
work. Both would have introduced exactly the cascade the design avoids.

live_forecasts wants to run at 00/06/12/18 regardless, so there is no
acceleration to be had from a sensor there. Sensors are still wanted on
the ingest assets, where they save booting Fargate for a no-op fetch;
that is what #324 is actually for, and the table row now says so.

Note the asymmetry that makes this work: the lineage edge stays. It is
what builds a developer's inputs on a laptop. What it must never become
is a runtime gate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two real errors:

- Rule 11 said a missing forecast is "rung 0 of the ladder". Rung 0 is
  "everything fresh — best skill, narrowest bands", so the rule asserted
  that a missing forecast is the best possible outcome. A missing
  forecast is not on the ladder at all — not even rung 4, where the
  service still emits a very wide but true forecast.
- Principle 14 claimed a failed ingest "cannot make it absent". That is
  false today and this repo's own failure-modes table says so: with no
  NWP run on disk still covering the horizon, select_nwp_init_time
  raises and live_forecasts raises on an empty frame. #446 is the issue
  that will convert it into a weather-blind forecast. Asserting as
  achieved the property an open issue exists to build is exactly the
  over-claiming these pages are meant to resist, so the exception is now
  stated inline.

Also corrected: "records how stale that input was" was true of NWP
(nwp_init_time is on every row) but false of telemetry — PowerForecast
has no power-freshness column, so the claim is now specific about which
input is stamped and points at #424 for the other; the promoted_model
anecdote was a mis-citation, since that edge was removed for asset-graph
truthfulness rather than to avoid run-status coupling, and it now reads
as the related decision it actually is; and a lineage edge does not
build a developer's upstreams by itself, it lets them ask for the asset
and its upstreams together.

Principle 14 now names its relationship to principle 10 (same
blast-radius axis, one layer up), which is the closest neighbour and was
the source of the redundant feel; its duplicated *Without it:* clause is
cut back. Hard-coded rule counts are dropped from the preamble and from
principle 1, per 4674ac7's reasoning, and the schedules.py docstring
cites the rule by name and rendered URL rather than by list position.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@JackKelly
JackKelly force-pushed the coupling-through-data-at-rest branch from 9ad9f60 to 848c6b5 Compare August 7, 2026 08:02
@JackKelly
JackKelly deleted the branch intervention-log August 7, 2026 08:06
@JackKelly JackKelly closed this Aug 7, 2026
@JackKelly

Copy link
Copy Markdown
Member Author

Superseded by #454 — GitHub auto-closed this PR when its base branch intervention-log was deleted as part of merging #451. Same branch, same commits, rebased onto main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation inherent-stability Degrade gracefully rather than fail: the inherent-stability workstream

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant