Skip to content

Commit c8f7ce9

Browse files
authored
docs: add PR rebase guidelines and CI failure reproduction tips (#7298)
Signed-off-by: Keiven Chang <keivenchang@users.noreply.github.com> Co-authored-by: Keiven Chang <keivenchang@users.noreply.github.com>
1 parent 9f01dd2 commit c8f7ce9

File tree

3 files changed

+84
-0
lines changed

3 files changed

+84
-0
lines changed

.ai/pr-workflow-guidelines.md

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
# PR Workflow Guidelines
2+
3+
Conventions for keeping pull requests healthy and reviewable.
4+
5+
## Keep Your Branch Close to Main
6+
7+
Be aware of PRs targeting `main` that are more than ~25 commits behind.
8+
Stacked PRs targeting another branch are exempt.
9+
10+
### 1. Slower CI builds
11+
12+
CI builds start from a base image matching a recent `main` commit. The closer
13+
your branch is to `main`, the more cache hits you get. We use BuildKit with
14+
remote cache (`--cache-from`), but BuildKit layers are still content-addressed
15+
-- if a `COPY` brings in source that changed since your branch point, that
16+
layer and everything downstream rebuilds regardless. When your branch is far
17+
behind:
18+
19+
- **Docker layer cache misses**: Changed source or Dockerfile instructions
20+
since your branch point invalidate cached layers, forcing rebuilds
21+
downstream.
22+
- **Rust compilation cache misses**: More crate sources differ from what's
23+
cached, so more crates recompile from scratch.
24+
- **Dependency drift**: `Cargo.lock` and `requirements.txt` evolve on `main`.
25+
Stale branches pull older versions that aren't cached, triggering fresh
26+
downloads and builds.
27+
28+
A branch too far behind `main` can trigger a near-cold build that takes
29+
45-60 minutes. A branch close to `main` reuses most of the cache and builds
30+
in a fraction of that time.
31+
32+
### 2. Reviewer burden
33+
34+
Merge commits from main pollute the diff with unrelated changes. Rebasing
35+
produces a clean, linear history where every commit is the PR author's.
36+
37+
```bash
38+
# BAD -- merge pulls in unrelated files; reviewer has to filter them out
39+
git merge main
40+
41+
# GOOD -- clean diff showing only your work
42+
git fetch origin && git rebase origin/main
43+
```
44+
45+
### Guidance
46+
47+
- **Rebase when you are more than ~25 commits behind main.**
48+
- **Prefer `rebase` over `merge`** for linear history. Force-push after
49+
rebasing (`git push --force-with-lease`).
50+
- **Before requesting review**, check your distance from main:
51+
```bash
52+
git fetch origin
53+
git rev-list --count HEAD..origin/main
54+
```
55+
More than 25? Rebase first.
56+
- **If CI is slow**, check your base commit age before debugging other causes.
57+
- **Stacked PRs** are exempt. If a PR targets another branch in a stack,
58+
distance from `main` is expected and not a problem until the stack lands.

.ai/pytest-guidelines.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,27 @@ Rules and conventions for Python tests in this repository.
88
wastes shared GPU minutes and blocks other PRs. A local run catches most
99
failures in seconds.
1010

11+
### Reproducing CI failures locally
12+
13+
Don't guess from log snippets. Pull the same container image CI built and
14+
reproduce the failure locally:
15+
16+
```bash
17+
# 1. Find the image:tag in the CI job's "docker build" or "docker push" step.
18+
19+
# 2. Run it with GPU access:
20+
docker run --rm -it --gpus all <ci-image>:<tag> bash
21+
22+
# 3. Run the failing test:
23+
python3 -m pytest -xvv tests/path/to/test_that_failed.py::test_name
24+
```
25+
26+
If the test passes locally in the CI container, the failure is likely a
27+
resource or timing issue in CI. If it fails, you have an exact reproduction.
28+
29+
Pull the container, reproduce, fix, verify -- in that order. Don't make
30+
speculative fixes from logs.
31+
1132
Always use the venv-aware invocation -- never bare `pytest`:
1233

1334
```bash

.coderabbit.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ reviews:
2828
unit_tests:
2929
enabled: false
3030
path_instructions:
31+
- path: "**"
32+
instructions: |
33+
Flag PRs more than ~25 commits behind main. Recommend rebasing.
34+
Exception: stacked PRs targeting another branch are exempt.
35+
See .ai/pr-workflow-guidelines.md.
3136
- path: "**/*.py"
3237
instructions: "Review against .ai/python-guidelines.md"
3338
- path: "**/tests/**/*.py"

0 commit comments

Comments
 (0)