Repository navigation
promote, drift-check: a copy that caught up to an unrecorded canonical version, or in its own asset-path spelling, is refused as a collision and reported as drift #18
Description
Follow-up from the review of #14. Line numbers are at #14's head as it merges, with #17 included.
What happens
Since 7128fb1 (in #14), a recorded source that no longer matches the canonical copy counts as stale, not edited, only if its raw digest equals one of two recorded digests: its own, or the canonical copy's at the last promotion (resolve_sources, tools/promote:718-726; check_lecture, tools/drift-check:377 and :394). Two kinds of copy that have only caught up fall outside those two digests:
- (a) A copy at a canonical version that was never recorded. In the nightly cycle the ledger records a canonical version only when a refresh PR is merged, and
drift/refreshis rebuilt from main on every run. If the canonical copy changes twice before that merge, or a version comes and goes between two nightly syncs, a series that ports the version in between holds a digest the ledger never saw. - (b) A normalised-equal source that caught up in its own asset-path spelling. Since promote: differing same-name copies are an error, jax is considered, and a consumer series class (project-monorepo#74) #14, promote admits a source that equals the canonical copy once asset references are normalised, and records the source's own raw digest. When that copy catches up with the canonical version recorded at the last refresh, it keeps its own spelling, so its digest cannot equal the canonical copy's recorded digest.
In both cases, once the canonical copy has moved on:
promote --refreshrefuses the entry as a collision, and the nightly run goes red. Its message (tools/promote:761-765) says to "name the canonical series in sync/canonical.yml"; doing that makes the entryinterim, with the lagging copy recorded as a divergent lecture.drift-checkreportsdivergedand opens a drift issue saying the copy "has since been edited on its own", although it is only behind, not edited.
Both persist until the lagging copy reaches the canonical copy's current version. The same steps go through as stale-copy when the version was recorded (the v2 refresh merged before v3) or the spelling is identical; the latter is test_a_copy_that_caught_up_then_fell_behind_again_is_still_stale in tools/tests/test_promote_collisions.py.
What #17 changes
PR #17, which lands with #14, makes drift-check compare a moved source with the canonical copy's current text under promote's normalised_text (same_lecture, tools/drift-check:385). That covers the lockstep case in #15, but not the stale rule: tools/drift-check:394 still compares the copy's raw digest with the canonical copy's recorded raw digest, and promote is unchanged. With #17, the copy in (b) is reported as converged ... up to asset path spelling while the canonical copy stays at the version it caught up with, then as diverged once the canonical copy moves on. (a) is the same with or without #17.
Related to #15, which this extends: there, drift-check alone raised a false diverged that cleared once the refresh PR merged. Here promote refuses too, so no refresh PR clears it.
Before #14
On main, drift-check already reported (a) as diverged, since it had no caught-up rule, but the refresh went through (marking the entry interim) and the run stayed green. (b) cannot arise on main, which admitted only byte-identical sources.
Reproduction (with the tools/tests harness)
from conftest import lecture_text
FIG = "_static/lecture_specific/short_path/graph.png"
def test_a_copy_at_an_unrecorded_canonical_version_is_called_an_edit(make_repo):
repo = make_repo(["programming", "intermediate"])
for series in ("programming", "intermediate"):
repo.lecture(series, "pandas_panel", lecture_text("Panel Data", "Version one."))
assert repo.promote("pandas_panel").returncode == 0 # both recorded at v1
# programming publishes v2 and then v3 before a refresh PR is merged, so
# the ledger still records v1. intermediate ports v2, exactly.
repo.lecture("programming", "pandas_panel", lecture_text("Panel Data", "Version three."))
repo.move_pin("programming", 3)
repo.lecture("intermediate", "pandas_panel", lecture_text("Panel Data", "Version two."))
repo.move_pin("intermediate")
assert "[diverged] lectures/pandas_panel.md" in repo.drift_check().stdout
res = repo.promote("--refresh")
assert res.returncode == 1
assert "[pandas_panel] FAILED: pandas_panel.md differs across series" in res.stderr
# Following the advice in that message makes the entry interim.
repo.write_canonical_map({"pandas_panel": "programming"})
assert repo.promote("--refresh").returncode == 0
assert repo.entry("pandas_panel")["interim"] is True
def test_a_normalised_equal_copy_that_caught_up_is_called_an_edit(make_repo):
repo = make_repo(["intro", "intermediate"])
def publish(series, body): # one lecture: intro writes /_static/..., intermediate _static/...
repo.mirror_file(series, FIG, b"png")
ref = "/" + FIG if series == "intro" else FIG
repo.lecture(series, "short_path", lecture_text("Shortest Paths", body, refs=[ref]))
publish("intro", "Version one.")
publish("intermediate", "Version one.")
assert repo.promote("short_path").returncode == 0
publish("intro", "Version two.")
repo.move_pin("intro", 2)
assert repo.promote("--refresh").returncode == 0 # intermediate kept as a stale source
publish("intermediate", "Version two.") # caught up, in its own spelling
repo.move_pin("intermediate", 2)
publish("intro", "Version three.") # intro moves on before a refresh records the catch-up
repo.move_pin("intro", 3)
assert "[diverged] lectures/short_path.md" in repo.drift_check().stdout
res = repo.promote("--refresh")
assert res.returncode == 1
assert "[short_path] FAILED: short_path.md differs across series" in res.stderrBoth tests pass on #14's head, with #17 included (and at 7128fb1 before it), and the asserted steps print the same lines throughout:
[diverged] lectures/pandas_panel.md: the intermediate copy lectures/pandas_panel.md no longer matches its canonical home in programming
[pandas_panel] FAILED: pandas_panel.md differs across series (same: programming:91ab2fd94f58; different: intermediate:9edfc192d91e) — a same-named file must be one lecture: reconcile or rename it upstream, or name the canonical series in sync/canonical.yml
[diverged] lectures/short_path.md: the intermediate copy lectures/short_path.md no longer matches its canonical home in intro
[short_path] FAILED: short_path.md differs across series (same: intro:d9aec721be1c; different: intermediate:0deebc23b47c) — a same-named file must be one lecture: reconcile or rename it upstream, or name the canonical series in sync/canonical.yml
A drift-check run between intermediate's catch-up and intro's next edit reports [converged] lectures/short_path.md: the intermediate copy changed and now matches intro up to asset path spelling since #17; before #17 it reported [diverged] there too (the #15 case).
When it bites
Not today. Every non-canonical source in the committed ledger is a dp-test copy (25 entries), byte-identical to its canonical copy, and the sandbox is only edited deliberately, to exercise the tools. Apart from those, and two housekeeping pages that are not in the pool (status, zreferences), no lecture on the real mirror at the current pins is held identically by two canonical-eligible series, byte for byte or after normalisation.
(a) becomes possible with the two merges in QuantEcon/project-monorepo#76: pandas_panel (programming, which is canonical, and intermediate) and autodiff (programming and jax, once jax leaves PROMOTE_EXCLUDE) are to be held identically by two series and kept in step by hand. It then takes one port of a version that was never recorded: the canonical copy edited twice in one day, or edited again while the refresh PR for the first edit is still open.
(b) also needs the pair to spell an asset reference differently. Both copies of each pair spell theirs the same way today, but the acceptance criterion in QuantEcon/project-monorepo#76 only asks for the copies to be identical after the ledger's path normalisation. The first candidates that #15 names, short_path and ifp_egm, are not candidates: on the real mirror their jax copies are different lectures, which that item renames.
Suggested fix
Make "stale" mean "the same lecture as a version the canonical copy has had", with one rule shared by both tools, as #17 did for converged:
- For (b), compare in the form promotion uses. For example, on an entry with a normalised-equal source, record each source's normalised digest (sha256 of
normalised_text) beside its rawdigest, and apply the stale test to normalised digests; other entries, which today means all of them, re-dump unchanged. drift-check, which importsnormalised_textsince drift-check: judge a non-canonical copy by promotion's own rule #17, applies the same test at:394. - For (a), also accept a copy that equals, up to normalisation, any version the canonical file had between its recorded
promoted_atpin and the current pin. drift-check already fetches the canonical copy by pin (fetch_at_pin), and GitHub's commits API filtered by path lists the versions in between. promote reads only the mirror today, so it would need the same best-effort lookup. - Until 2 lands, reword the refusal (
tools/promote:761-765) and thedivergeddetail (tools/drift-check:406-411): when the canonical copy has moved since promotion, a copy at no recorded version may be a port that is behind rather than an edit, and async/canonical.ymlline is the wrong remedy for it.
The two tests above, with their assertions inverted, cover the change; for (a) the harness needs a fake of the version lookup, since it cannot reach GitHub.
Activity