promote: asset provenance and placement gaps #23

Follow-up from the review of #14.

Four gaps in how tools/promote places a pool asset and records where it comes from. They share a root: an asset's pool path depends on which of its lectures happen to be in the batch, and its provenance on byte identity across series, rather than on the lectures that use it. Nothing has gone wrong in the pool yet, but (2) already has six such records in the committed ledger.

Related to QuantEcon/project-monorepo#11, whose deferred promote hardening includes re-homing assets within the same batch (relevant to 1) and recording a series' asset bytes when they differ from the canonical copy's (relevant to 4).

Each reproduction below uses the tools/tests harness (make_repo, lecture_text) and asserts the current behaviour, so it fails once the gap is addressed. All of them pass on #14's head as it merges, with #17 included. Line numbers are at that head and will move; the function and check names given with them will not.

1. A partial refresh splits a shared asset across two pool paths

What happens. Promotion decides each asset's destination per (src_rel, digest), counting the lectures in the batch plus the ledger entries outside it (tools/promote:1330-1351). Take _static/_shared/amss2/crra_utility.py, used by amss2 and amss3. advanced changes the file, and in the same refresh amss3 is refused because its copy in dp-test, the sandbox series, was edited to differ. The new bytes then have one consumer, amss2, so plan_destination puts them at _static/amss2/crra_utility.py, and the old _shared file stays for amss3. Since #14 the refresh PR carries the entries that did refresh, so once it is merged the pool holds one upstream file at two paths, with different bytes, and amss2's rewrite points at the new one.

