Skip to content

Six defects observed syncing a six-PR batch to lecture-intro.zh-cn (declared files silently dropped, toc clobbered, .bib never synced) #256

Description

@mmcky

Reporting six defects observed while merging a batch of six sync PRs into QuantEcon/lecture-intro.zh-cn (en → zh-cn). Sync tool-version 0.24.0, review engineVersion 0.25.0, model claude-sonnet-5.

Only one of the six PRs was mergeable as delivered. The rest needed hand repair, and two of the defects would have silently published wrong content rather than failing the build. Ranked by damage below, with reproduction evidence.

# Defect Blast radius in this batch
1 A file declared in translation-sync-metadata.files receives no diff 3 of 5 PRs
2 _toc.yml overwritten with the source-language version 2 of 2 PRs that added a lecture
3 .bib files are never synced every PR introducing a citation
4 Rebase automation can empty a branch and auto-close its PR 1 PR
5 Rendered PR body understates the files changed at least 2 PRs
6 Review mode reported a directional false positive 1 PR

1. A declared file can receive no diff at all

This is the root cause of most of the damage and the one worth fixing first.

The translation-sync-metadata block lists the files in scope. A file can appear there and receive zero lines of diff, with nothing warning that it happened.

PR Declared but not delivered Consequence
#278 lectures/prob_dist.md Upstream QuantEcon/lecture-python-intro#811 was a split: it moved the observed-distributions material into a new lecture and enriched what remained (+157 −287). Only the new file arrived. Merging as-is would have published the same sections twice, in both lectures. No Sphinx warning covers duplication, so CI was green on the duplicated content.
#280 lectures/fitting_distributions.md Upstream QuantEcon/lecture-python-intro#815 is fitting_distributions.md +1 -0 (adding a (qq_plots)= anchor) and heavy_tails.md +7 -3 (referencing it). Only the reference arrived, so the build failed with undefined label: 'qq_plots'.
#283 lectures/prob_dist.md Upstream QuantEcon/lecture-python-intro#819 rewrites that file's outline +7 −2 to introduce a three-lecture sequence. The PR's stated purpose is linking the sequence; the third leg was missing.

Worth noting #280 later self-corrected: a subsequent automatic resync produced the missing fitting_distributions.md diff without intervention. So the pipeline can produce the file — it just doesn't always, and doesn't notice when it doesn't.

Suggested fix: after generating the diff, assert that every entry in the metadata files array produced either a change or an explicitly recorded "no change required" with a reason. Fail the run on an unexplained empty. A silent half-applied refactor is far more expensive than a failed run.

2. _toc.yml is overwritten rather than merged

Both PRs that added a lecture replaced the translated table of contents with upstream's English one.

Two distinct harms. Every part caption reverted from Chinese to English — 导言Introduction, 概率与分布Probability and Distributions, and 12 more. And entries were added for tax_smooth, msy_fishery and bayes_intro, lectures that exist upstream but are not translated in this repo, producing toctree contains reference to nonexisting document and failing the build.

The correct diff in both cases was a single added line. #278 shipped +38 −34 where +1 −0 was right; #279 shipped +32 −14 for the same reason.

Suggested fix: treat type: toc files as a structured merge, not a text replacement. Add and remove - file: entries only; never write captions from the source language; and skip any entry whose target document does not exist in the target repo. As it stands, every sync that adds a lecture corrupts the toc of a translated repo.

3. .bib files are never synced

observed_distributions.md cites decock2011ames. Upstream QuantEcon/lecture-python-intro#811's own commit includes lectures/_static/quant-econ.bib +9 -0 adding exactly that entry — it was present in the source diff and simply not carried across. Result: could not find bibtex key "decock2011ames" and a red build.

Bibliography entries are language-neutral, so this is a straight copy rather than a translation problem. Any sync that introduces a new {cite} key will fail the target build until someone notices.

Suggested fix: carry across new bib entries for citation keys referenced by synced content.

4. The rebase automation can empty a branch and auto-close the PR

When #278 merged, the rebase workflow fired against the remaining PRs. On #279 the timeline records head_ref_force_pushed and closed at the same instant (2026-08-10T04:42:20Z), the PR's recorded head became the base branch's own tip, and the API began returning no commit found on the pull request. GitHub closed it because the branch had zero commits ahead of base.

Nothing was lost — the branch still held its commits and reopening restored the PR — but a reviewer not watching would reasonably conclude the PR had vanished. The same workflow also fails frequently on translation branches; several of its recent runs on this repo ended in failure.

Suggested fix: refuse to force-push when the resulting branch would be at or behind the base commit, and fail loudly instead. A rebase that legitimately results in no changes should close the PR deliberately with an explanatory comment, not by accident.

5. The rendered PR body understates the files changed

The human-readable "Files Updated" section is generated from a narrower source than the machine-readable files array, so a reviewer trusting the body sees less than the PR does.

PR Body omits Actually changed
#278 lectures/prob_dist.md, lectures/_static/quant-econ.bib declared in metadata
#280 lectures/fitting_distributions.md and its state file +2 −7 and +2 −2

The #280 case is the sharp one: the omitted file is precisely the one supplying the (qq_plots)= ref target that the change the body does advertise depends on. This is the same weak spot as defect 1 seen from the other side — one path drops the file, the other drops the mention of it.

Suggested fix: render "Files Added"/"Files Updated" from the same computed diff that populates the metadata array.

6. Review mode: one directional false positive

Review mode was genuinely valuable in this batch — see below — so this is a narrow accuracy note rather than a complaint.

On #283 it reported: "Numerous mystnb figure directives (caption/name metadata) were stripped from code cells throughout observed_distributions.md target, which is a structural regression not present in the source diff."

The direction is inverted. That diff adds 19 mystnb blocks, 19 name: fig: lines and 19 caption: lines, and removes zero. The resulting figure names are byte-identical to the English source at a453cf5. The PR is what introduced those captions to the repo.

Relatedly, its diff-quality score cannot account for a legitimately newer baseline. We closed a superseded PR and folded its content into a later one, so that later PR's diff necessarily exceeded its own source PR's diff. Review mode read the extra content as drift and scored diff quality 2.5/10 for what was expected and correct. Some signal that the target may legitimately be catching up across several source PRs would help.

What worked well

Review mode caught two things I would rather it had not had to:

It flagged #278 with structurePreserved: false and named the specific missing material — the E[f(X)] generalisation, moments, skewness, kurtosis, quantiles — which is exactly defect 1 manifesting. That verdict was accurate and actionable from the day the PR was opened, a week before anyone acted on it.

It also correctly identified that a (qq_plots)= anchor I had added to #279 was an unrequested addition not present in that PR's source. It was right: the anchor belongs to QuantEcon/lecture-python-intro#815, whose sync is a different PR. Acting on that feedback prevented a duplicate-anchor conflict when #280 later landed the same line.

Two of its findings in this batch were false positives against deliberate local convention — Baidu Baike links substituted for Wikipedia, and a Chinese font-loading block in the import cell — both pre-existing, repo-wide choices. Teaching it to recognise target-repo conventions would sharpen the signal.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinghigh-priorityAddress soon

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions