Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 27 additions & 27 deletions bot/AUTHOR_POLICY.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,50 +6,50 @@
Rules for the bot that writes code and submits PRs.

## Identity

| Role | GitHub Account |
|------|---------------|
| Author | `hlin99` |

## Before Coding

1. Pull latest main: `git pull origin main`
2. Create feature branch: `git checkout -b <type>/<short-description>`
2. Create branch: `git checkout -b <type>/<short-description>`
3. Read [DESIGN_PRINCIPLES.md](DESIGN_PRINCIPLES.md) for architecture constraints.

## Code Quality

1. Run lint: `ruff check xpyd_bench tests`
2. Run tests: `pytest tests/ -q`
3. Rebase before push: `git pull --rebase origin main`
- Run pre-commit: `pre-commit run --all-files`
- Run lint: `ruff check xpyd_bench tests`
- Run tests: `pytest tests/ -q`
- All must pass locally before pushing.

## PR Submission

1. **One PR per task.** Don't bundle unrelated changes.
2. **Descriptive title.** Format: `type: short description` (e.g., `feat: add SLA validation`).
3. **PR body must include:** what changed, why, test coverage, breaking changes.
4. **All CI must pass** before requesting review.
1. One PR per task. Don't bundle unrelated changes.
2. Descriptive title: `type: short description`
3. PR body: what changed, why, test coverage, breaking changes. Reference issues (`closes #N`).
4. All CI must pass before requesting review.

## Responding to Review

