Skip to content

Commit 2c3d624

Browse files
mmckyclaude
andauthored
Backfill bibliography entries for citations a sync introduces (#226)
* Backfill bibliography entries for citations a sync introduces sync and forward translate {cite} roles, but the bibliography those keys resolve against is a shared file that never crossed the boundary: sync-orchestrator filters changed files to .endsWith('.md'), and the engine had zero occurrences of .bib or bibtex anywhere in src/. It had no bibliography concept at all — init only ever got one as a side effect of copying every non-markdown file wholesale. Under a strict build (-n -W) Sphinx promotes "could not find bibtex key" to a hard failure, so the target stopped building on a file nobody edited. Three manual repairs so far: four lectures in one resync wave, then 70 keys (see QuantEcon/lecture-python.zh-cn#203), then 21 more on 2026-07-27. The trigger is demand-driven, not diff-driven. lecture-python-intro's msy_fishery.md cites five keys lecture-intro.zh-cn lacks and no source PR touches the bib — the target simply has not translated that lecture yet. A "did the source diff touch the .bib" design misses that class entirely. Safety model: a key that already resolves in the target is never a candidate, and appends never rewrite existing bytes, so an edition that has localised an entry cannot have it clobbered. The observation that the estate's bibs are currently strict subsets of their sources is not relied on anywhere. Ambiguity is reported rather than guessed — a key declared twice in the source, or differing from an existing target key only in case, fails the run. Keys already dangling before the run are warnings, not errors: the run did not introduce them. Two measured details drive the implementation, both pinned by tests: - The entry parser tolerates leading whitespace. Eight of the 501 entries in quant-econ.bib are indented; an ^@-anchored parser finds 493 and would re-append those eight, producing exactly the duplicate-key -W failure this prevents. - The citation walker is stack-based and selectively transparent, descending into {note}/{exercise}/{warning}/{prf:*} while staying opaque inside code cells. 71 of 922 citations in lecture-python.myst (7.7%) live inside admonitions; a flat skip-all-fences walker misses every one. New `bibliography` input selects backfill (default), lint or off. An unrecognised value fails the run — a disabled guard looks exactly like a passing one. Refs #117 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Fail closed on bibliography fetch errors, and correct a stale metric Both from Copilot's review of #226. 1. `fetchBibliographies` swallowed every error from reading the target's `_config.yml` as "this edition has no bibliography". A rate limit, a 5xx or a permissions failure therefore disabled the guard silently and the run went green having skipped the check — the exact failure shape this module exists to remove, and a contradiction of the fail-closed discipline applied to the bibliography read one function lower. Only a genuine 404 now means "not applicable"; every other status fails the run with the status in the message. The same bare catch guarded the source-side read. Copilot flagged one instance; the principle covers both. A transient failure there would have surfaced as a misleading "resolves in neither bibliography" error attributed to the wrong cause. 2. The module header quoted 45 of 872 citations (5.2%) inside non-code containers, from the design note. The figure measured against the current corpus is 71 of 922 (7.7%), which is what the tests and the PR body state. Corrected, anchored to the commit it was measured at, and marked indicative — nothing depends on its exact value; it is quoted to justify why the walker is stack-based rather than flat. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent c43a78f commit 2c3d624

12 files changed

Lines changed: 1666 additions & 17 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111
- **The `\translate-resync` comment trigger is gated on commenter trust, and the sync workflow's job token drops to read-only** (#192, absorbing #138; surfaced by Copilot on QuantEcon/test-translation-sync#671 and QuantEcon/lecture-python.myst#979): every documented copy of the source-sync workflow fired on *any* commenter. `issue_comment` workflows run in default-branch context with full access to secrets, and GitHub cannot filter the event by comment body at the trigger level — so the `if:` is the only gate, and it checked only that the body contained the magic string. Any GitHub account could comment `\translate-resync` on a merged PR and spend Anthropic credits and runner minutes at will, repeatedly. The commenter controls nothing but the trigger (translated content comes from the repo), so this is cost abuse rather than code execution — but it was live in the canonical template and therefore on every deployed instance. Two smaller holes came with it: the condition never required `github.event.issue.pull_request`, so a comment on a plain **issue** fired a run too; and the generated workflow carried no `permissions:` block at all, leaving the ambient `GITHUB_TOKEN` at whatever the repo default is. **Now** the `issue_comment` clause requires all four conditions — a comment on a PR, the command, and an author in `["OWNER", "MEMBER", "COLLABORATOR"]` — and the job declares `permissions: contents: read`, since the action authenticates to the target repo with the PAT input and checkout is the ambient token's only consumer. `CONTRIBUTOR` (anyone with one merged PR) is deliberately excluded: `src/inputs.ts` has always enforced exactly this three-way set *inside* the action, so admitting it at the workflow level would only buy a billed run that then no-ops — the outer gate has to be at least as tight as the inner one. The fix lands in all **fourteen** in-repo copies at once (the scaffolder in `src/cli/commands/setup.ts`, twelve documented copies across README, `examples/`, quickstart, action-reference and the three tutorials, and the E2E harness template) plus the FAQ's stated requirements, because the exposure was never one file — each new doc page had copied the shape from an older one. A new guard in `workflow-templates.test.ts` **sweeps** the publishable surfaces rather than enumerating them: it parses every workflow it finds under `README.md`, `examples/`, `docs/`, `tool-test-action-on-github/` and `.github/`, plus the scaffolder's generated output, and fails per job on a missing condition, a missing `contents: read`, or any surviving copy of the pre-fix one-line form — so a doc page added tomorrow is covered the day it lands. It also asserts the workflow's association set still matches `TRUSTED_ASSOCIATIONS` in `inputs.ts`, which is the invariant that keeps the two gates from drifting apart. The shape is the one already deployed on QuantEcon/lecture-python.myst, where both trigger branches have fired since 2026-07-22 — the folded `if:` is not a theory. Deployed instances still need the same edit; this ships the template they are copied from.
1212

1313
### Fixed
14+
- **Citations a sync introduces now carry their bibliography entries across** (#117 — turns previously-green runs red by design when a key resolves nowhere): `sync` and `forward` translate `{cite}` roles but the bibliography those keys resolve against is a shared file that never crossed. `sync-orchestrator.ts` filters changed files to `.endsWith('.md')`, and the engine had **zero** occurrences of `.bib` or `bibtex` anywhere in `src/` — it had no bibliography concept at all, so `init` only ever got one as a side effect of copying every non-markdown file wholesale. Under a strict build (`-n -W`) Sphinx promotes "could not find bibtex key" to a hard failure, so the target repo stopped building on a file nobody edited. Field record: four lectures in one resync wave, then a 70-key backfill (see QuantEcon/lecture-python.zh-cn#203), then 21 more keys on 2026-07-27 — three manual repairs of a mechanism that did not exist. **Now** a new `src/bibliography.ts` extracts the citation keys a run *introduces into the target*, and appends the matching entries from the source bibliography. The trigger is deliberately **demand-driven rather than diff-driven**: `lecture-python-intro/lectures/msy_fishery.md` cites five keys `lecture-intro.zh-cn`'s bibliography lacks and no source PR touches the bib — the target simply has not translated that lecture yet — so a "did the source diff touch the .bib" design misses the larger class entirely. **The safety model is that a key which already resolves in the target is never a candidate**, and appends never rewrite existing bytes, so an edition that has localised an entry cannot have it clobbered; the observation that the estate's bibliographies are currently strict subsets of their sources is not relied on anywhere. Ambiguity is reported, never guessed: a key declared twice in the source, or one differing from an existing target key only in case, fails the run rather than appending. Keys that were *already* dangling before the run are surfaced as warnings and left alone — the run did not introduce them. Two measured details drive the implementation and are pinned by tests: the entry parser tolerates **leading whitespace**, because eight of the 501 entries in `quant-econ.bib` are indented and an `^@`-anchored parser finds only 493 — it would report those eight as missing and re-append them, producing exactly the duplicate-key `-W` failure this prevents; and the citation walker is **stack-based and selectively transparent**, descending into `{note}`/`{exercise}`/`{warning}`/`{prf:*}` containers while staying opaque inside code cells, because 71 of 922 citations in `lecture-python.myst` (7.7%) live inside admonitions and a flat skip-all-fences walker misses every one. A new `bibliography` input selects `backfill` (default), `lint` (report without copying — the opt-out for editions that hand-author entries) or `off`; an unrecognised value fails the run, because a disabled guard looks exactly like a passing one.
1415
- **Review mode no longer fails the run when a document was deliberately deleted** (#210): a source PR that removes a lecture produces a translation PR that removes the corresponding target file — a routine editorial operation that failed its review run in **every** language, reproducibly. The F40 guard (#163) refuses to review when no source content could be fetched, which is right, but it could not tell a failed fetch from a file that is *supposed* to be gone; a deletion has no source content by definition, and `getSourceDiff` skips that fetch deliberately, so the guard was firing on this engine's own intent. A red that is always expected is precisely what trains people to ignore the reds F40 exists to raise. **Now** `getSourceDiff` reports which files the source PR deleted, and those are partitioned out before the content loop: a PR that only deletes is reported as a deletion — no model calls, so it costs nothing — with a comment that states outright that no quality evaluation was performed, and it routes to a human rather than auto-merge, because deleting a translated document is an editorial decision and nothing about the content was verified. A PR that deletes *and* edits now reviews the surviving files instead of aborting on the deleted one. **The guard stays fatal everywhere else, and fails closed**: if the source PR cannot be listed the deletion set comes back empty, so nothing is excused and absent source content is still an abort — absence of evidence is never read as evidence of deletion. A **new** deterministic check falls out of the same information: a target file removed with no matching source deletion — the translation dropping a document the source still has — is now a blocker finding that gates. Also fixed alongside it: the per-file `try` was wide enough that the target-content 404 raised by *any* deletion skipped the rest of the loop body, including the source fetch that would have succeeded, which is how a single deleted file emptied `sourceEnglish` and took the whole run down under a misleading `Error processing …` line. Renames are covered by the same partition, including the delete+add form GitHub reports when heavy edits defeat its rename detection. Resync PRs are unaffected: a commit is a state, not a diff, so there is no deletion to read off it and that path's behaviour is unchanged.
1516
- **The E2E harness tests one version across every workflow, gains Malayalam, and stops destroying the Farsi target's workflows** (#202 and follow-ons): a full run previously pinned **2 of 9 workflows**. The harness rendered only the zh-cn and fa sync templates; the other seven — a hand-made ml sync workflow plus review and rebase in each target repo — floated on `@v0`, so one run tested two versions and reported it as one. Worse, the fa reset ran `rm -rf … .github/` **without re-rendering**, so it silently deleted that target's review and rebase workflows on every run; fa had none at all. And ml, absent from the script entirely but wired up by hand, fired and failed on all 26 PRs of every run with `Unsupported target language` — unreported, because the banner did not know it existed. **Now**: one `LANGUAGES` array drives everything (adding a language is one line plus three fixtures), the harness writes all nine workflows across all four repos from templates carrying a substituted ref, and a per-workflow **census** prints before any PR is created so "this run tested version X" is falsifiable rather than asserted. Target-repo workflows render from `examples/` — the canonical templates users receive — so the harness cannot drift from what it is meant to be testing. The three near-duplicate reset blocks (which had already drifted three separate ways) collapse into subshelled functions with absolute `cd`s, and `git commit || echo` — which made a genuine commit failure indistinguishable from nothing-to-do, then pushed anyway — now fails the run. **`@v0` is preserved, not traded away**: rather than leaving some workflow permanently floating, `--action-ref v0` renders the tag into every workflow as a post-release smoke, so the check moves to *after* the tag move, the only point where its answer means anything (#109). **The default is now `main`, not the package.json version** — the version bump happens in the release commit, so that default tested the *previous* release rather than the code under development, and refused to run at all during a release PR. Release gating is an explicit `--action-ref vX.Y.Z` step. **ml is a first-class third language**, its fixtures seeded with `translate init` so they carry the current `translation:` frontmatter and passed the #159 parity guard on write; the fa and zh-cn fixtures are migrated off the legacy `heading-map:` key with `translate headingmap` (no LLM calls, translations byte-untouched), which matters because the writer *deletes* that key on rewrite — the harness's own baseline used to mutate on run one. Docs corrected: both pages claimed the harness built from `main` and that TEST mode makes "no Claude API calls" — it makes real, billed calls, and this PR roughly doubles them by installing review workflows in every target (~1.4M input tokens measured on two languages pre-dates that, so it is a floor). Guards added in the `workflow-templates.test.ts` mold cover ref substitution, the `main` default, fixture presence and format per language, structural parity against source, and that `.github/` is only deleted where it is also re-rendered.
1617

action.yml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,16 @@ inputs:
5757
required: false
5858
# Keep in sync with DEFAULT_CLAUDE_MODEL in src/models.ts
5959
default: 'claude-sonnet-5'
60+
bibliography:
61+
description: >-
62+
How to handle citations a run introduces that the target bibliography lacks:
63+
"backfill" (default) copies the entry from the source bibliography, "lint"
64+
reports and fails without copying, "off" disables the check. Entries are only
65+
ever appended — a key that already resolves in the target is never touched,
66+
so localised entries are safe.
67+
required: false
68+
# Keep in sync with DEFAULT_BIBLIOGRAPHY_MODE in src/bibliography.ts
69+
default: 'backfill'
6070
github-token:
6171
description: 'GitHub token for API access'
6272
required: true

0 commit comments

Comments
 (0)