If amss3 is settled after that, as its FAILED line suggests (a sync/canonical.yml line, or reconciling dp-test's copy upstream), the next refresh targets amss3 alone, which would move the new bytes back to _shared. The re-homing check (tools/promote:1353-1370) refuses that because amss2, already at its pin and so outside the batch, still references _static/amss2/crra_utility.py. The run stays red every night until advanced's pin moves again for some other reason and both lectures land in one batch. Settling amss3 before the partial PR is merged avoids this, since the next run then refreshes both from main, but merging the refreshed entries promptly is what the partial PR is for.

Reproduction.

CRRA = "_static/lecture_specific/amss2/crra_utility.py"

def amss(title, body=""):
    """A lecture whose code cell :load:s the shared amss2 family file."""
    return lecture_text(title, body + "\n\n```{code-cell} python3\n:load: " + CRRA + "\n```")

repo = make_repo(["advanced", "dp-test"])
for s in ("advanced", "dp-test"):
    repo.mirror_file(s, CRRA, "v1\n")
    repo.lecture(s, "amss2", amss("AMSS2"))
    repo.lecture(s, "amss3", amss("AMSS3"))
assert repo.promote("amss2", "amss3").returncode == 0   # both use _static/_shared/amss2/crra_utility.py

repo.mirror_file("advanced", CRRA, "v2\n")               # advanced edits the file and both lectures
repo.lecture("advanced", "amss2", amss("AMSS2", "advanced edit"))
repo.lecture("advanced", "amss3", amss("AMSS3", "advanced edit"))
repo.move_pin("advanced")
repo.lecture("dp-test", "amss3", amss("AMSS3", "sandbox edit"))   # dp-test edits its amss3
repo.move_pin("dp-test")

assert repo.promote("--refresh").returncode == 1         # night 1: amss2 refreshes, amss3 is refused
assert repo.entry("amss2")["assets"] == ["_static/amss2/crra_utility.py"]          # holds v2
assert repo.entry("amss3")["assets"] == ["_static/_shared/amss2/crra_utility.py"]  # holds v1

repo.write_canonical_map({"amss3": "advanced"})          # settle amss3 as its FAILED line says
assert repo.promote("--refresh").returncode == 1         # night 2, and every night after
assert repo.promote("amss2", "amss3").returncode == 0    # the way out today: both by name

Night 1, then night 2 and every night after (reconciling dp-test's copy instead of the map line gives the same):

[amss3] FAILED: amss3.md differs across series (same: advanced:5308d67e33c8; different: dp-test:b8d8825c1504) — a same-named file must be one lecture: reconcile or rename it upstream, or name the canonical series in sync/canonical.yml
[amss3] FAILED: asset _static/lecture_specific/amss2/crra_utility.py is recorded at _static/amss2/crra_utility.py but now maps to _static/_shared/amss2/crra_utility.py — re-promote its other consumer(s) (amss2) in the same batch

On main. The placement code is main's, but main handles the same upstream state without a split: both lectures refresh (dp-test's copy is recorded as divergent) and the file stays in _shared. The split comes from #14's refusal of an edited copy together with its partial refresh PR.

When it bites. All three lecture_specific families in _static/_shared/ have this shape, each named after one of its two users: amss2 (amss2, amss3; 4 files), chang_credible (chang_credible, chang_ramsey; 1 file) and opt_tax_recur (opt_tax_recur, amss; 3 files). All six lectures are canonical in advanced, and dp-test is their only other canonical-eligible holder, so today it takes an unsettled dp-test edit to amss3, chang_ramsey or amss at the time advanced changes a file in that family. For these families the window closes when dp-test is retired (QuantEcon/project-monorepo#24), but the pattern recurs wherever a _shared family is named after one of its users and another user can be refused. Once stuck, the way out is to promote both users by name in one batch, with the exclusions CI uses (today tools/promote amss2 amss3 --exclude dp jax).

Suggested fix. Make the refusal symmetric, as it already is in the reverse case: if the refused lecture is amss2, the one the family directory is named after, the overwrite check (tools/promote:1410-1438) refuses amss3 too, and both refresh together once amss2 is settled. Here, when a refreshing plan's file is still recorded, with older bytes from the same series, at another pool path that an entry outside the batch references, cull the plan and name that entry, so both lectures stay behind the pin and refresh together once the refused one is settled. Counting consumers per src_rel alone is not enough: it also makes two different files that share a path in different series collide, the case the (src_rel, digest) key exists for. The same-batch re-homing deferred in QuantEcon/project-monorepo#11 points at the fuller answer: let --refresh add the consumers that the re-homing check names to the batch, as the by-name workaround does by hand.

2. An asset's canonical series need not be one its lectures come from

What happens. asset_provenance (tools/promote:1465-1483) records as sources every eligible series whose file at the asset's path has the promoted bytes (line 1474), and takes the first of them in manifest order as canonical (line 1480). It never looks at the lectures that use the asset, so the canonical, or any watched source, can be a series whose copy of the lecture is divergent, or which holds no user lecture at all. drift-check's check_asset (tools/drift-check:450-523) then follows that series:

• when that series changes the file, drift-check raises asset-moved, a refresh request, but --refresh only targets entries whose own canonical pin moved (refresh_targets, tools/promote:1066-1125), so nothing is refreshed and the request repeats every night while the nightly stays green;
• when the series the lectures do come from changes the file, drift-check opens an asset-diverged drift issue against that series and requests no refresh, although promote --refresh would take the new file.

Reproduction.

FIG = "_static/lecture_specific/lq/fig.png"
repo = make_repo(["intermediate", "dp-test"])
for s in ("intermediate", "dp-test"):
    repo.mirror_file(s, FIG, b"fig v1")
    repo.lecture(s, "lq", lecture_text("LQ", f"{s}'s version", refs=[FIG]))
repo.write_canonical_map({"lq": "dp-test"})              # as for lqcontrol today
assert repo.promote("lq").returncode == 0
assert repo.entry("lq")["canonical"] == "dp-test"
assert repo.ledger()["assets"]["_static/lq/fig.png"]["canonical"] == "intermediate"

repo.mirror_file("intermediate", FIG, b"fig v2")         # intermediate, whose lq is divergent, changes it
repo.move_pin("intermediate")
for night in (1, 2):
    assert "[asset-moved] lectures/_static/lq/fig.png" in repo.drift_check().stdout
    assert "nothing to refresh" in repo.promote("--refresh").stdout

Every night:

[asset-moved] lectures/_static/lq/fig.png: canonical asset in intermediate changed upstream
[stale-copy] lectures/_static/lq/fig.png: the dp-test copy is unchanged while intermediate moved
nothing to refresh — every canonical pin matches its ledger entry.

If dp-test changes the figure instead, drift-check --print-issues lists drift(dp-test): lectures/_static/lecture_specific/lq/fig.png differs from its canonical copy in intermediate with refresh_needed false.

On main. The same: asset_provenance is main's apart from leaving consumers out, and the records below are in main's ledger.

When it bites. Already latent in six records: the five _static/lqcontrol/solution_lqc_*.png figures are canonical in intermediate, while lqcontrol is canonical in dp-test through sync/canonical.yml and intermediate's copy is divergent; and _static/_shared/_admonition/gpu.md is canonical in programming, which no pool lecture comes from. On a copy of the committed pool and ledger, with the mirror at its pinned commits, an intermediate edit to solution_lqc_ex1.png gives the asset-moved finding above every night, the refresh leaves lqcontrol alone, and from the second night it prints nothing to refresh. Since #14, jax is considered by default (CI still excludes it), which adds cases: a whole-estate scratch promotion during the review recorded jax as a source of the four arellano figures although jax's arellano is divergent, and simulating the rename of jax's copy planned in QuantEcon/project-monorepo#76 gives four drift(jax): … removed upstream issues.

Suggested fix. Derive an asset's provenance from the lectures that use it: take as its canonical the highest-priority series that is canonical for one of its users and holds those bytes, and record another series as a source only if it holds an identical copy of at least one user lecture (a sources member of that lecture's entry). Re-promoting lqcontrol and the gpu.md users once then rewrites the six records.

3. An asset destination collision is settled by run order

What happens. Two lectures from different series that use the same family file with different bytes both map to one _static/_shared/ path. The collision check (tools/promote:1372-1408) lets the first plan claim the path and refuses the later one, so which lecture enters the pool, and whose bytes the shared file holds, depends on run order: argument order, toc order, or slug order for --refresh (tools/promote:1086). That contradicts the docstring's "Which copy the pool keeps never falls out of run order" (tools/promote:29), which #14 made the rule for lectures.

Reproduction.

FAM, DEST = "_static/lecture_specific/fam/x.png", "_static/_shared/fam/x.png"

def use_family_file(repo, intro_slug, intermediate_slug):
    """An intro and an intermediate lecture use one family file, whose bytes differ by series."""
    repo.mirror_file("intro", FAM, b"intro bytes")
    repo.lecture("intro", intro_slug, lecture_text("Intro lecture", refs=[FAM]))
    repo.mirror_file("intermediate", FAM, b"intermediate bytes")
    repo.lecture("intermediate", intermediate_slug, lecture_text("Intermediate lecture", refs=[FAM]))

outcome = {}
for order in (("aaa", "bbb"), ("bbb", "aaa")):
    repo = make_repo(["intro", "intermediate"])
    use_family_file(repo, "aaa", "bbb")
    assert repo.promote(*order).returncode == 1
    outcome[order] = (sorted(k for k in repo.ledger() if k != "assets"), repo.pool(DEST).read_bytes())
assert outcome[("aaa", "bbb")] == (["lectures/aaa.md"], b"intro bytes")
assert outcome[("bbb", "aaa")] == (["lectures/bbb.md"], b"intermediate bytes")

[bbb] FAILED: asset destination collision at _static/_shared/fam/x.png: intermediate:_static/lecture_specific/fam/x.png differs from intro:_static/lecture_specific/fam/x.png
[aaa] FAILED: asset destination collision at _static/_shared/fam/x.png: intro:_static/lecture_specific/fam/x.png differs from intermediate:_static/lecture_specific/fam/x.png

In a refresh where both lectures start using the file and both pins move, the slug that sorts first wins (intro's aaa over intermediate's bbb, but bbb over an intro zzz). Each run exits 1 with one lecture promoted, and since #14 the refresh PR carries that winner.

On main. The same code.

When it bites. Nothing is affected today: a whole-estate scratch promotion of 297 lectures during the review found no asset collisions. It bites when two series start using one family file with different bytes and both lectures are in one batch: a whole-estate or --toc promotion, or a refresh in which both series' pins moved.

Suggested fix. On a destination collision, refuse every plan in the batch that maps different bytes to that path, not only the later one, and name all the series and lectures involved, as #14 does for lectures.

4. A refresh drops an edited or stale asset source, and its drift issue closes as "Resolved"

What happens. Every promotion of a lecture rebuilds the records of its assets (tools/promote:1515-1516), and asset_provenance takes their sources from byte identity again (tools/promote:1474-1478). Asset sources record no per-series digest, so a recorded source whose copy no longer matches the promoted bytes is dropped silently, whether it was edited or is merely stale, and the run exits 0. A refresh re-promotes every entry whose canonical pin moved, so an unrelated change in the same series is enough to trigger it. drift-check then stops comparing that copy, and the next nightly --issues run closes its open asset-diverged issue with "Resolved: tools/drift-check no longer detects this divergence" (tools/drift-check:686-690) while the copy still differs.

This is the asset counterpart of what #14 settled for lectures: a lecture copy edited to differ is refused and keeps the run red, and a stale lecture copy stays a source. An asset copy gets neither.

Reproduction.

FIG = "_static/lecture_specific/career/fig.png"
repo = make_repo(["intermediate", "dp-test"])
for s in ("intermediate", "dp-test"):
    repo.mirror_file(s, FIG, b"fig v1")
    repo.lecture(s, "career", lecture_text("Career", refs=[FIG]))
repo.lecture("intermediate", "other", lecture_text("Other"))
assert repo.promote("career", "other").returncode == 0      # asset sources: intermediate, dp-test

repo.mirror_file("dp-test", FIG, b"fig edited in dp-test")   # night 1: the drift issue opens
repo.move_pin("dp-test")
issue = "drift(dp-test): lectures/_static/lecture_specific/career/fig.png differs from its canonical copy in intermediate"
assert issue in repo.drift_check("--print-issues").stdout

repo.lecture("intermediate", "other", lecture_text("Other", "an unrelated fix"))
repo.move_pin("intermediate")
assert repo.promote("--refresh").returncode == 0             # night 2: career is re-promoted too
sources = repo.ledger()["assets"]["_static/career/fig.png"]["sources"]
assert [s["series"] for s in sources] == ["intermediate"]    # dp-test dropped

out = repo.drift_check("--print-issues").stdout              # night 3: --issues would close it
assert "--issues would keep 0 issue(s) open" in out
assert repo.pool("_static/career/fig.png").read_bytes() == b"fig v1"   # dp-test's copy still differs

The stale case goes the same way: if intermediate updates the figure while dp-test's copy stays at the recorded bytes, drift-check reports stale-copy (info), the refresh drops dp-test, and a later dp-test edit of its copy goes unreported (drift-check exits 0).

On main. The same: main rebuilds asset sources the same way and closes issues with the same note.

When it bites. Today 23 of the 32 pool assets also list dp-test as a source (16 alongside advanced, 6 alongside intermediate, and gpu.md), so a dp-test edit to any of them opens a drift issue that the next refresh of one of its lectures silently retires. After the whole-estate promotion, the same holds for any asset held by two canonical-eligible series.

Suggested fix. Record a digest per asset source, as lecture sources already have; this is the asset-bytes item deferred in QuantEcon/project-monorepo#11. A refresh can then keep a source whose copy is still at its recorded digest (stale, reported as stale-copy as for lectures), and either refuse when a source's copy was edited to something else or keep it recorded so drift-check keeps reporting it. Separately, the close note at tools/drift-check:689 says the divergence is resolved whenever a finding stops appearing, including when its copy has merely stopped being compared; a neutral "no longer reported" would be accurate.