Skip to content

drift-check.yml: a refused refresh entry is not retried until some canonical copy changes upstream, and the run goes green meanwhile #20

Open

Description

@mmcky

Follow-up from the review of #14.

What happens

tools/promote --refresh picks its targets by pin, not by text. It re-resolves every ledger entry whose canonical series' pin in sync/state.yml differs from the entry's recorded promoted_at pin (refresh_targets, tools/promote:1066-1125). So whenever a series' pin moves, every entry of that series is a target, including entries whose canonical copy did not change (pin-only targets). If one of those entries has a same-name copy that was edited to differ, promote refuses it, the refresh pull request lists it under "Not refreshed", and the run is red. That much is as designed.

The retry is the problem. The refresh job runs only when drift-check reports a refresh finding: if: needs.check.outputs.refresh_needed == 'true' && github.event_name != 'pull_request' (.github/workflows/drift-check.yml:120). refresh_needed is summary["refresh"] > 0 (tools/drift-check:819, written to $GITHUB_OUTPUT at :747), and the only refresh findings are canonical-moved (a canonical lecture's text changed since promotion, :346-351) and asset-moved (:488-493). The refused entry has neither, because its canonical copy did not change: drift-check reports only a diverged drift finding for it (:403-415). Once the refresh pull request for the other entries merges, the next night has no refresh finding, so the refresh job is skipped and the run is green. The entry is still refused, and still behind its pin.

What follows:

  • The run is red only until the partial refresh pull request merges. After that it is green, with nothing settled.
  • A sync/canonical.yml line added as the "Not refreshed" note asks is not applied. The entry stays non-interim, the ledger keeps listing the edited copy as a source, and its diverged drift issue stays open.
  • The retry, and with it the map line, waits until the canonical copy of some other pool lecture or asset changes upstream, in any series. Only that sets refresh_needed, and the refresh it triggers retries every entry behind its pin. A pin move without a text change does not trigger it. Neither does a manual workflow_dispatch run, since the refresh job is gated on the same output.
  • If the refusal came from a same-name copy that appeared after promotion (a series that did not hold the lecture then), drift-check never sees that copy. After the merge it is clean, exit 0, with no drift issue either. (Checked the same way, with intro gaining a different amss.md while advanced edits another lecture.)

#14 documents this case. The workflow header (drift-check.yml:21-33) and the refresh pull request's note (drift-check.yml:194) say that an entry refused when only its series' pin moved is retried only when some canonical copy next moves. An entry whose canonical copy moved is different: it is refused, and the run is red, every night until it is settled, as the README also says (README.md:101). The question here is whether the pin-only case is good enough: its red run lasts only until the partial pull request merges, and a settlement waits on unrelated upstream edits.

Line numbers are at #14's head as it merges, with #17 included.

Reproduction (with the tools/tests harness)

Each night follows the workflow: the check job passes when drift-check exits 0 or 1, and the refresh job runs only if drift-check's refresh_needed step output is true. Saved as tools/tests/test_issue.py, this passes on #14's head, with #17 included:

from conftest import lecture_text


def nightly_check(repo, tmp_path, monkeypatch):
    """drift-check as the check job runs it; returns its refresh_needed step output."""
    out = tmp_path / "github_output"
    out.write_text("")
    monkeypatch.setenv("GITHUB_OUTPUT", str(out))
    assert repo.drift_check().returncode in (0, 1)  # the check job passes
    return dict(l.split("=", 1) for l in out.read_text().splitlines())["refresh_needed"]


def test_a_refused_pin_only_target_is_not_retried(make_repo, tmp_path, monkeypatch):
    repo = make_repo(["advanced", "dp-test"])
    for series in ("advanced", "dp-test"):
        repo.lecture(series, "amss", lecture_text("AMSS"))
        repo.lecture(series, "calvo", lecture_text("Calvo"))
    assert repo.promote("amss", "calvo").returncode == 0
    # dp-test edits its copy of amss; advanced edits calvo, not amss
    repo.lecture("dp-test", "amss", lecture_text("AMSS", "Edited in dp-test only."))
    repo.move_pin("dp-test")
    repo.lecture("advanced", "calvo", lecture_text("Calvo", "Fixed upstream."))
    repo.move_pin("advanced")

    # night 1: calvo moved, so the refresh job runs; amss is a target too and is refused
    assert nightly_check(repo, tmp_path, monkeypatch) == "true"
    assert repo.promote("--refresh").returncode == 1  # red; the PR (calvo only) merges

    # night 2: amss would still be refused, but nothing sets refresh_needed: green
    assert nightly_check(repo, tmp_path, monkeypatch) == "false"
    assert "[amss] FAILED" in repo.promote("--refresh", "--dry-run").stderr

    # the operator names the winner, as the refresh PR says; night 3 is still skipped
    repo.write_canonical_map({"amss": "advanced"})
    assert nightly_check(repo, tmp_path, monkeypatch) == "false"
    assert "interim" not in repo.entry("amss")

    # control: the same refresh, run by hand, applies the map at once
    assert repo.promote("--refresh").returncode == 0
    assert repo.entry("amss")["interim"] is True
cd tools/tests && uv run -q --with pytest --with pyyaml python -I -m pytest -q -p no:cacheprovider test_issue.py

On night 1, promote --refresh exits 1 with:

[amss] FAILED: amss.md differs across series (same: advanced:b9eb1b772814; different: dp-test:5fa6d8053356) — a same-named file must be one lecture: reconcile or rename it upstream, or name the canonical series in sync/canonical.yml

On night 2, after the merge, drift-check exits 1 with this single finding, and its step outputs skip the refresh job:

DRIFT — a copy diverged from its canonical home (each becomes an issue under --issues) (1)
  [diverged] lectures/amss.md: the dp-test copy lectures/amss.md no longer matches its canonical home in advanced

violations=0
drift=1
refresh=0
info=0
refresh_needed=false

When it bites

Nothing is affected today. On the committed data, drift-check is clean. A refresh would target 41 entries (every advanced and intermediate entry, and one intro entry), all of them pin-only, and would refuse none: promote --refresh --exclude dp jax --dry-run reports 41 promoted, 0 failed.

It bites the first time an entry's second recorded copy is edited. Today 25 of the 41 targets have one (18 advanced, 6 intermediate, 1 intro), all in dp-test, the sandbox. The drift-check demo of 2026-09-07 edited exactly such a copy (amss, #11). All 41 entries already lag their pins, so the next refresh run, whatever triggers it, refuses the edited entry and the sequence above follows. The whole-estate promotion (QuantEcon/project-monorepo#24) and dropping jax from PROMOTE_EXCLUDE will give many more entries a second holder.

The wait for a retry depends on unrelated upstream edits, and it can be long. After the last refresh pull request (#9, 2026-09-07), the refresh job was skipped on all 32 nightly runs up to 2026-10-08, although the pins moved on 11 of those days. Meanwhile nothing in the pool is wrong: the entry's canonical copy did not change, so its pool copy is current. What fails is the promised red run, and the settlement is left waiting.

This is new with #14. On main, the same refresh exits 0 and records the edited copy as divergent and the entry as interim, so there is nothing to retry.

Suggested fix

Make the retry independent of unrelated upstream edits: while a refresh would refuse an entry, something must set refresh_needed every night until the refusal is settled. The most faithful way is to record the refusal. When --refresh refuses a target, promote writes a marker on that ledger entry (for example refresh_refused, with the pin and the message), and the partial pull request carries it. drift-check raises a refresh finding for every marked entry, and the next successful refresh of the entry drops the marker. The run then stays red every night until the copies are reconciled or a map line lands, whatever caused the refusal, and a new map line is applied the night after it merges.

A cheaper option, with no ledger change, is for drift-check to raise a refresh finding for an entry that is behind its canonical pin while a recorded source has diverged. It misses a refusal caused by an unrecorded same-name copy, and it turns a fresh divergence red as soon as its entry lags its pin (most nights), not only once a refresh has refused it.

With either drift-check change, the reproduction becomes the regression test, with the night 2 and night 3 refresh_needed assertions inverted. Separately, a workflow_dispatch input that runs the refresh job regardless of refresh_needed would give operators a retry button. If waiting for the next refresh is acceptable, this issue can close as documented.

Until then, the workaround after adding a map line is to run tools/sync and tools/promote --refresh --exclude dp jax locally, and to open the pull request by hand.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions