fix(#499): preflight passed with 6 of 9 publish variables missing, and its disk floor was below the one it delegates to

tests/test_fao_delivery_runner.py

@@ -109,6 +109,37 @@ def test_it_refuses_rather_than_proceeding(self, result):
             "publish credentials. It would then have proceeded to spend GPU hours."
         )
 
+    def test_it_builds_the_publish_config_rather_than_counting_variables(self):
+        """The check that would have caught the real failure.
+
+        PredictionStoreConfig.from_environment() requires NINE variables — the three secrets
+        plus PROD_FORECASTS_BUCKET_ID/NAME, PROD_FORECASTS_COLLECTION_ID/NAME and
+        METADATA_DATABASE_ID/NAME. The first version of this preflight checked the three
+        secrets, so it would have reported ready with six of nine missing and the run would
+        have failed at the publish, after the whole roster trained.
+
+        Counting variables cannot be made correct by adding six more names either: the extra
+        can be absent, the endpoint unreachable, the key expired (#359 — 2026-11-17). Only
+        constructing the thing answers the question.
+        """
+        code = _code_only(FAO.read_text())
+        assert "PredictionStoreConfig" in code and "from_environment()" in code, (
+            "preflight must CONSTRUCT the publish config, not enumerate variable names"
+        )
+        assert "import appwrite" in code, (
+            "preflight must also confirm the appwrite extra is importable — variables can all "
+            "be set while the SDK that uses them is missing (#517)"
+        )
+
+    def test_it_tells_the_operator_the_variables_must_be_exported(self):
+        """pipeline-core stopped auto-loading a .env from the working directory (#346, C-177),
+        so a correct .env sitting beside the operator is not enough. Without this line the
+        failure looks like wrong credentials rather than unexported ones."""
+        code = _code_only(FAO.read_text())
+        assert re.search(r"set -a", code) and re.search(r"C-177|#346", FAO.read_text()), (
+            "preflight must say the nine variables need exporting, and name why"
+        )
+
     def test_it_names_the_missing_publish_secrets(self, result):
         out = result.stdout + result.stderr
         for var in ("APPWRITE_ENDPOINT", "APPWRITE_DATASTORE_PROJECT_ID",
@@ -130,6 +161,27 @@ def test_it_checks_conda_because_the_postprocessor_needs_it(self, result):
             "(tools/launcher/postprocessor.sh:72) and step 4 is the last step."
         )
 
+    def test_the_disk_floor_is_not_below_the_floor_it_delegates_to(self):
+        """Found by a falsification pass on the forecasting claim.
+
+        This script delegates to pod_run_model.sh eight times, and that script refuses below
+        40GB for ONE model. A floor below 40 here is a preflight that reports ready and then
+        refuses at model 3 — the exact opposite of why this mode exists. The first version
+        said 60GB for all eight, which was lower than the per-model transient for a run eight
+        times the size.
+        """
+        fao = _code_only(FAO.read_text())
+        model = _code_only((REPO / "tools" / "podrun" / "pod_run_model.sh").read_text())
+        delegated = re.search(r'AVAIL_GB" -ge (\d+)', model)
+        assert delegated, "pod_run_model.sh's disk floor moved; re-read it"
+        mine = re.search(r"DISK_FLOOR_GB=(\d+)", fao)
+        assert mine, "this script no longer declares DISK_FLOOR_GB"
+        assert int(mine.group(1)) >= int(delegated.group(1)), (
+            f"this orchestrator demands {mine.group(1)}GB for eight models while the script it "
+            f"delegates to demands {delegated.group(1)}GB for one. Preflight would pass and a "
+            "later model would refuse, after hours."
+        )
+
     def test_it_reports_every_problem_not_only_the_first(self, result):
         """A preflight that dies on the first missing thing costs a round trip per problem,
         on hardware billed by the second."""

tools/podrun/pod_run_fao_delivery.sh

@@ -126,19 +126,62 @@ if [ -s /root/.netrc ] && [ "$(stat -c %a /root/.netrc 2>/dev/null)" != "600" ];
     echo "  fixed: /root/.netrc was not 600"
 fi
 
-# The publish credentials. Three of the nine are real secrets; the rest are identifiers.
-# Checked HERE and not at first publish, because views-pipeline-core's own
-# PredictionStoreConfig claims to read these "once at startup and fail loud" and does not —
-# it is called from _build_datastore, i.e. after training (views-pipeline-core#557). Until
-# that moves, this is the only check that happens before the money is spent.
+# The publish credentials. NINE variables are required, not three — measured, not assumed:
+# PredictionStoreConfig.from_environment() names ENDPOINT, DATASTORE_PROJECT_ID,
+# DATASTORE_API_KEY, PROD_FORECASTS_BUCKET_ID, PROD_FORECASTS_BUCKET_NAME,
+# PROD_FORECASTS_COLLECTION_ID, PROD_FORECASTS_COLLECTION_NAME, METADATA_DATABASE_ID and
+# METADATA_DATABASE_NAME. Only the first three are secrets; the rest are identifiers, and a
+# missing identifier fails the publish exactly as hard as a missing secret.
+#
+# The first version of this block checked the three secrets only. It would have passed with six
+# of nine missing, and the run would have failed at the publish — after the entire roster
+# trained. The authoritative check is the CONSTRUCTION below; this loop survives only to give a
+# faster, friendlier message for the case an operator hits most often.
 for v in APPWRITE_ENDPOINT APPWRITE_DATASTORE_PROJECT_ID APPWRITE_DATASTORE_API_KEY; do
     eval "val=\${$v:-}"
     [ -n "$val" ] || note "\$$v — a publish secret; see reports/fao_delivery_runbook.md"
 done
