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
52 changes: 27 additions & 25 deletions bot/AUTHOR_POLICY.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,48 +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_acc 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_acc 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 KV cache comparison`).
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 |
| Design decision | `bot/DESIGN_PRINCIPLES.md` — append if needed |
| 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.
| 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`.
44 changes: 35 additions & 9 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_acc 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 acc logic, no external dependencies.
- Integration tests in [xPyD-integration](https://github.com/xPyD-hub/xPyD-integration) — cross-component tests.
- Unit tests in `tests/` — pure bench logic, no external dependencies.
- 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
- acc is a diagnostic tool for PD accuracy issues. No server components.
- All inference backend interaction goes through xPyD-integration tests.
- Follow step-by-step isolation approach (see [DESIGN_PRINCIPLES.md](DESIGN_PRINCIPLES.md)).
- acc is a PD accuracy diagnostic tool. No server components.
- Follow vLLM bench CLI compatibility (see [DESIGN_PRINCIPLES.md](DESIGN_PRINCIPLES.md)).
85 changes: 46 additions & 39 deletions bot/REVIEW_POLICY.md
Original file line number Diff line number Diff line change
@@ -1,65 +1,72 @@
<!-- CRITICAL: DO NOT SUMMARIZE OR COMPRESS THIS FILE -->
<!-- This file contains precise rules that must be read in full. -->

# Review Policy
# Review Policy — xPyD-acc

## 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 |
| Implementer | `hlin99` | Write code, submit PRs |
| Reviewer 1 | `hlin99-Review-Bot` | Review PRs |
| Reviewer 2 | `hlin99-Review-BotX` | Review PRs |

## Timing
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) |
| Total round timeout | 1 hour from PR creation |
| Round timeout action | close PR (iteration failed) |

## Review Criteria
## 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)

Reviewers evaluate each PR on two dimensions:
Before looking at any code, evaluate the design:

### 1. Idea Value
- Is the direction/approach valuable for the project?
- Does it align with the project goals?
- **If NO → close PR immediately** (one close = PR rejected)
- **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.

### 2. Code Quality
- Is the code correct?
- Are tests included/passing?
- Is `bot/iterations/current.md` updated with clear description?
- Does `docs/guide.md` reflect changes (if applicable)?
- **If idea is good but code has issues → request changes**
**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.

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

| Scenario | Action |
|----------|--------|
| Both reviewers approve | Auto-merge |
| One approves, one requests changes | Implementer fixes, reviewers re-review |
| Either reviewer closes | PR closed, iteration failed |
| Both approve after fixes | Auto-merge |
| Timeout (15min no review) | PR closed, iteration failed |
| Total timeout (1 hour) | PR closed, iteration failed |
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.

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

Every PR MUST update `bot/iterations/current.md` with:
- What was done this iteration
- Result: merged / closed (with reason)
- Reviewer scores/comments summary
| 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. |

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

- 2 approvals from designated reviewers
- CI passes (all checks green)
- No unresolved review comments
## 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