diff --git a/bot/AUTHOR_POLICY.md b/bot/AUTHOR_POLICY.md index 54c6d36..3802f4a 100644 --- a/bot/AUTHOR_POLICY.md +++ b/bot/AUTHOR_POLICY.md @@ -1,23 +1,55 @@ - + + # Author Policy — xPyD-plan -## Code Standards +Rules for the bot that writes code and submits PRs. -- **Language:** Python 3.10+ -- **Lint:** `ruff check xpyd_plan tests` -- **Type hints:** Required for all public APIs -- **Tests:** Required for every new feature or bug fix +## Identity +| Role | GitHub Account | +|------|---------------| +| Author | `hlin99` | -## Git +## Before Coding +1. Pull latest main: `git pull origin main` +2. Create branch: `git checkout -b /` +3. Read [DESIGN_PRINCIPLES.md](DESIGN_PRINCIPLES.md) for architecture constraints. -- **Committer:** `hlin99 ` -- **Branch naming:** `feat/`, `fix/`, `docs/`, `refactor/` -- **Commit format:** Conventional Commits +## Code Quality +- Run pre-commit: `pre-commit run --all-files` +- Run lint: `ruff check xpyd_plan tests` +- Run tests: `pytest tests/ -q` +- All must pass locally before pushing. -## PR Checklist +## PR Submission +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. -- [ ] Code + tests -- [ ] `ruff check xpyd_plan tests` passes -- [ ] `bot/iterations/current.md` updated -- [ ] PR body contains `Closes #N` (if applicable) +## Responding to Review +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` | +| 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 iteration complete: rename `bot/iterations/current.md` to `YYYY-MM-DD-.md`, create fresh `current.md`. diff --git a/bot/BOT_POLICY.md b/bot/BOT_POLICY.md index 48cd598..88108f0 100644 --- a/bot/BOT_POLICY.md +++ b/bot/BOT_POLICY.md @@ -1,34 +1,49 @@ - + + # Bot Policy — xPyD-plan -## Identity - -- **Project:** xPyD-plan -- **Repo:** `xPyD-hub/xPyD-plan` -- **Architecture:** Offline planning tool — analyze vLLM/SGLang/TRT-LLM benchmark data to recommend optimal P:D instance ratios. - -## Accounts - -| Role | GitHub Account | -|------|---------------| -| Implementer | `hlin99` | -| Reviewer 1 | `hlin99-Review-Bot` | -| Reviewer 2 | `hlin99-Review-BotX` | - -## Rules - -1. **Never push directly to `main`.** All changes go through PRs. -2. **Rebase onto latest `main`** before every push. -3. **Run pre-commit** before every commit. -4. **Never self-merge.** Wait for both reviewer bots to approve. -5. **All code, docs, issues, PRs in English.** -6. **Conventional Commits** format for all commit messages. -7. **CI must be green** before merge. - -## References - -- `bot/DESIGN_PRINCIPLES.md` — what to build and why -- `bot/DEV_LOOP.md` — how to iterate -- `bot/REVIEW_POLICY.md` — review process -- `ROADMAP.md` — milestone tracker +## Language +- **English only** — all code, docs, issues, PRs, comments on GitHub must be in English. No Chinese characters. + +## 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: `: ` (fix, feat, test, docs, refactor, chore, ci). + +## Before Pushing +1. Run pre-commit: `pre-commit run --all-files` +2. Run lint: `ruff check xpyd_plan 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. + +## Testing +- 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 +- plan is an offline PD ratio planning tool. No server components. +- Follow vLLM bench CLI compatibility (see [DESIGN_PRINCIPLES.md](DESIGN_PRINCIPLES.md)). diff --git a/bot/ENTRY.md b/bot/ENTRY.md index 473c2db..3cf7125 100644 --- a/bot/ENTRY.md +++ b/bot/ENTRY.md @@ -1,14 +1,18 @@ - + + + -# Bot Entry Point — xPyD-plan +# Bot Entry Point -Read these files **in order** before every iteration: +Read this file first when starting any automated task on this repo. -1. `bot/BOT_POLICY.md` — project rules and constraints -2. `bot/AUTHOR_POLICY.md` — coding standards -3. `bot/DESIGN_PRINCIPLES.md` — architectural guidelines -4. `bot/DEV_LOOP.md` — iteration steps -5. `bot/REVIEW_POLICY.md` — PR review process -6. `bot/iterations/current.md` — current state +## Required Reading (in order) -**Do not skip any file. Do not summarize from memory.** +1. **[BOT_POLICY.md](BOT_POLICY.md)** — Hard rules. Must follow. +2. **[AUTHOR_POLICY.md](AUTHOR_POLICY.md)** — Rules for writing code and submitting PRs. +3. **[REVIEW_POLICY.md](REVIEW_POLICY.md)** — Rules for reviewing PRs. +4. **[DESIGN_PRINCIPLES.md](DESIGN_PRINCIPLES.md)** — Architecture constraints and design rules. +5. **[DEV_LOOP.md](DEV_LOOP.md)** — Development workflow (operational steps, references policies above). +6. **[iterations/current.md](iterations/current.md)** — Current task context. + +Files 1-4 are mandatory for all repos. Files 5-6 are repo-specific. diff --git a/bot/REVIEW_POLICY.md b/bot/REVIEW_POLICY.md index cfd721f..c05e142 100644 --- a/bot/REVIEW_POLICY.md +++ b/bot/REVIEW_POLICY.md @@ -1,38 +1,72 @@ - + + -# Review Policy +# Review Policy — xPyD-plan ## Roles +| Role | GitHub Account | Action | +|------|---------------|--------| +| Implementer | `hlin99` | Write code, submit PRs | +| Reviewer 1 | `hlin99-Review-Bot` | Review PRs | +| Reviewer 2 | `hlin99-Review-BotX` | Review PRs | -| Role | GitHub Account | -|------|---------------| -| Implementer | `hlin99` | -| Reviewer 1 | `hlin99-Review-Bot` | -| Reviewer 2 | `hlin99-Review-BotX` | +Each reviewer uses its own dedicated token. Never use author's token for reviews. -## Review Criteria +## 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 (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) | -### 1. Idea Value -- Does the direction align with project goals? -- **If NO → close PR immediately** (one close = PR rejected) +## 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. -### 2. Code Quality -- Correct code, tests included/passing -- `bot/iterations/current.md` updated -- **If idea good but code has issues → request changes** +## Review Process: Two-Stage Gate -## Decision Rules +### Stage 1: Design Review (Gate) -| Scenario | Action | -|----------|--------| -| Both approve | Auto-merge | -| One approves, one requests changes | Fix, re-review | -| Either closes | PR closed, iteration failed | -| Timeout (15 min no review) | PR closed | -| Total timeout (1 hour) | PR closed | +Before looking at any code, evaluate the design: -## Auto-Merge Requirements +- **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 approvals from designated reviewers -- CI green -- No unresolved review comments +**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. + +## 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.