Pull Request Process

This page describes the complete process from submitting a PR to merging, including PR templates, code review standards, CI checks, merge strategies, and conflict resolution. All code changes must be merged into the main branch through the PR process.

Pre-Submission Checklist

Please confirm each item before submitting a PR:

• Branch is based on the latest main, conflicts resolved via rebase
• cargo fmt --all -- --check passes
• cargo clippy --all-targets -- -D warnings passes
• cargo nextest run all pass
• cargo nextest run -p <my-crate> all pass
• New features include tests, coverage not decreased
• Public API changes have updated documentation and CHANGELOG
• Commit messages follow Conventional Commits
• PR title and description are clear, with linked Issue
• No sensitive content such as keys, certificates, or temporary files
• No sensitive files such as .env, *.pem, *.key
• cargo deny check passes (no banned licenses or duplicate dependencies)
• PR line count under 800 lines (split large changes)

PR Process

Complete Flow Diagram

Development Complete → Self-Check → Push → Create PR → CLA/CI Checks → Review → Modify → Merge → Delete Branch
                                              ↓           ↓
                                            On Failure   On Rejection
                                              ↓           ↓
                                            Fix         Close/Resubmit

Step Details

1. Create a PR

Click “Compare & pull request” on the GitHub web page, or use the command line:

# Push the branch
git push origin feat/powerflow-pv-limits

# Create PR using GitHub CLI
gh pr create \
    --title "feat(powerflow): support PV bus reactive power limits" \
    --body "Closes #123" \
    --base main \
    --head feat/powerflow-pv-limits

# Or open in browser
gh pr create --web

2. Fill in PR Description

Fill in complete information according to the PR template, linking the Issue. See PR Template for details.

3. Wait for Automated Checks

After PR creation, the following bots will automatically check:

4. Wait for Review

• Maintainer will assign a Reviewer within 2 business days
• Reviewer will provide initial feedback within 5 business days
• Urgent PRs can be marked with [URGENT] in the PR description and @mention maintainers

5. Respond to Review

• Reply to or address each comment; do not ignore them
• For major changes, prefer git push --force-with-lease over merge commits to keep history linear
• PRs inactive for a long time (>30 days) will be closed and can be reopened at any time

6. Merge

After passing CI and approval from at least one Reviewer, the Maintainer will merge. After merging, the branch will be deleted by the Maintainer and the linked Issue will be automatically closed.

PR Template

PR descriptions must follow the template below (the repo has configured .github/PULL_REQUEST_TEMPLATE.md):

## Change Description

(A paragraph summarizing what this PR does and why. If linked to an Issue, briefly describe the background.)

## Linked Issue

Closes #123
Depends on: #120 (if there are dependent PRs)

## Change Type

- [ ] New feature (feat)
- [ ] Bug fix (fix)
- [ ] Refactor (refactor)
- [ ] Documentation (docs)
- [ ] Test (test)
- [ ] Performance optimization (perf)
- [ ] Build/CI (build/ci)
- [ ] Misc (chore)

## Breaking Changes

- [ ] No
- [ ] Yes (please explain migration path)

If there are breaking changes, please provide a migration guide:

\`\`\`rust
// Old API (v0.46.0)
let bus = Bus::new(1, 1.06);

// New API (v0.47.0)
let bus = Bus::builder().id(1).voltage(1.06).build();
\`\`\`

## Verification Method

(Describe how to reproduce and verify this change)

1. Start API server: `cargo run -p eneros-api`
2. Call API: `curl -X POST localhost:8080/api/powerflow/solve -d @ieee14.json`
3. Expected output: JSON response containing `converged: true`

## Checklist

- [ ] `cargo fmt --all -- --check` passes
- [ ] `cargo clippy --all-targets -- -D warnings` passes
- [ ] `cargo nextest run --workspace` passes
- [ ] New features have tests added
- [ ] Public API changes have updated `cargo doc`
- [ ] `CHANGELOG.md` updated (if behavior changes are involved)
- [ ] No sensitive files (`.env`, `*.pem`, `*.key`)
- [ ] Commit messages follow Conventional Commits

## Screenshots / Performance Data

(If UI changes or performance optimizations are involved, please attach screenshots or benchmark data)

Complete PR Description Example

## Change Description

This PR adds PV bus reactive power limit support to `eneros-powerflow`'s `NewtonRaphsonSolver`. When a PV node's reactive power output exceeds the `[q_min, q_max]` range, the solver automatically converts it to a PQ node and re-solves, complying with IEEE Std 1547-2018 requirements.

## Linked Issue

Closes #123

## Change Type

- [x] New feature (feat)
- [ ] Bug fix (fix)
- [ ] Refactor (refactor)
- [ ] Documentation (docs)
- [x] Test (test)
- [ ] Performance optimization (perf)
- [ ] Build/CI (build/ci)
- [ ] Misc (chore)

## Breaking Changes

- [x] No

The new `q_min`, `q_max` fields use `Option<f64>`, defaulting to `None` for backward compatibility.

## Verification Method

1. Run unit tests:

```bash
cargo nextest run -p eneros-powerflow pv_limits
```

2. Run IEEE 14-bus case to verify convergence:

```bash
cargo run -p enerosctl -- powerflow solve --case ieee14 --verbose
```

3. Verify reactive power violation scenario:

```bash
cargo run -p enerosctl -- powerflow solve --case ieee14_pv_limit --verbose
```

Expected: Bus 8 converts from PV to PQ after the 3rd iteration, ultimately converging.

## Checklist

- [x] `cargo fmt --all -- --check` passes
- [x] `cargo clippy --all-targets -- -D warnings` passes
- [x] `cargo nextest run --workspace` passes
- [x] New features have tests added (5 cases)
- [x] Public API changes have updated `cargo doc`
- [x] `CHANGELOG.md` updated
- [x] No sensitive files
- [x] Commit messages follow Conventional Commits

## Performance Data

IEEE 14-bus benchmark results (10 runs average):

| Scenario | Before | After | Change |
|----------|--------|-------|--------|
| No PV violation | 12.3 ms | 12.4 ms | +0.8% |
| PV violation (re-solve) | - | 18.7 ms | New |

Performance change is within acceptable range (< 10%).

Code Review Standards

Review Dimensions

Reviewers will review PRs from the following five dimensions:

Correctness Review

• Does the algorithm align with power domain knowledge?
• Are boundary conditions covered (empty set, single element, max, min)?
• Are floating-point comparisons using tolerance instead of ==?
• Is error handling complete, are errors being swallowed?

// Wrong: swallowing errors
let result = solve_powerflow(&topo).unwrap_or_default();

// Correct: propagating errors
let result = solve_powerflow(&topo).map_err(|e| {
    tracing::error!("Load flow failed: {}", e);
    e
})?;

Security Review

• Does it bypass ConstraintEngine validation?
• Does it bypass AuditChain auditing?
• Does it introduce new plaintext channels (e.g., println! outputting sensitive data)?
• Are keys, certificates, tokens exposed in logs?
• Does it use unsafe code? If so, is it adequately justified?

Performance Review

• Does the hot path introduce unnecessary memory allocations?
• Does it call blocking functions in async context?
• Does it create new connections or locks in loops?
• Is the data structure choice appropriate (Vec vs HashMap vs BTreeMap)?

// Wrong: allocating in loop
for bus in &buses {
    let result = format!("bus_{}", bus.id);  // Allocates String each time
    println!("{}", result);
}

// Correct: reuse buffer
let mut buf = String::with_capacity(32);
for bus in &buses {
    buf.clear();
    use std::fmt::Write;
    write!(&mut buf, "bus_{}", bus.id).unwrap();
    println!("{}", buf);
}

Maintainability Review

• Does naming clearly express intent?
• Do public APIs need doc comments?
• Are module boundaries reasonable?
• Is there duplicate code that can be extracted?
• Is complex logic explained with comments?

Test Sufficiency Review

• Do tests cover failure paths?
• Do they depend on the external environment (network, time, file system)?
• Are tests repeatable?
• Does test naming follow conventions?

CI Checks

Required Checks

The following CI checks must all pass before merging:

Optional Checks

CI Failure Handling

When CI fails, please follow these steps:

1. Click the failed check to view logs
2. Reproduce the failure locally:

# Reproduce CI environment
cargo +stable fmt --all -- --check
cargo +stable clippy --all-targets -- -D warnings
cargo +stable nextest run --workspace

3. Fix and push:

git add .
git commit --fixup <commit-sha>
git rebase -i --autosquash upstream/main
git push origin feat/my-feature --force-with-lease

4. CI will automatically re-run

Handling Flaky Tests

If tests fail intermittently on CI:

• Check if they depend on the external environment (network, time)
• Use --retries 3 to reproduce locally
• Explain in the PR; Maintainer will decide whether to block

Merge Strategies

Merge Method Selection

Squash Merge

Suitable for small changes. GitHub merges all commits into one, using the PR title as the commit message:

feat(powerflow): support PV bus reactive power limits (#124)

Rebase Merge

Suitable for complex changes with multiple commits, preserving the details of each commit. Requires each commit to be independently compilable and conform to Conventional Commits.

Create Merge Commit

Only used for release branches merging back to main, not used in normal operations.

Post-Merge Operations

• After merging, the branch is deleted by the Maintainer (branches in personal forks are deleted by contributors themselves)
• Linked Issues are automatically closed
• CHANGELOG.md automatically generates entries (parsed via Conventional Commits)
• CI triggers documentation site redeployment (if the eneros-web/ directory is involved)

Conflict Resolution

Preventing Conflicts

• Sync with main frequently: git fetch upstream && git rebase upstream/main
• Take small steps, shorten PR cycles
• Coordinate with other contributors to avoid modifying the same file simultaneously

Resolving Conflicts

# 1. Sync with main
git fetch upstream
git rebase upstream/main

# 2. On conflict, git will list conflicting files
# CONFLICT (content): Merge conflict in crates/eneros-powerflow/src/solver.rs

# 3. Open the conflicting file and resolve manually
# <<<<<<< HEAD
# (your changes)
# =======
# (changes on main)
# >>>>>>> upstream/main

# 4. Mark conflict as resolved
git add crates/eneros-powerflow/src/solver.rs

# 5. Continue rebase
git rebase --continue

# 6. Push (force)
git push origin feat/my-feature --force-with-lease

Conflict Resolution Principles

1. Preserve both parties’ intent: Understand the purpose of both sides’ changes, merge rather than overwrite
2. Test verification: Must rerun tests after resolving conflicts
3. Ask for collaboration: If unsure, discuss with @original author in the PR
4. Avoid auto-merge: Do not use git merge -X ours/theirs for automatic resolution

Handling Large-Scale Conflicts

If conflicts involve many files:

# Use mergetool
git mergetool

# Or use VS Code's integrated conflict resolver
# Open the conflicting file in VS Code and use the "Resolve Conflict" button

Complete PR Example

The following is a complete example of a full PR process:

1. Preparation

# Sync with main
git checkout main
git pull upstream main

# Create branch
git checkout -b feat/powerflow-pv-limits

2. Implement the Feature

Modify crates/eneros-powerflow/src/bus.rs:

use serde::{Deserialize, Serialize};

/// Bus type
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
pub enum BusType {
    /// Slack bus
    Slack,
    /// PQ bus (load)
    PQ,
    /// PV bus (generator)
    PV,
}

/// Bus
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Bus {
    pub id: u64,
    pub bus_type: BusType,
    pub voltage_magnitude: f64,
    pub voltage_angle: f64,
    pub p_generation: f64,
    pub q_generation: f64,
    pub p_load: f64,
    pub q_load: f64,
    /// PV node reactive power lower limit (None means no limit)
    pub q_min: Option<f64>,
    /// PV node reactive power upper limit (None means no limit)
    pub q_max: Option<f64>,
}

impl Bus {
    pub fn new_pv(id: u64, voltage: f64, p_gen: f64, q_min: f64, q_max: f64) -> Self {
        Self {
            id,
            bus_type: BusType::PV,
            voltage_magnitude: voltage,
            voltage_angle: 0.0,
            p_generation: p_gen,
            q_generation: 0.0,
            p_load: 0.0,
            q_load: 0.0,
            q_min: Some(q_min),
            q_max: Some(q_max),
        }
    }

    /// Check if reactive power is violated
    pub fn is_q_violated(&self) -> bool {
        if let (Some(q_min), Some(q_max)) = (self.q_min, self.q_max) {
            self.q_generation < q_min || self.q_generation > q_max
        } else {
            false
        }
    }

    /// Convert to PQ node
    pub fn convert_to_pq(&mut self) {
        self.bus_type = BusType::PQ;
        // Clamp reactive power output to boundary
        if let (Some(q_min), Some(q_max)) = (self.q_min, self.q_max) {
            self.q_generation = self.q_generation.clamp(q_min, q_max);
        }
    }
}

3. Modify the Solver

Modify crates/eneros-powerflow/src/solver.rs (key snippet):

impl NewtonRaphsonSolver {
    pub fn solve(&self, topology: &Topology) -> Result<PowerflowResult> {
        let mut buses = topology.buses.clone();
        let mut converted_buses = Vec::new();

        for iteration in 0..self.max_iterations {
            let result = self.iterate_once(&mut buses)?;
            if result.converged {
                return Ok(PowerflowResult {
                    converged: true,
                    iterations: iteration + 1,
                    buses,
                    converted_pv_buses: converted_buses,
                });
            }

            // Check PV node reactive power violation
            for bus in &mut buses {
                if bus.bus_type == BusType::PV && bus.is_q_violated() {
                    let bus_id = bus.id;
                    bus.convert_to_pq();
                    converted_buses.push(bus_id);
                    tracing::info!(
                        bus_id,
                        q = bus.q_generation,
                        "PV node reactive power violated, converting to PQ node"
                    );
                }
            }

            if !converted_buses.is_empty() {
                // Re-solve
                continue;
            }
        }

        Err(PowerflowError::NonConvergence {
            iterations: self.max_iterations,
            max_iterations: self.max_iterations,
        })
    }
}

4. Add Tests

Create crates/eneros-powerflow/tests/pv_limits_test.rs:

use eneros_powerflow::{NewtonRaphsonSolver, PowerflowResult, PowerflowError};
use eneros_topology::Topology;

#[test]
fn test_pv_bus_within_limits_no_conversion() {
    let topo = Topology::ieee_14bus_with_pv_limits(-5.0, 5.0);
    let solver = NewtonRaphsonSolver::new(50, 1e-8);
    let result: PowerflowResult = solver.solve(&topo).unwrap();

    assert!(result.converged);
    assert!(result.converted_pv_buses.is_empty(), "No violation, should not convert nodes");
}

#[test]
fn test_pv_bus_q_exceeds_max_gets_converted() {
    let topo = Topology::ieee_14bus_with_pv_limits(-5.0, 1.0);  // Very low upper limit
    let solver = NewtonRaphsonSolver::new(50, 1e-8);
    let result: PowerflowResult = solver.solve(&topo).unwrap();

    assert!(result.converged);
    assert!(!result.converted_pv_buses.is_empty(), "Should have PV nodes converted");
}

#[test]
fn test_pv_bus_q_below_min_gets_converted() {
    let topo = Topology::ieee_14bus_with_pv_limits(10.0, 20.0);  // Very high lower limit
    let solver = NewtonRaphsonSolver::new(50, 1e-8);
    let result: PowerflowResult = solver.solve(&topo).unwrap();

    assert!(result.converged);
    assert!(!result.converted_pv_buses.is_empty());
}

#[test]
fn test_pv_limits_none_no_conversion() {
    let topo = Topology::ieee_14bus();  // Default no limits
    let solver = NewtonRaphsonSolver::new(50, 1e-8);
    let result: PowerflowResult = solver.solve(&topo).unwrap();

    assert!(result.converged);
    assert!(result.converted_pv_buses.is_empty());
}

#[test]
fn test_pv_limits_non_convergence_returns_error() {
    let topo = Topology::ieee_14bus_with_pv_limits(0.0, 0.0);  // Extreme limits
    let solver = NewtonRaphsonSolver::new(1, 1e-8);  // Only 1 iteration allowed
    let result = solver.solve(&topo);

    assert!(matches!(result, Err(PowerflowError::NonConvergence { .. })));
}

5. Update CHANGELOG

## [Unreleased]

### Added
- `eneros-powerflow`: PV bus reactive power limit support, auto-converts to PQ node on violation (#124)

6. Local Verification

cargo fmt --all -- --check
cargo clippy --all-targets -- -D warnings
cargo nextest run -p eneros-powerflow
cargo nextest run --workspace
cargo doc --no-deps -p eneros-powerflow

7. Commit and Push

git add crates/eneros-powerflow/src/bus.rs \
        crates/eneros-powerflow/src/solver.rs \
        crates/eneros-powerflow/tests/pv_limits_test.rs \
        CHANGELOG.md

git commit -m "feat(powerflow): support PV bus reactive power limits

- Add q_min/q_max fields to Bus, default None for backward compatibility
- NewtonRaphsonSolver auto-converts PV node to PQ node on reactive power violation
- Add 5 test cases covering violation scenarios

Closes #123"

git push origin feat/powerflow-pv-limits

8. Create PR

Using GitHub CLI or web page:

gh pr create \
    --title "feat(powerflow): support PV bus reactive power limits" \
    --body-file .github/PULL_REQUEST_TEMPLATE.md

Fill in the template and submit.

Related Documentation

• Contribution Process Overview
• Testing Standards
• Development Environment Setup
• Documentation Contribution Guide
• Project Structure