Repository navigation
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
rwgk
added
enhancement
ChatGPT Codex Connector
280c425 to
672e513
Compare
|
/ok to test 672e513 |
|
672e513 to
d24e19f
Compare
Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
d24e19f to
983ac75
Compare
rwgk
changed the title
rwgk
added
the
PR review get-together
📝 Summary
Priority: ⬇️ Low Change: Feature Merge Risk: 🟡 Moderate · up to
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments: 1
ci/tools/run_lychee.py: 1
66-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winsuggestion: Exercise nonempty remaps at the subprocess boundary.
The
run_lychee.main()tests replacerepo_link_remapswith[], so they do not detect a regression that drops remap options beforesubprocess.run. The helper tests cover remap construction separately, but not this caller wiring. Add nonempty remaps to both authored and rendered tests and assert their exact position in the Lychee command.Suggested fix
- patch("run_lychee.repo_link_remaps", return_value=[]), + patch("run_lychee.repo_link_remaps", return_value=["--remap", "source=target"]), ... - assert command[1:4] == ["--format", "json", "--output"] - report = Path(command[4]) + assert command[1:3] == ["--remap", "source=target"] + assert command[3:6] == ["--format", "json", "--output"] + report = Path(command[5]) ... - patch("run_lychee.repo_link_remaps", return_value=[]), + patch("run_lychee.repo_link_remaps", return_value=["--remap", "source=target"]), ... [ str(tmp_path / "lychee"), + "--remap", + "source=target", "--files-from",
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: NVIDIA/cuda-python/.coderabbit.yaml
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
13a83dd1-9c60-4b9d-84a2-a179372b52cb
📒 Files selected for processing: 9
.github/workflows/build-docs.yml.github/workflows/lychee.yml.github/workflows/pre-commit.ymlci/README.mdci/tools/checkout_link_defaults.pyci/tools/lychee_cache.pyci/tools/run_lychee.pyci/tools/tests/test_checkout_link_defaults.pyci/tools/tests/test_run_lychee.py
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.
| # BEGIN SYNCED LYCHEE REPOSITORY METADATA | ||
| env: | ||
| LYCHEE_REPOSITORY: ${{ github.repository }} | ||
| LYCHEE_DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
fd -e yml -e yaml . .github/workflows --exec sh -c 'echo "== $1"; sed -n "1,45p" "$1" | rg -n "on:|schedule|cron|workflow_call|workflow_dispatch|push|pull_request|uses: ./.github/workflows/(lychee|pre-commit)" ' _ {}Repository: NVIDIA/cuda-python
Length of output: 6774
🏁 Script executed:
set -e
printf '%s\n' '== workflow triggers and caller references =='
for f in .github/workflows/lychee.yml .github/workflows/pre-commit.yml .github/workflows/ci-nightly.yml .github/workflows/ci.yml .github/workflows/build-docs.yml; do
echo "== $f"
nl -ba "$f" | sed -n '1,180p'
done
printf '%s\n' '== relevant helper and prepare_ci references =='
rg -n -F --glob '*.py' -e 'def _branch' -e 'LYCHEE_DEFAULT_BRANCH' -e 'prepare_ci' ciRepository: NVIDIA/cuda-python
Length of output: 41037
🏁 Script executed:
set -e
nl -ba ci/tools/checkout_link_defaults.py | sed -n '35,205p'Repository: NVIDIA/cuda-python
Length of output: 8299
🌐 Web query:
GitHub Actions schedule event payload empty github.event.repository default_branch reusable workflow github context caller
💡 Result:
Yes—**a `schedule` event has no webhook payload**, so `github.event.repository.default_branch` is not available. GitHub’s event reference lists the schedule payload as “Not applicable”; the event object isn’t a repository object. ([github.com](https://github.com/github/docs/blob/main/content/actions/reference/workflows-and-actions/events-that-trigger-workflows.md?utm_source=openai))
In a reusable workflow called by that scheduled workflow, **`github` still refers to the caller’s context**. So `github.event.repository.default_branch` remains unavailable there; calling a reusable workflow doesn’t add the missing payload fields. ([docs.github.com](https://docs.github.com/en/actions/reference/workflows-and-actions/reusing-workflow-configurations?utm_source=openai))
Use the caller’s `github.repository` to identify the repo and, if you need its default branch, query repository metadata (for example, with the GitHub API). The event’s `github.ref` is the default branch ref for a scheduled run, but it is not itself the `default_branch` property. ([github.com](https://github.com/github/docs/blob/main/content/actions/reference/workflows-and-actions/events-that-trigger-workflows.md?utm_source=openai))
Citations:
- 1: https://github.com/github/docs/blob/main/content/actions/reference/workflows-and-actions/events-that-trigger-workflows.md?utm_source=openai
- 2: https://docs.github.com/en/actions/reference/workflows-and-actions/reusing-workflow-configurations?utm_source=openai
- 3: https://github.com/github/docs/blob/main/content/actions/reference/workflows-and-actions/events-that-trigger-workflows.md?utm_source=openai
🏁 Script executed:
set -e
printf '%s\n' '== workflow references =='
rg -n -F --glob '*.yml' --glob '*.yaml' -e './.github/workflows/lychee.yml' -e './.github/workflows/pre-commit.yml' -e './.github/workflows/build-docs.yml' .github/workflows
printf '%s\n' '== metadata blocks =='
for f in .github/workflows/lychee.yml .github/workflows/pre-commit.yml .github/workflows/build-docs.yml; do
echo "== $f"
rg -n -C 8 -F 'LYCHEE_DEFAULT_BRANCH:' "$f"
doneRepository: NVIDIA/cuda-python
Length of output: 3010
important: Add a schedule fallback for the default branch.
ci-nightly.yml runs on schedule and calls lychee.yml. Scheduled events do not provide github.event.repository.default_branch, so the helper receives an empty branch and fails before the nightly link checks run.
pre-commit.yml is not scheduled, and build-docs.yml guards this step to pull-request refs. Apply the same fallback to all synced blocks:
Suggested fix
--- a/.github/workflows/lychee.yml
+++ b/.github/workflows/lychee.yml
@@
- LYCHEE_DEFAULT_BRANCH: ${{ github.event.repository.default_branch }}
+ LYCHEE_DEFAULT_BRANCH: ${{ github.event.repository.default_branch || (github.event_name == 'schedule' && github.ref_name) || '' }}
--- a/.github/workflows/pre-commit.yml
+++ b/.github/workflows/pre-commit.yml
@@
- LYCHEE_DEFAULT_BRANCH: ${{ github.event.repository.default_branch }}
+ LYCHEE_DEFAULT_BRANCH: ${{ github.event.repository.default_branch || (github.event_name == 'schedule' && github.ref_name) || '' }}
--- a/.github/workflows/build-docs.yml
+++ b/.github/workflows/build-docs.yml
@@
- LYCHEE_DEFAULT_BRANCH: ${{ github.event.repository.default_branch }}
+ LYCHEE_DEFAULT_BRANCH: ${{ github.event.repository.default_branch || (github.event_name == 'schedule' && github.ref_name) || '' }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| LYCHEE_DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} | |
| LYCHEE_DEFAULT_BRANCH: ${{ github.event.repository.default_branch || (github.event_name == 'schedule' && github.ref_name) || '' }} |
Source: Path instructions
This branch has not been deployed
1 participant
Highlights
Let Lychee validate default-branch (
main) GitHub links against the proposed checkout, so new or moved paths can pass link checks before merging.ctk-next. Each URL is matched against its own repository's default branch.This builds on #3142's shared Lychee pre-commit hooks, adding repository-link handling to the existing development, PR, documentation-build, and nightly checks.
How it works
For example, a PR can add
new-directory/and immediately link tohttps://github.com/NVIDIA/cuda-python/tree/main/new-directory. Lychee validates the proposed target before it exists on remotemain.The checked-out commit can be a CI mirror or merge revision. The current working branch and PR base do not determine which source URLs are remapped.
Scope and tradeoffs
Remapping applies to
https://github.com/.../blob/<default-branch>/...and.../tree/<default-branch>/...links naming the canonical public repository or the selected checkout repository. Version tags, explicit commits, other branches, unrelated repositories, and URLs embedded inside other URLs retain their targets.Local hooks discover defaults from cached Git remote
HEADreferences, without making discovery requests. A checkout may therefore need its remote metadata initialized or refreshed. Missing, dangling, or conflicting defaults fail with specific repair instructions. Rendered checks still require the checked-out commit to be available on GitHub; authored checks cover local unpublished edits.These rules model the proposed default-branch tree. Maintenance branches need an explicit policy before adopting them when their links intentionally refer to a different tree.
Implementation details and suggested review order
Repository discovery and matching
CI prepares repository metadata once per job, using the workflow event's repository identity and default branch. When checking another repository, preparation also queries the public repository's advertised default branch once. Each hook invocation reuses that metadata.
Local discovery selects
upstreamwhen present, otherwiseorigin. An explicitgit config lychee.repository OWNER/REPOSITORYselects the repository when a different arrangement is needed. Matching remotes supply its cached default; the helper reports commands such asgit remote set-head upstream --autowhen a refresh is required. Synchronized checkouts also need cached default metadata for the public repository.The wrapper passes anchored
--remaprules to native Lychee. Rules match the supported repository and its complete default-branch prefix, preserving the remaining path, query, and fragment. Authored rules point to file URIs; rendered rules use the current repository andgit rev-parse HEAD.The new helper participates in the cache policy hash, so caches produced under the previous mapping policy are not reused. The three-day lifetime, separate authored/rendered caches, and publication policy come from #3142.
Suggested review order
LYCHEE REPOSITORY METADATAsynchronized block.Validation
All-files pre-commit passed locally on 983ac757fea1, after restacking on the refreshed #3142. The feature diff is unchanged. The
LYCHEE REPOSITORY METADATAblocks inpre-commit.yml,lychee.yml, andbuild-docs.ymlwere compared byte-for-byte again.Earlier validation of this implementation
Before the latest stack refresh:
Checklist