+# They must be EXPORTED, not merely present in a .env beside you: pipeline-core stopped
+# auto-loading a .env from the working directory (#346, register C-177), because a library
+# reading whatever .env its caller happens to be standing in is what the Appwrite seam contract
+# §3 forbids. `set -a; . /root/.secrets; set +a` is the shape that works.
 case "${APPWRITE_DATASTORE_API_KEY:-}" in
     *[![:print:]]*) note "\$APPWRITE_DATASTORE_API_KEY contains a non-printable character — re-paste it" ;;
 esac
 
+# And then BUILD the thing, rather than concluding from three non-empty strings that it will
+# build. Variables being set is not the same as the store being constructible: the appwrite
+# extra can be missing, the endpoint can be unreachable, the key can be expired (#359: this one
+# expires 2026-11-17). Every one of those surfaces at `_build_datastore`, whose first caller is
+# the publish — i.e. after the entire roster has trained (views-pipeline-core#557).
+#
+# Suggested by the views-pipeline-core session, which pointed out this is exactly what #557
+# argues the manager should do itself, and that doing it by hand costs nothing until it does.
+if [ -x "$VENV/bin/python" ] && [ "$FAIL" = "0" ]; then
+    if PUBERR=$("$VENV/bin/python" - <<'PUBCHECK' 2>&1
+import sys
+try:
+    from views_pipeline_core.configs.prediction_store import PredictionStoreConfig
+except Exception as e:
+    sys.exit("cannot import PredictionStoreConfig: %s: %s" % (type(e).__name__, e))
+try:
+    cfg = PredictionStoreConfig.from_environment()
+except Exception as e:
+    sys.exit("PredictionStoreConfig.from_environment() failed: %s: %s" % (type(e).__name__, e))
+try:
+    import appwrite  # noqa: F401
+except Exception as e:
+    sys.exit("the appwrite extra is not installed: %s: %s" % (type(e).__name__, e))
+print("publish config builds")
+PUBCHECK
+    ); then
+        echo "  publish path: $PUBERR"
+    else
+        note "the publish path does not build — this WOULD have failed after the whole roster trained:
+      $PUBERR"
+    fi
+fi
+
 # conda, for the postprocessor leg ONLY. tools/launcher/postprocessor.sh uses
 # `conda shell.bash hook` / `conda create --prefix` / `conda activate`, while this pod builds
 # a uv venv — so the two legs need different interpreters and a pod can satisfy one and not
@@ -158,10 +201,32 @@ fi
 
 nvidia-smi -L >/dev/null 2>&1 || note "no GPU visible"
 AVAIL_GB=$(df -BG --output=avail "$ROOT" 2>/dev/null | tail -1 | tr -dc '0-9')
-# Eight forecasting runs plus the pooled ensemble, on one disk.
-[ "${AVAIL_GB:-0}" -ge 60 ] || note "only ${AVAIL_GB:-0}GB free on $ROOT; eight forecasts plus the pool need ~60GB"
+# DISK_FLOOR_GB, and the honest state of what it is based on.
+#
+# pod_run_model.sh refuses below 40GB for ONE model ("one model needs ~20GB"), and this script
+# delegates to it eight times — so 40 is a hard lower bound that will be re-checked at every
+# model whatever is written here. A floor BELOW the floor of the thing it delegates to is a
+# preflight that says "ready" and then refuses at model 3, which is the opposite of this
+# script's purpose. The first version of this check said 60 for all eight, which was exactly
+# that mistake: lower than the per-model transient for a run eight times the size.
+#
+# The retained component is an ESTIMATE and cannot be better than that yet: measured on this
+# machine, one model's calibration output is ~2.5GB per predictions directory, and NO
+# FORECASTING RUN HAS EVER COMPLETED ON THIS ROSTER, so the retained size of a forecast is
+# unmeasured. A forecast has one origin against calibration's 13, so it should be smaller —
+# "should be" is doing real work in that sentence.
+#
+# 40 transient + 8 x ~5GB retained, rounded up for the pooled ensemble, which is also
+# unmeasured. Revise this number from the first completed run rather than reasoning about it
+# again; that is the whole of the trigger.
+DISK_FLOOR_GB=80
+[ "${AVAIL_GB:-0}" -ge "$DISK_FLOOR_GB" ] || note "only ${AVAIL_GB:-0}GB free on $ROOT; eight forecasts plus the pool need >=${DISK_FLOOR_GB}GB (40GB is the per-model transient that pod_run_model.sh enforces on its own, eight times over, plus retained output)"
 
 if [ "$FAIL" = "1" ]; then
+    echo
+    echo "  Note: the nine publish variables must be EXPORTED into this shell, not just present"
+    echo "  in a file — pipeline-core no longer auto-loads a .env (#346, C-177). Try:"
+    echo "      set -a; . /root/.secrets; set +a"
     echo
     die "preflight failed — nothing above costs GPU time to fix. Fix them and re-run --preflight."
 fi