1. Fix all blockers before re-requesting review.
2. Reply to every comment — "Fixed in <commit>" or explain disagreement with evidence.
3. Push new commits (don't force push over reviewer comments).
4. **Never force push.** If the branch is too messy, close the PR and open a new one.
1. Address ALL `REQUEST_CHANGES` feedback before requesting re-review.
2. Always push new commits — never amend or force-push.
3. Reply to each review comment with fix commit ref (e.g. "Fixed in `abc1234`").
4. Re-request review after pushing fixes (don't wait for reviewer to notice).
5. If reviewer is wrong, explain with evidence (link to source, docs, spec).

## PR Maintenance
When there are open (non-draft) PRs, check every 5 minutes:
1. Update branch — if behind main, merge main into branch.
2. CI check — if any check failed, examine logs, fix code, push new commit.
3. Review comment check — read all `CHANGES_REQUESTED` reviews, fix issues, push, reply.
4. Re-request review after fixes.

## Documentation Updates

Every PR must update relevant documentation:

| Change Type | Update |
|---|---|
| New feature / CLI argument | `docs/guide.md` — add usage section |
| Architecture change | `docs/architecture.md` — update descriptions |
| Design decision | `docs/design.md` — append decision record |
| Quick Start affected | `README.md` — update (keep it one screen max, link to guide.md) |
| PR completed | `bot/iterations/current.md` — append summary |

`docs/guide.md` is the source of truth for how to use the tool.
`docs/architecture.md` and `docs/design.md` are append-only — never delete history.
| New feature / CLI argument | `docs/guide.md` |
| Architecture change | `docs/architecture.md` |
| Design decision | `docs/design.md` |
| Quick Start affected | `README.md` (keep one screen, link to guide.md) |
| PR completed | `bot/iterations/current.md` |

When current iteration is complete, rename `bot/iterations/current.md` to `YYYY-MM-DD-<topic>.md` and create a fresh `current.md`.
When iteration complete: rename `bot/iterations/current.md` to `YYYY-MM-DD-<topic>.md`, create fresh `current.md`.
38 changes: 32 additions & 6 deletions bot/BOT_POLICY.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,44 @@
## Language
- **English only** — all code, docs, issues, PRs, comments on GitHub must be in English. No Chinese characters.

## Code Rules
- All changes go through PR. Never push directly to main.
- Every PR must have tests. No untested code.
## Branch Rules
- **Never push directly to main.** All changes go through PR.
- **Never force push.** If branch is too messy, close PR and open new one.
- Branch from latest main. Keep branch up-to-date by merging main into it.
- Each PR must be independent — no stacking PRs or branching off feature branches.

## Commit Rules
- **Commit identity**: `git -c user.name="hlin99" -c user.email="tony.lin@intel.com" commit -s`
- Always use `tony.lin@intel.com` as commit email. Never use noreply address.
- Always include `Signed-off-by` trailer (`-s` flag) for DCO compliance.
- Never add `Co-authored-by` trailers.
- Follow conventional commits: `<type>: <short description>` (fix, feat, test, docs, refactor, chore, ci).

## Before Pushing
1. Run pre-commit: `pre-commit run --all-files`
2. Run lint: `ruff check xpyd_bench tests`
3. Run tests: `pytest tests/ -q`
4. All three must pass locally before pushing.

## Merge Policy (Loop Mode)
- Bot MAY merge a PR after: both reviewers approve + CI is fully green.
- Bot MAY close a PR on reviewer timeout or iteration failure.
- Auto-merge is part of the autonomous development loop.

## CI
- CI must be 100% green before merge. No skips allowed.
- No test may be skipped. If a test can't run, fix it or remove it.
- Rebase to latest main before pushing.

## Testing
- Unit tests in `tests/` — pure bench logic, no external dependencies.
- Integration tests in [xPyD-integration](https://github.com/xPyD-hub/xPyD-integration) — cross-component tests.
- Integration tests in [xPyD-integration](https://github.com/xPyD-hub/xPyD-integration).

## Secrets
- Never hardcode tokens or credentials in code, PR descriptions, or bot prompts.

## Freshness
- **Always pull latest main and re-read BOT_POLICY.md before starting any work.** This is a living document. Never rely on cached copies.

## Architecture
- Bench is a pure client tool. No server components.
- All inference backend interaction goes through xPyD-integration tests.
- Follow vLLM bench CLI compatibility (see [DESIGN_PRINCIPLES.md](DESIGN_PRINCIPLES.md)).
66 changes: 53 additions & 13 deletions bot/REVIEW_POLICY.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,69 @@
# Review Policy — xPyD-bench

## Roles

| Role | GitHub Account | Action |
|------|---------------|--------|
| Implementer | `hlin99` | Write code, submit PRs, fix issues |
| Reviewer 1 | `hlin99-Review-Bot` | Review PRs: approve / request changes / close |
| Reviewer 2 | `hlin99-Review-BotX` | Review PRs: approve / request changes / close |

## Timing Parameters
| Implementer | `hlin99` | Write code, submit PRs |
| Reviewer 1 | `hlin99-Review-Bot` | Review PRs |
| Reviewer 2 | `hlin99-Review-BotX` | Review PRs |

These are the single source of truth for all timing values:
Each reviewer uses its own dedicated token. Never use author's token for reviews.

## Timing Parameters
| Parameter | Value |
|-----------|-------|
| Iteration interval | 10 minutes |
| PR wait for review | max 15 minutes |
| Fix after request changes | max 10 minutes |
| Reviewer check frequency | every 5 minutes |
| Reviewer check frequency (has PRs) | every 5 minutes |
| Reviewer check frequency (no PRs) | every 15 minutes |
| Reviewer response deadline | 15 minutes after assign |
| Reviewer timeout action | close PR (iteration failed) |

## Review Standards
## What to Review
1. Skip draft PRs.
2. Skip already-reviewed commits (only APPROVE counts as reviewed).
3. Re-requested reviews take priority — always perform fresh review.
4. One review per PR per commit SHA — never submit multiple reviews for same commit.

## Review Process: Two-Stage Gate

### Stage 1: Design Review (Gate)

Before looking at any code, evaluate the design:

- **Is this change valuable?** Does it solve a real problem? Is it worth the complexity?
- **Is the approach sound?** Is this the right way to solve it?
- **Does it match the linked Issue spec?** If the PR deviates from the agreed design, reject.

**If the design has no value or the approach is wrong → CLOSE the PR immediately.** Do not proceed to code review. Do not waste time reviewing code for a feature that shouldn't exist.

### Stage 2: Code Review (only after Stage 1 passes)

Only if the design is valuable and the approach is sound, review the code using the checklist below. Apply proxy-level strict standards — every line examined.

## Review Checklist
For each non-draft PR with a new commit:

| Area | Check |
|---|---|
| CI | Must be fully green before APPROVE. May submit REQUEST_CHANGES or COMMENT while pending. |
| Merge conflicts | If mergeable == false, REQUEST_CHANGES. |
| Logic errors | Incorrect conditions, off-by-one, unhandled edge cases. |
| Type safety | Mismatched types, missing None checks. |
| Concurrency | Race conditions, missing locks, shared mutable state. |
| Exception handling | Bare except, swallowed exceptions, resource leaks. |
| Security | Injection risks, hardcoded secrets, unsanitized input. |
| Code style | Unused imports, shadowed variables, unclear naming. |
| Test coverage | New logic must have corresponding tests. |
| Design conformance | Implementation must match the linked GitHub Issue design. |

## Verdicts
- **APPROVE** — code correct, CI green, no issues.
- **REQUEST_CHANGES** — any issue found. Use inline comments.
- **COMMENT** — CI pending or noting something without blocking.

- At least 1 approval required to merge.
- Blockers (🔴) must be fixed. No exceptions.
- Yellow (🟡) issues should be fixed unless author provides good reason.
- All CI checks must pass.
## Merge Policy (Loop Mode)
- Both reviewers approve + CI green → bot auto-merges.
- Reviewer timeout (15 min) → bot closes PR (iteration failed).
- Single approve is not enough to merge. Both must approve.
Loading