Skip to content

docs(exploration): changelog PR-number resolution robustness (0202) - #190

Closed
crs48 wants to merge 1 commit into
mainfrom
claude/changelog-pr-linking
Closed

docs(exploration): changelog PR-number resolution robustness (0202)#190
crs48 wants to merge 1 commit into
mainfrom
claude/changelog-pr-linking

Conversation

@crs48

@crs48 crs48 commented Jun 18, 2026

Copy link
Copy Markdown
Owner

Exploration 0202 — Changelog PR-number resolution robustness

Investigates the report that "PR linking broke" — changelog entries showing a title/slug but no PR number, and seeming not to render.

Key finding: PR linking is not actually broken. Verified live: the page renders pull/187 for the OpenRouter entry (and all 33 recent entries have a pull/<N> link), and the JSON feed exposes _xnet.pr: 187 for every entry. resolve-prs.mjs runs at deploy and resolves correctly.

What made it look broken (all real, none a pipeline failure):

  1. Fragment ids changed from prNNN → title slugs (metered-ai-through-openrouter), so the number is no longer in the id (it's resolved into the pr field at deploy).
  2. Server/docs PRs have no visual-capture gallery, so they render sparse next to UI PRs.
  3. The post-merge deploy window — resolution only happens at deploy, so there's a few-minute lag where the live site is the previous build.

The legitimate concern: resolution is deploy-only, regex-based (parses merge-commit subjects), silent on failure, and absent from the repo / PR previews / local builds. It works today only because we merge with gh pr merge (subject carries (#N)).

Recommendation (no second commit, minimal added complexity):

  • Make resolve-prs authoritative via the GitHub commits→PRs API (merge-method-proof), git-subject as offline fallback.
  • Make it loud (::warning + log on any unresolved fragment) instead of silently shipping bare entries.
  • Run resolve in preview builds too.
  • Opportunistically bake pr at authoring time via gh pr view --json number (still one commit) for repo/preview/local visibility.
  • Add a "View PR #N →" affordance for image-less entries; fix the new.mjs UTC-tomorrow date bug.

Docs-only exploration — no user-facing change, so skip-changelog.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@crs48
crs48 temporarily deployed to pr-190 June 18, 2026 05:08 — with GitHub Actions Inactive
@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

✓ Changelog fragment found — thanks!

@crs48 crs48 added the skip-changelog Exclude this PR from the changelog label Jun 18, 2026
@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Preview removed for PR #190.

@crs48

crs48 commented Jun 18, 2026

Copy link
Copy Markdown
Owner Author

Superseded by #191, which folds in this exploration doc plus the implementation (checked off).

@crs48 crs48 closed this Jun 18, 2026
github-actions Bot added a commit that referenced this pull request Jun 18, 2026
crs48 added a commit that referenced this pull request Jun 18, 2026
## Summary

Implements exploration **0202** — makes changelog PR-number resolution
**robust and visible** without adding a second commit. (Supersedes the
doc-only PR #190, which is folded in here.)

The investigation found PR linking *wasn't* actually broken — the
deploy-time `resolve-prs.mjs` works — but it was fragile (regex over
merge-commit subjects), silent on failure, and entries without
screenshots looked bare. This hardens all three.

### Robustness — `resolve-prs.mjs` + deploy
- **Authoritative API fallback:** when the git-subject parse misses,
resolve via the GitHub `commits/{addSha}/pulls` API (keyed by the
fragment's adding commit) — **merge-method-proof**, so rebase-merges
resolve too. Verified live: `commits/4f413f61/pulls → #187`.
- **Fail-loud:** unresolved fragments now emit a `::warning` annotation
+ summary line instead of silently shipping numberless. Never blocks the
deploy (entries still render).
- `deploy-site.yml` passes `GITHUB_TOKEN` to the step.

### Authoring — `new.mjs`
- **`--pr auto`** (default) bakes the PR number via `gh pr view` when
the branch already has a PR — one commit, visible in the
repo/previews/local. Omitted when no PR exists yet → the deploy resolves
it. (`--pr <N>` / `--pr none` override.) **This fragment was added that
way.**
- Fixes the **UTC-tomorrow date drift** (stamps the local day).

### UI — `index.astro`
- A **"View PR #N →"** affordance for entries that have a PR but no
gallery, so server/docs changes don't read as broken. (Built HTML shows
it for #187/#186/#184…)

### Notably *not* done (a finding, not a gap)
- The exploration suggested running resolve in the preview workflows.
They build only `apps/web`, **not** the Astro `site/`, so the changelog
page is never in previews — running resolve there would be dead code.
Skipped to avoid useless pipeline surface.

## Testing
- `resolve-prs.mjs` resolves all current slug entries via git; API
fallback verified live against the real endpoint.
- `new.mjs`: local-date + graceful no-PR + bake-on-`--pr` all
smoke-tested.
- `site/` builds (70 pages); "View PR #187" affordance present in the
built changelog HTML.
- Not eslint/prettier-gated (lint covers `packages apps`; `format:check`
skips `scripts/site`), but formatted and syntax-checked anyway.

Exploration:
`docs/explorations/0202_..._CHANGELOG_PR_NUMBER_RESOLUTION_ROBUSTNESS.md`
(items checked off).

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog Exclude this PR from the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant