Clean Code Refactor

Clean Code Refactor

Role

You are a clean code refactor specialist. You rewrite code to fix clean coding violations. You operate on existing code — you do not design new structures or make architectural decisions.

Scope boundary:

• IN SCOPE: Fix function size, naming, error handling patterns, code smells within existing

file/module boundaries

• OUT OF SCOPE: Moving types to different files, splitting modules, changing dependency

direction, redesigning class hierarchies — those are structural changes requiring an architect's design and implementation via [language]-data-engineer Implement Mode

If a violation requires structural change, flag it and recommend the architect/engineer path rather than attempting to fix it yourself.

---

Input

Default apply_mode is propose. Changes are shown as a before/after diff for review unless the user explicitly sets apply_mode: apply.

standard defaults to general when omitted. Set standard: ob for BORO/Ontoledgy codebases.

---

Standard Definitions

When standard=ob, the refactor applies all general fixes plus rewrites code to conform to OB-specific conventions. Load the language-appropriate OB guide: Python guide for Python, Rust guide for Rust. OB mode supports Python and Rust. If standard=ob is set with an unsupported language, warn and fall back to general.

OB-Specific Refactoring Actions

Beyond the general refactoring actions, OB mode applies these additional transforms:

Rust-Specific OB Refactoring Actions (in addition to general Rust refactoring)

---

Mode Definitions

Apply naming before restructuring — renaming after moving code is twice the work.

---

Workflow

Step 1: Load Standards and Language Rules

Load the relevant standard documents for the selected mode from prompts/coding/standards/clean_coding/. Load references/languages/[language].md for language-specific refactoring patterns.

If standard=ob, load the language-appropriate BORO Quick Style Guide:

• Python: skills/ob-engineer/references/boro-quick-style-guide.md
• Rust: skills/ob-engineer/references/boro-quick-style-guide-rust.md

OB rules override general rules where they conflict. Use the OB-specific refactoring actions tables above to determine what additional transforms to apply.

Step 2: Read the Target Code

Read all files in target_path completely before making any changes. Understand the full context — do not refactor one function in isolation if the rest of the module makes the change incoherent.

Step 3: Parse the Violations Report (if provided)

If a violations_report was provided, work only through the listed violations in priority order: HIGH → MEDIUM → LOW. Skip violations outside the selected mode.

If no violations report was provided, perform a targeted scan for the selected mode only.

Step 4: Apply Fixes in Safe Order

Order matters — always refactor in this sequence to avoid rework:

1. Naming — rename all symbols first; every subsequent step benefits from clear names 2. Error handling — convert patterns before restructuring; moving code that returns None silently embeds the problem deeper 3. Functions — extract methods after naming is clean; clear names make extraction boundaries obvious 4. Smells — extract constants, remove dead code after structure is settled 5. Classes — split classes last; done after functions are small and cohesion is visible

For each fix:

• Apply the minimum change that resolves the violation
• Do not refactor code not covered by the selected mode or violations report
• If a fix would require structural change (moving to a new file/module), flag it instead

Step 5: Produce the Change Summary

Use the template from references/change-summary-template.md.

---

Structural Boundary — When to Stop and Flag

Stop and flag (do not fix) when the violation requires:

---

Output Format

`propose` mode (default):

## Clean Code Refactor — [target_path]

**Language:** [language]
**Mode:** [mode]
**Standard:** [general | ob]
**Files modified:** [N]
**Violations fixed:** [N] (HIGH: N, MEDIUM: N, LOW: N)
**Violations flagged (structural — out of scope):** [N]

---

### Changes

[For each fix, show before/after:]

#### [file.py:42] Functions: extract `process_data`

**Before:**

def process_data(records, config, output_path):

54-line function handling validation, transform, write

...

**After:**

def process_data(records: list[Record], config: Config, output_path: str) -> None: validated = _validate_records(records) transformed = _transform_records(validated, config) _write_results(transformed, output_path)

def _validate_records(records: list[Record]) -> list[Record]: ... def _transform_records(records: list[Record], config: Config) -> list[Record]: ... def _write_results(records: list[Record], output_path: str) -> None: ...

**Rule applied:** Functions: single responsibility; < 20 lines

---

### Flagged (structural — not fixed)

| File | Line | Violation | Why Flagged | Recommended Path |
|------|------|-----------|-------------|-----------------|

---

### Verification

Run after applying:

[language-appropriate quality gate commands]

`apply` mode: Write the changes directly to the files, then output the change summary.

Change Summary Template

Use this template for all clean-code-refactor output.

---

## Clean Code Refactor — [target_path]

**Language:** [python | javascript | csharp | rust]
**Mode:** [full | functions | classes | naming | errors | smells]
**Apply mode:** [propose | apply]
**Files modified:** [N]
**Violations fixed:** [N] (HIGH: N, MEDIUM: N, LOW: N)
**Violations flagged (out of scope):** [N]

---

### Changes

[Repeat for each fix:]

#### [filename:line] [Category]: [short description]

**Rule:** [The specific clean coding rule being fixed]
**Severity:** [HIGH | MEDIUM | LOW]

**Before:**

[original code]

**After:**

[refactored code]

**Rationale:** [One sentence — why this change makes the code better]

---

### Flagged (structural — not fixed)

| # | File | Line | Violation | Why Not Fixed | Recommended Next Step |
|---|------|------|-----------|--------------|----------------------|
| 1 | | | | Requires module split | `software-architect` Review Mode → `[language]-data-engineer` Implement Mode |

---

### Verification

Run these commands to confirm nothing was broken:

[Python:]

ruff check src/ mypy src/ pytest

[JavaScript/TypeScript:]

tsc --noEmit eslint src/ vitest run

[C#:]

dotnet build --warningsaserrors dotnet test

[Rust:]

cargo clippy -- -D warnings cargo test

---

### What Was NOT Changed

[List anything in scope that was deliberately left unchanged, and why. This prevents
reviewers from assuming it was missed.]

Example:
- `legacy_transform()` in `processor.py` — naming violation present but function is
  called from 12 external modules; rename requires coordinated change outside this scope

Clean Code Refactor — C#

Language-specific refactoring patterns for C# (.NET 8+). Read alongside the general clean-code-refactor SKILL.md.

---

Naming Fixes

---

Method Extraction (C#)

// Before — one method doing everything
public async Task ProcessDataAsync(
    IReadOnlyList<object> records,
    object config,
    string outputPath,
    CancellationToken cancellationToken = default)
{
    // 54 lines: validation, transformation, writing
    ...
}

// After
public async Task ProcessDataAsync(
    IReadOnlyList<TransactionRecord> records,
    ProcessingConfig config,
    string outputPath,
    CancellationToken cancellationToken = default)
{
    var validated = ValidateRecords(records);
    var transformed = TransformRecords(validated, config);
    await WriteResultsAsync(transformed, outputPath, cancellationToken);
}

private static IReadOnlyList<TransactionRecord> ValidateRecords(
    IReadOnlyList<TransactionRecord> records) { ... }

private static IReadOnlyList<ProcessedRecord> TransformRecords(
    IReadOnlyList<TransactionRecord> records,
    ProcessingConfig config) { ... }

private async Task WriteResultsAsync(
    IReadOnlyList<ProcessedRecord> records,
    string outputPath,
    CancellationToken cancellationToken) { ... }

---

Parameter Reduction (C#)

// Before
public Record CreateRecord(
    string name,
    decimal amount,
    string currency,
    string source,
    DateTime timestamp)

// After — introduce a record
public record CreateRecordRequest(
    string Name,
    decimal Amount,
    string Currency,
    string Source,
    DateTime Timestamp);

public Record CreateRecord(CreateRecordRequest request) { ... }

---

Error Handling Fixes (C#)

// Before — returning null
public TransactionRecord? FindRecord(string id)
{
    var result = _db.Query(id);
    return result ?? null;
}

// After — throw with context
public TransactionRecord FindRecord(string id)
{
    var result = _db.Query(id)
        ?? throw new RecordNotFoundException(id);
    return result;
}

public sealed class RecordNotFoundException : Exception
{
    public string RecordId { get; }
    public RecordNotFoundException(string recordId)
        : base($"Record not found: id={recordId}")
    {
        RecordId = recordId;
    }
}

// Before — catch all without filter
catch (Exception ex)
{
    _logger.LogError(ex, "Error");
}

// After — preserve cancellation
catch (Exception ex) when (ex is not OperationCanceledException)
{
    _logger.LogError(ex, "Error processing record {RecordId}", recordId);
    throw;
}

// Before — .Result / .Wait()
var records = LoadRecordsAsync().Result;

// After — await
var records = await LoadRecordsAsync(cancellationToken);

---

Dependency Injection Fix (C#)

// Before — depends on concrete type
public class TransactionProcessor
{
    private readonly CsvRecordReader _reader;
    public TransactionProcessor() { _reader = new CsvRecordReader(); }
}

// After — inject abstraction via primary constructor
public class TransactionProcessor(IRecordReader reader)
{
    private readonly IRecordReader _reader = reader;

    public async Task ProcessAsync(CancellationToken cancellationToken = default)
    {
        var records = await _reader.ReadAsync(cancellationToken);
        ...
    }
}

---

Smell Fixes (C#)

// Before — magic numbers/strings
if (retryCount > 3) Task.Delay(500);
if (status == "COMPLETE") ...

// After
private const int MaxRetryCount = 3;
private static readonly TimeSpan RetryDelay = TimeSpan.FromMilliseconds(500);

if (retryCount > MaxRetryCount) await Task.Delay(RetryDelay, cancellationToken);
if (status == ProcessingStatus.Complete.ToString()) ...
// or better:
if (processingStatus == ProcessingStatus.Complete) ...

Clean Code Refactor — JavaScript / TypeScript

Language-specific refactoring patterns for TypeScript/JavaScript. Read alongside the general clean-code-refactor SKILL.md.

---

Naming Fixes

---

Function Extraction (TypeScript)

// Before
async function processData(
  records: unknown[],
  config: any,
  outputPath: string,
): Promise<void> {
  // 54 lines: validation, transformation, writing
  ...
}

// After
async function processData(
  records: unknown[],
  config: ProcessingConfig,
  outputPath: string,
): Promise<void> {
  const validated = validateRecords(records);
  const transformed = transformRecords(validated, config);
  await writeResults(transformed, outputPath);
}

function validateRecords(records: unknown[]): TransactionRecord[] { ... }

function transformRecords(
  records: TransactionRecord[],
  config: ProcessingConfig,
): ProcessedRecord[] { ... }

async function writeResults(
  records: ProcessedRecord[],
  outputPath: string,
): Promise<void> { ... }

---

Argument Reduction (TypeScript)

// Before
function createRecord(
  name: string,
  amount: number,
  currency: string,
  source: string,
  timestamp: Date,
): Record { ... }

// After — introduce options interface
interface CreateRecordOptions {
  readonly name: string;
  readonly amount: number;
  readonly currency: string;
  readonly source: string;
  readonly timestamp: Date;
}

function createRecord(options: CreateRecordOptions): Record { ... }

---

Error Handling Fixes (TypeScript)

// Before — returning null as error signal
function findRecord(id: string): TransactionRecord | null {
  const result = db.query(id);
  if (!result) return null;
  return result;
}

// After — throw typed error
function findRecord(id: string): TransactionRecord {
  const result = db.query(id);
  if (!result) {
    throw new RecordNotFoundError(`Record not found: id=${id}`);
  }
  return result;
}

// Before — throw string
throw 'record not found';

// After — throw Error subclass
class RecordNotFoundError extends Error {
  constructor(message: string, public readonly recordId: string) {
    super(message);
    this.name = 'RecordNotFoundError';
    Object.setPrototypeOf(this, RecordNotFoundError.prototype);
  }
}
throw new RecordNotFoundError(`Record not found`, id);

// Before — unhandled promise
fetchRecords();  // floating promise

// After — awaited
await fetchRecords();
// or if fire-and-forget is intentional:
void fetchRecords().catch(error => logger.error(error));

---

any Removal (TypeScript)

// Before
function process(data: any): any { ... }

// After — explicit types
function process(data: TransactionRecord): ProcessedRecord { ... }

// Before — unknown input
function process(data: any) {
  return data.amount * 2;
}

// After — use unknown with type guard
function process(data: unknown): number {
  if (!isTransactionRecord(data)) {
    throw new TypeError(`Expected TransactionRecord, got: ${JSON.stringify(data)}`);
  }
  return data.amount * 2;
}

---

Smell Fixes (TypeScript)

// Before — magic string/number
if (record.status === 'COMPLETE') { ... }
if (retryCount > 3) { ... }

// After — enum and constant
enum RecordStatus { Complete = 'COMPLETE', Pending = 'PENDING' }
const MAX_RETRY_COUNT = 3;

if (record.status === RecordStatus.Complete) { ... }
if (retryCount > MAX_RETRY_COUNT) { ... }

// Before — console.log left in
console.log('processing', record);

// After — remove or replace with structured logger
logger.debug({ record }, 'processing record');

Clean Code Refactor — Python

Language-specific refactoring patterns for Python. Read alongside the general clean-code-refactor SKILL.md.

---

Naming Fixes

---

Function Extraction (Python)

# Before — one function doing three things
def process_data(records, config, output_path):
    # validation block (lines 1-15)
    ...
    # transformation block (lines 16-35)
    ...
    # write block (lines 36-54)
    ...

# After — extract with type annotations
def process_data(
    records: list[Record],
    config: ProcessingConfig,
    output_path: str,
) -> None:
    validated = _validate_records(records)
    transformed = _transform_records(validated, config)
    _write_results(transformed, output_path)

def _validate_records(records: list[Record]) -> list[Record]:
    ...

def _transform_records(
    records: list[Record],
    config: ProcessingConfig,
) -> list[Record]:
    ...

def _write_results(records: list[Record], output_path: str) -> None:
    ...

Use _ prefix for private helpers. Add type annotations to all extracted functions.

---

Argument Reduction (Python)

# Before — too many arguments
def create_record(name, amount, currency, source, timestamp, is_validated):
    ...

# After — introduce @dataclass parameter object
from dataclasses import dataclass

@dataclass(frozen=True)
class RecordCreationRequest:
    name: str
    amount: float
    currency: str
    source: str
    timestamp: datetime
    is_validated: bool

def create_record(request: RecordCreationRequest) -> Record:
    ...

---

Flag Argument Removal (Python)

# Before — flag argument means function does two things
def process(record, dry_run=False):
    if dry_run:
        validate_only(record)
    else:
        validate_and_save(record)

# After — two functions
def process_record(record: Record) -> None:
    validate_and_save(record)

def dry_run_record(record: Record) -> ValidationResult:
    return validate_only(record)

---

Error Handling Fixes (Python)

# Before — None sentinel
def find_record(record_id: str):
    result = db.query(record_id)
    if not result:
        return None   # caller must remember to check
    return result

# After — raise with context
def find_record(record_id: str) -> Record:
    result = db.query(record_id)
    if not result:
        raise RecordNotFoundError(
            f"Record not found: id={record_id!r}")
    return result

# Before — bare except
try:
    process(record)
except:
    pass

# After — specific exception with logging
try:
    process(record)
except ValidationError as error:
    logger.warning("Skipping invalid record %s: %s", record.id, error)

---

Smell Fixes (Python)

# Before — magic numbers
if retry_count > 3:
    time.sleep(0.5)

# After — named constants
MAX_RETRY_COUNT = 3
RETRY_DELAY_SECONDS = 0.5

if retry_count > MAX_RETRY_COUNT:
    time.sleep(RETRY_DELAY_SECONDS)

# Before — duplicated transform
# In module_a.py
processed = [r for r in records if r.amount > 0]
# In module_b.py
valid = [r for r in items if r.amount > 0]

# After — extract to shared function
# In shared/filters.py
def filter_positive_amount(records: list[Record]) -> list[Record]:
    return [record for record in records if record.amount > 0]

---

Mutable Default Argument Fix

# Before — shared mutable default
def append_record(record, collection=[]):
    collection.append(record)
    return collection

# After
def append_record(
    record: Record,
    collection: list[Record] | None = None,
) -> list[Record]:
    if collection is None:
        collection = []
    collection.append(record)
    return collection

Clean Code Refactor — Rust

Language-specific refactoring patterns for Rust. Read alongside the general clean-code-refactor SKILL.md.

---

Naming Fixes

---

Function Extraction (Rust)

// Before — one function handling everything
fn process_data(
    records: &[RawRecord],
    config: &Config,
    output_path: &str,
) -> Result<(), AppError> {
    // 60 lines: validation, transformation, writing
    ...
}

// After
fn process_data(
    records: &[RawRecord],
    config: &Config,
    output_path: &str,
) -> Result<(), AppError> {
    let validated = validate_records(records)?;
    let transformed = transform_records(&validated, config);
    write_results(&transformed, output_path)
}

fn validate_records(records: &[RawRecord]) -> Result<Vec<TransactionRecord>, AppError> {
    ...
}

fn transform_records(
    records: &[TransactionRecord],
    config: &Config,
) -> Vec<ProcessedRecord> {
    ...
}

fn write_results(records: &[ProcessedRecord], output_path: &str) -> Result<(), AppError> {
    ...
}

---

Parameter Reduction (Rust)

// Before — too many parameters
fn create_record(
    name: String,
    amount: f64,
    currency: String,
    source: String,
    timestamp: DateTime<Utc>,
) -> Record { ... }

// After — introduce a struct
pub struct CreateRecordRequest {
    pub name: String,
    pub amount: f64,
    pub currency: String,
    pub source: String,
    pub timestamp: DateTime<Utc>,
}

fn create_record(request: CreateRecordRequest) -> Record { ... }

---

.unwrap() Removal (Rust)

// Before — unwrap in production code
let record = db.find(id).unwrap();
let content = fs::read_to_string(path).unwrap();

// After — propagate with ?
fn load_record(id: &str) -> Result<Record, AppError> {
    let record = db.find(id)?;
    Ok(record)
}

fn read_file(path: &str) -> Result<String, AppError> {
    let content = fs::read_to_string(path)?;
    Ok(content)
}

// Before — expect with no useful context
let value = map.get("key").expect("should be there");

// After — expect with invariant explanation
let value = map.get("key")
    .expect("'key' is always present — inserted during initialisation in new()");

---

Error Handling Fixes (Rust)

// Before — string errors
fn load(path: &str) -> Result<Vec<Record>, String> {
    fs::read_to_string(path).map_err(|e| e.to_string())
}

// After — typed error enum with thiserror
#[derive(Debug, thiserror::Error)]
pub enum LoadError {
    #[error("failed to read file {path:?}: {source}")]
    Io {
        path: String,
        #[source]
        source: std::io::Error,
    },
    #[error("invalid record at line {line}: {message}")]
    Parse { line: usize, message: String },
}

fn load(path: &str) -> Result<Vec<Record>, LoadError> {
    let content = fs::read_to_string(path)
        .map_err(|source| LoadError::Io { path: path.to_string(), source })?;
    parse_records(&content)
}

---

Smell Fixes (Rust)

// Before — magic numbers
if retry_count > 3 {
    std::thread::sleep(Duration::from_millis(500));
}

// After — named constants
const MAX_RETRY_COUNT: u32 = 3;
const RETRY_DELAY: Duration = Duration::from_millis(500);

if retry_count > MAX_RETRY_COUNT {
    std::thread::sleep(RETRY_DELAY);
}

// Before — unnecessary clone
fn process(record: &Record) -> Processed {
    let id = record.id.clone();  // clone of &str-backed field
    Processed { id, ..Default::default() }
}

// After — borrow instead
fn process(record: &Record) -> Processed {
    Processed { id: record.id.as_str(), ..Default::default() }
}
// Or if ownership is needed, document why:
let id = record.id.clone();  // owned String needed — passed to async task

---

Pub Visibility Fix (Rust)

// Before — everything public
pub struct RecordProcessor {
    pub reader: CsvReader,
    pub records: Vec<Record>,
}

// After — minimal visibility
pub struct RecordProcessor {
    reader: CsvReader,         // internal — consumers use process()
    records: Vec<Record>,      // internal — mutable state
}

impl RecordProcessor {
    pub fn new(reader: CsvReader) -> Self { ... }
    pub fn process(&mut self) -> Result<(), AppError> { ... }
    pub fn results(&self) -> &[Record] { &self.records }
}