Skip to content

fix(docs): fail the build on a product image with a rooted path - #1431

Merged
jfrench9 merged 1 commit into
mainfrom
feature/roboledger-docs-depth
Sep 18, 2026
Merged

jfrench9 merged 1 commit into
mainfrom
feature/roboledger-docs-depth

Conversation

@jfrench9

Copy link
Copy Markdown
Member

Summary

Follow-up to #1430, from its automated review. A product page image written with a rooted path, like ![](/images/plan.png), skipped the image check and shipped a link nothing serves. It now fails the build like any other image that isn't a file under the site's images/ folder.

Changes

  • robosystems/scripts/publish_docs.py: rewrite_product_links still passes a site path through untouched when it's a link ([pricing](/pricing)), but no longer when it's an image. A rooted image falls through to the existing check and is reported as image must be a file under images/ that exists.
  • tests/scripts/test_publish_docs.py: the missing-image test also covers a rooted image, and checks that a rooted link still passes.

None of the published pages use a rooted image path, so the published output is unchanged.

Breaking Changes

None.

Testing

  • uv run pytest tests/scripts/test_publish_docs.py tests/scripts/test_product_docs.py: 36 passed.
  • just docs-build: 54 pages, 63 files, no warnings or errors.
  • The pre-commit hook ran ruff, format and basedpyright clean.
  • The full just test-all unit suite was not run.

Certification

  • I have the right to submit this work under the Apache 2.0 license, and do so. Where any part of it is owned by my employer, I have their permission.

🤖 Generated with Claude Code

A target starting with / passed through before the image check, so a page
showing /images/x.png shipped a link nothing serves. A site path stays a
valid link; as an image it is now the same build error as any other file
that is not under images/.
@github-actions github-actions Bot added the change:normal Normal change: automated review documented on the pull request before merge label Sep 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Change classification: normal — platform or application code changed.

A normal change carries a documented automated review on this pull request before merge, in place of a second human approver. The change-classification job requests that review and passes once it has been posted. Recorded by the change-classification job.

@jfrench9

Copy link
Copy Markdown
Member Author

@claude please review this PR

@claude

claude Bot commented Sep 18, 2026

Copy link
Copy Markdown

Claude finished @jfrench9's task in 44s —— View job


Review: fix(docs): fail the build on a product image with a rooted path

  • Read PR description and context
  • Examine diff in publish_docs.py and test changes
  • Review logic correctness and edge cases
  • Post review feedback

Verdict: LGTM. Small, correct, well-tested fix.

The change (robosystems/scripts/publish_docs.py:551-558, in rewrite_product_links):

if target.startswith(("http://", "https://", "mailto:", "#")):
  return match.group(0)
# A site path is a fine link and a broken image: nothing serves it.
if target.startswith("/") and not bang:
  return match.group(0)
if bang:
  if target not in (media or {}):
    ...

Previously "/" was lumped into the same early-return tuple as http(s):///mailto:/#, so a rooted target skipped the image-exists check entirely regardless of whether it was a link or an image (!). Gating the early return on not bang means:

  • A rooted link ([pricing](/pricing)) still passes through untouched — correct, since it's a valid site path.
  • A rooted image (![](/images/plan.png)) now falls into the if bang: branch, where target not in media is always true for a rooted path (media keys are relative, e.g. images/plan.png), so it correctly reports image must be a file under images/ that exists: /images/plan.png and fails the build instead of silently shipping a dead link.

Tests (tests/scripts/test_publish_docs.py:367-382): test_a_missing_or_misplaced_image_is_an_error now includes a rooted image (asserted as a new error, matching the existing message format for images/gone.png and ../shot.png) and a rooted link ([a site path](/pricing)) in the same input, with the exact build.errors list asserted — so the absence of a 4th error for the link is implicitly verified too. Good use of an exhaustive-list assertion to cover both the positive and negative case in one test.

No issues found. The PR body's own testing notes (36 tests passed, just docs-build clean, pre-commit clean) line up with what I see in the diff. CI (Test CI, PR #1431) is currently in progress on this PR at the time of this review.
· branch

@jfrench9
jfrench9 merged commit d2d1356 into main Sep 18, 2026
8 checks passed
@jfrench9
jfrench9 deleted the feature/roboledger-docs-depth branch September 18, 2026 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:normal Normal change: automated review documented on the pull request before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant