Skip to content

[SC-18090] Treat a missing partial index as empty when merging Quarto indexes - #1474

Merged
juanmleng merged 1 commit into
mainfrom
juan/sc-18090/fix-targeted-preview-listings-merge
Aug 26, 2026
Merged

[SC-18090] Treat a missing partial index as empty when merging Quarto indexes#1474
juanmleng merged 1 commit into
mainfrom
juan/sc-18090/fix-targeted-preview-listings-merge

Conversation

@juanmleng

Copy link
Copy Markdown
Contributor

Pull Request Description

What and why?

Docs pull requests that change only ordinary content pages fail the validate check. The page renders correctly, then the build dies:

FileNotFoundError: [Errno 2] No such file or directory: '_site/listings.json'

merge_file reads the partial index unconditionally, but Quarto only writes listings.json when a rendered page declares a listing: — the card grids on hub pages. A targeted render of pages that carry none produces no partial index, so the merge crashes, and it crashes before the preview upload step. Red check, no pr_previews/ URL, and a Python traceback on the author's PR that reads as the author's mistake.

Before: a missing partial index is a hard error. After: it means "no listings changed, nothing to merge" — the staging base is written out unchanged, which is the correct index for a PR that rendered no listing pages.

This is not a rare edge case. The targeted render path has never completed successfully. Of the last eight validate runs that executed, the three that took the targeted path all failed with this traceback, and the three that fell back to a full render all passed. Every green docs PR since the optimisation landed in 7e10dc6d (2026-07-13) got there by falling back, because it touched an underscore-prefixed include, a _sidebar.yaml, or something else outside site/. The fast path is reached only by exactly the kind of change it was built for, and that case fails closed — which is likely why this has gone six weeks unreported.

Currently blocked by it: documentation#1473, and two unrelated PRs from another author on branches panchicore/docs-sc17845-field-deletion and panchicore/docs-sc17216-pdf-parse-cancel, both with the identical traceback.

How to test

python3 -m unittest discover -s .github/scripts -p 'test_merge_quarto_indexes.py' -v

4 passed. The new case, test_absent_partial_leaves_base_as_the_merged_index, reproduces the production failure: with the fix stashed it errors with the same FileNotFoundError, and the suite goes to FAILED (errors=1). CI already runs this suite on every PR (validate-docs-site.yaml:168), so the change is covered here.

Selector behaviour was checked directly rather than assumed:

$ printf 'modified\tsite/guide/risk-tiering/manage-risk-tier-templates.qmd\t\n' | python3 .github/scripts/select_docs_preview_targets.py --changes - --targets /tmp/t --assets /tmp/a
Targeted render pages:
guide/risk-tiering/manage-risk-tier-templates.qmd

What needs special review?

The end-to-end path is still unverified, and this PR cannot verify it. select_docs_preview_targets.py fast-paths only files under site/, so any PR touching .github/ forces a full render — including this one. The unit test covers the fix; nothing here exercises the real targeted pipeline.

That matters more than usual, because everything downstream of the merge step has also never run on the targeted path: the preview upload, the preview comment, and whether links into a partially rendered site resolve. This unblocks the road; it does not prove the road is clear. documentation#1473 changes a single content page and does take the targeted path, so re-running validate on it after this merges is the first genuine exercise. Expect the possibility of a second problem behind this one.

Also worth a second opinion on the direction. Given a 0% completion rate over six weeks, the alternative is retiring the targeted path and always doing a full render, so the configuration matches what has actually been happening. That is cheaper to reason about than a fast path nobody has watched succeed, and it is a call for whoever owns the preview pipeline rather than mine.

One behavioural trade-off to name: after this change, "there were genuinely no listings" and "Quarto should have written listings and didn't" look identical, and both produce a preview carrying staging's grids. A loud failure becomes a quiet, slightly stale preview. The same tolerance now applies to search.json, which Quarto does appear to always write, so that path should not be reachable in practice.

Dependencies, breaking changes, and deployment notes

CI tooling only, no site content. No dependencies.

Release notes

Internal CI fix, no user-facing change.

Checklist

  • What and why
  • Screenshots or videos (Frontend)
  • How to test
  • What needs special review
  • Dependencies, breaking changes, and deployment notes
  • Labels applied
  • PR linked to Shortcut
  • Unit tests added (Backend)
  • Tested locally
  • Documentation updated (if required)
  • Environment variable additions/changes documented (if required)

merge_file read the partial index unconditionally, but Quarto only writes
listings.json when a rendered page declares a listing. A targeted preview
render of pages carrying no card listing therefore produces no partial index,
and the merge died with FileNotFoundError before the preview was uploaded — so
the check went red on exactly the kind of change the targeted path exists for.

No listings changed means nothing to merge: fall back to an empty list and
write the staging base out unchanged. The new test reproduces the production
traceback and fails without this change.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@juanmleng juanmleng added the internal Not to be externalized in the release notes label Aug 26, 2026
@juanmleng
juanmleng marked this pull request as ready for review August 26, 2026 12:36
@juanmleng
juanmleng requested a review from cachafla August 26, 2026 12:36
@juanmleng juanmleng self-assigned this Aug 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Validate docs site

✓ INFO: A live preview of the docs site is available — Open the preview

@github-actions

Copy link
Copy Markdown
Contributor

Lighthouse check results

✓ INFO: No site pages to audit in this PR.

Commit SHA: 95f8192

@juanmleng
juanmleng added this pull request to the merge queue Aug 26, 2026
Merged via the queue into main with commit 41d05a9 Aug 26, 2026
10 checks passed
@juanmleng
juanmleng deleted the juan/sc-18090/fix-targeted-preview-listings-merge branch August 26, 2026 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Not to be externalized in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants