Skip to content

Commit 78446f6

Browse files
authored
Merge pull request #449 from DeepL/docs/pipeline-dedupe-open-prs
fix(pipeline): don't open a PR for docs that already have one
2 parents eaf70ef + 7c0bf79 commit 78446f6

9 files changed

Lines changed: 726 additions & 9 deletions

File tree

‎.github/workflows/docs-pipeline.yml‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@ name: Docs Pipeline
55
# pipeline detects gaps, drafts + reviews the missing content, and opens a PR.
66
# There is deliberately no schedule/cron trigger — spec change is the only
77
# automatic entry point. (workflow_dispatch is a manual escape hatch.)
8+
#
9+
# Gaps stay open until their PR merges, so consecutive runs see the same ones.
10+
# The pipeline skips gaps an open PR already covers and won't open a duplicate
11+
# PR (pipeline/open_prs.py) — a run with nothing new to say exits cleanly having
12+
# done nothing. GH_TOKEN below is what lets it read the open PRs.
813
on:
914
push:
1015
branches: [main]
@@ -62,7 +67,8 @@ jobs:
6267
# Required — add this secret in repo Settings → Secrets → Actions.
6368
# The pipeline can't call Claude without it.
6469
ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}
65-
# ship.py / post_review.py use the gh CLI, which reads GH_TOKEN.
70+
# ship.py / post_review.py and the open-PR duplicate check all use the
71+
# gh CLI, which reads GH_TOKEN.
6672
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
6773
run: |
6874
set -euo pipefail

‎.github/workflows/test-scripts.yml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,13 @@ on:
44
pull_request:
55
paths:
66
- "scripts/**"
7+
- "pipeline/**"
78
push:
89
branches:
910
- main
1011
paths:
1112
- "scripts/**"
13+
- "pipeline/**"
1214

1315
jobs:
1416
test:
@@ -19,3 +21,14 @@ jobs:
1921
with:
2022
node-version: 22
2123
- run: node --test scripts/*.test.mjs
24+
25+
pipeline:
26+
name: Test pipeline
27+
runs-on: ubuntu-latest
28+
steps:
29+
- uses: actions/checkout@v5
30+
- uses: actions/setup-python@v5
31+
with:
32+
python-version: "3.12"
33+
# Stdlib only — these tests stub out gh and never call the network.
34+
- run: python -m unittest discover -s pipeline -p "test_*.py" -v

‎pipeline/README.md‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,46 @@ pip install anthropic pyyaml
3030
export ANTHROPIC_API_KEY=...
3131
```
3232

33+
## Duplicate PRs
34+
35+
A gap stays open until its PR **merges**, so every run re-detects the gaps that are
36+
already waiting for review. Left alone, the pipeline opens one PR per run for the
37+
same missing pages.
38+
39+
`pipeline/open_prs.py` is the check that prevents that. It runs twice:
40+
41+
- **`generate.py`** drops gaps an open PR already covers, before any model call, so a
42+
duplicate run costs nothing.
43+
- **`ship.py`** refuses to open a PR when an open one already covers the whole run.
44+
45+
Both exit with code **3** ("nothing to do") instead of 0 or 1, and `run.py` stops the
46+
chain cleanly when it sees it.
47+
48+
A gap is "already in flight" if any open PR matches it on:
49+
50+
| Signal | Source | Catches |
51+
| --- | --- | --- |
52+
| Gap key | `<!-- docs-pipeline-gaps: [...] -->` marker in the PR body, written by `ship.py` | Any gap, including ones whose filename the model invents (two runs of the same gap produce different slugs) |
53+
| File path | The PR's changed files | Gaps with a predictable target path, including PRs the pipeline didn't open |
54+
| `covers:` frontmatter | The PR's diff | `missing_group_coverage` gaps, whose filename is unpredictable but whose group name is in the diff |
55+
56+
Overrides, for when you want the PR anyway:
57+
58+
```bash
59+
python pipeline/generate.py --ignore-open-prs # draft claimed gaps too
60+
python pipeline/ship.py --latest --force-new-pr # open a second PR
61+
```
62+
63+
If `gh` can't be reached the check is skipped with a warning rather than blocking the
64+
run, so a missing CLI never stops the pipeline (it just allows a duplicate).
65+
66+
The logic has tests (stdlib only, no gh, no network), run in CI on any `pipeline/**`
67+
change:
68+
69+
```bash
70+
python pipeline/test_open_prs.py
71+
```
72+
3373
## Pipeline Steps
3474

3575
The pipeline runs as a sequence of independent scripts. Each step reads the output of the previous one. You can run any step standalone, or use `run.py` to chain them all.
@@ -64,8 +104,12 @@ python pipeline/generate.py --section admin # generate for one fami
64104
python pipeline/generate.py --type missing_orientation # generate one gap type
65105
python pipeline/generate.py --force # regenerate even if files exist
66106
python pipeline/generate.py --section admin --force # regenerate one section
107+
python pipeline/generate.py --ignore-open-prs # include gaps an open PR covers
67108
```
68109

110+
Gaps that an open PR already covers are skipped before any model call. See
111+
[Duplicate PRs](#duplicate-prs).
112+
69113
Each run creates a timestamped folder under `pipeline/drafts/` with the generated `.mdx` files and a `report.json` with metadata.
70114

71115
### 2b. Rework Existing Pages (Phase 2 alternative to Generate)
@@ -137,10 +181,15 @@ python pipeline/ship.py --latest --dry-run # preview branch, commit
137181
python pipeline/ship.py --latest # create branch, commit, prompt before push
138182
python pipeline/ship.py --latest --yes # skip push confirmation
139183
python pipeline/ship.py --latest --branch docs/my-branch # custom branch name
184+
python pipeline/ship.py --latest --force-new-pr # ship even if an open PR covers it
140185
```
141186

142187
Requires `gh` CLI authenticated. Stages only `docs/**/*.mdx` and `docs.json`. Never force-pushes.
143188

189+
Exits 3 without creating a branch when an open PR already covers the whole run, and
190+
stamps every PR body with the gap keys it covers so later runs recognize it. See
191+
[Duplicate PRs](#duplicate-prs).
192+
144193
### 7. Post Review (PR Suggestions)
145194

146195
Posts "consider" findings from the review report as GitHub PR review comments with `suggestion` blocks. The reviewer can click "Apply suggestion" to accept changes directly in the PR.
@@ -306,5 +355,5 @@ The live site structure (which page sits in which tab) is read from `docs.json`
306355
- Python 3.8+
307356
- `anthropic` (for generate.py, rework.py, review.py, post_review.py)
308357
- `pyyaml` (for all scripts)
309-
- `gh` CLI (for ship.py, post_review.py)
358+
- `gh` CLI (for ship.py, post_review.py, and the open-PR check in generate.py)
310359
- `mint` / `npx` (optional, for broken-links check in promote.py)

‎pipeline/generate.py‎

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@
1212
python pipeline/generate.py --dry-run # show what would be generated
1313
python pipeline/generate.py --force # regenerate even if files exist
1414
python pipeline/generate.py --section admin --force # regenerate one section
15+
python pipeline/generate.py --ignore-open-prs # draft gaps even if a PR is open for them
16+
17+
Gaps that an open PR already covers are skipped before any model call — a gap is
18+
only closed when its PR merges, so every run would otherwise redraft and reship
19+
the same pages. See pipeline/open_prs.py.
1520
"""
1621

1722
import argparse
@@ -33,6 +38,7 @@
3338

3439

3540
from util import build_authoring_system_prompt
41+
from open_prs import EXIT_NOTHING_TO_DO, fetch_open_prs, split_claimed_gaps
3642

3743
REPO_ROOT = Path(__file__).resolve().parent.parent
3844
OPENAPI_PATH = REPO_ROOT / "api-reference" / "openapi.yaml"
@@ -292,6 +298,29 @@ def determine_output_path(gap, family_name, content=None):
292298
return None # missing_description handled inline; non-generative types skipped
293299

294300

301+
def _predicted_path(gap):
302+
"""The repo-relative path a gap would write to, or None if unpredictable.
303+
304+
Used only for open-PR matching. For missing_howto / missing_group_coverage the
305+
filename comes from the title the model invents, so there's nothing to predict
306+
and those gaps are matched on their gap key instead.
307+
"""
308+
out_path = determine_output_path(gap, gap.get("family", "unknown"))
309+
if not out_path:
310+
return None
311+
try:
312+
return str(out_path.relative_to(REPO_ROOT))
313+
except ValueError:
314+
return None
315+
316+
317+
def _pr_review_hint(claimed):
318+
"""One line pointing at the PRs worth reviewing, for the skip summary."""
319+
numbers = sorted({pr.get("number") for _, pr, _ in claimed if pr.get("number")})
320+
listed = ", ".join(f"#{n}" for n in numbers)
321+
return f"Review or close {listed} to let the pipeline redraft these."
322+
323+
295324
def apply_description(gap, description):
296325
"""Insert a frontmatter description into an existing page."""
297326
page_path = REPO_ROOT / gap["path"]
@@ -332,6 +361,10 @@ def main():
332361
parser.add_argument("--dry-run", action="store_true", help="Show what would be generated")
333362
parser.add_argument("--force", action="store_true", help="Regenerate even when files already exist")
334363
parser.add_argument("--gap-report", help="Path to existing gap report JSON (skips re-running detection)")
364+
parser.add_argument(
365+
"--ignore-open-prs", action="store_true",
366+
help="Draft every gap, even ones an open PR already covers (default: skip those)",
367+
)
335368
args = parser.parse_args()
336369

337370
if args.gap_report:
@@ -348,7 +381,33 @@ def main():
348381

349382
if not gaps:
350383
print("No gaps to generate for.")
351-
return 0
384+
return EXIT_NOTHING_TO_DO
385+
386+
# Drop gaps that an open PR already covers. Done here, before any model call,
387+
# so a duplicate run costs nothing instead of a full generate → review cycle
388+
# that ends in a PR nobody wants.
389+
if args.ignore_open_prs:
390+
print("Skipping the open-PR check (--ignore-open-prs).")
391+
else:
392+
prs, err = fetch_open_prs()
393+
if prs is None:
394+
print(f"Warning: could not check open PRs ({err}).")
395+
print(" Proceeding without deduplication — this run may duplicate an open PR.")
396+
else:
397+
gaps, claimed = split_claimed_gaps(
398+
gaps, prs,
399+
path_for_gap=_predicted_path,
400+
)
401+
if claimed:
402+
print(f"Skipping {len(claimed)} gap(s) already covered by an open PR:")
403+
for gap, pr, reason in claimed:
404+
print(f" - {gap['type']} ({gap.get('family', 'site-wide')}): {reason}")
405+
print(f" {_pr_review_hint(claimed)}")
406+
407+
if not gaps:
408+
print("\nEvery detected gap is already covered by an open PR. Nothing to do.")
409+
print("Merge or close those PRs, then re-run the pipeline.")
410+
return EXIT_NOTHING_TO_DO
352411

353412
standards = yaml.safe_load(open(STANDARDS_PATH))
354413
families = standards.get("families", {})

0 commit comments

Comments
 (0)