python-package-refactoring

The full skill.

Original instructions from the publisher’s SKILL.md

# Python Package Refactoring

Systematic approach to refactoring Python package code while maintaining functionality and avoiding regressions.

Reference the sibling R package at `../agentBuilder` to ensure alignment where possible in architecture, naming, runtime patterns, and parity decisions.

## Refactoring Workflow

### 1. Identify the Problem
- Understand the repo before beginning; read through key files, exported functions, and tests.
- Duplicated code across functions
- Long complex functions (>50 lines)
- Inconsistent patterns
- Confusing function organization
- Tight coupling between components

DO NOT leave comments like "Step 1:" or "Option 1" in code. Keep comments only when they add necessary clarity.

### 2. Plan the Refactoring

Understand the mirroring pattern I was looking for between tools and patterns (especially since I want agents to be able to work as tools)
 - We have list_tools vs list_agents, load_tool vs load_agent, new_tool vs new_agent, tool config vs agent spec, tool runtime object vs agent runtime object, tool schema management vs agent schema management, tool examples/tests vs agent examples/tests, tool docs vs agent docs, etc.
 - The `build_examples.py` script creates the example tools and agents (and their YAML files), which exported Python functions utilize. Dont adjust YAML files directly.
 - Aim for consistent patterns between tools and agents where it makes sense.
 - Ensure that tool and agent use, maintenance (e.g., saving out YAML), repr/inspection methods, and integration are as consistent as possible. Ensure docs, README, and tests reflect this consistency.
 - Consider how new_tool and new_agent are similar, and how they differ. Can we make them more similar without losing clarity?
 - Consider how build_agent and tool building functions are similar/different, and whether we can standardize more.
 - Consider how tool config management and agent spec management are similar/different, and whether we can standardize more.
 - Consider how agent runtime objects and tool runtime objects are similar/different, and whether we can standardize more.
 - Consider how schema management works for agents and tools, and whether we can standardize more.
 - Consider how examples and tests for agents and tools are similar/different, and whether we can standardize more.
 - Consider how documentation for agents and tools are similar/different, and whether we can standardize grouping and cross-linking more.
 - Use `get_thread()` to validate and understand tool calls and intermediate messages to confirm understanding of agent and tool interactions (run in the Python console or in tests).


Document:
- What needs to change
- Why the change improves code
- Which files/functions affected
- Potential breaking changes
- Test coverage needed

You may make minor refactors if you notice inconsistencies while working through the main refactor plan, but avoid scope creep withough coming back to the user for approval.


### 3. Make Changes Incrementally

Targeted changes, one logical change at a time:
1. Add new function/pattern
2. Update tests
3. Migrate old code to new pattern
4. Remove old code
5. Update documentation

### 4. Validate After Each Notable Change
```bash
uv sync
pytest -k "relevant_pattern"
```

DO NOT skip tests, or use poor expecations to get tests to pass. Tests should verify intended functionality after each change. If tests fail, fix issues before proceeding.
 - Use discernment when determining whether something is a bug or a test expectation issue.
 - Ensure that all exported functions, especially example tools and agents, are covered by tests. Any agent with access to tools should have a test that uses those tools and proves they were called correctly. Use `get_thread()` in tests to validate tool calls and intermediate messages where applicable.
 - Group tests logically, and aim to reduce the number of required API calls, yet still maintain good coverage and validation of functionality. Every tool should have an isolated tool test, as well as be verified in an at least one agent test.
 - Dont run all tests after making minor changes; run relevant tests first (or reproduce the example in the Python console to debug first) to save time and API calls

## Common Refactoring Patterns

### Extract Function

Before:
```python
def process_data(data):
    # 20 lines of validation
    # ...
    # 30 lines of transformation
    # ...
    # 15 lines of formatting
    # ...
```

After:
```python
def process_data(data):
    data = _validate_data(data)
    data = _transform_data(data)
    return _format_data(data)


def _validate_data(data):
    ...


def _transform_data(data):
    ...


def _format_data(data):
    ...
```

### Extract Common Logic

Before:
```python
def build_tool_a(...):
    ...


def build_tool_b(...):
    ...
```

After:
```python
def build_tool_a(...):
    config = {...}
    _save_tool_config(config, tool_id, save_to)


def build_tool_b(...):
    config = {...}
    _save_tool_config(config, tool_id, save_to)


def _save_tool_config(config, tool_id, save_to):
    ...
```

### Consolidate Similar Functions
This shouldnt always be done, but when there are many similar functions differing only by type or minor logic, consider a single function with type param. If it reduces overall code, improves maintainability, and keeps clarity, it can be beneficial.

Before:
```python
def new_py_tool(...):
    ...


def new_rag_tool(...):
    ...


def new_api_tool(...):
    ...
```
After:
```python
def new_tool(tool_type, **kwargs):
    builders = {
        "python_function": _build_python_function_tool,
        "rag": _build_rag_tool,
        "api": _build_api_tool,
    }
    try:
        return builders[tool_type](**kwargs)
    except KeyError as exc:
        raise ValueError(f"Unknown tool type: {tool_type}") from exc
```

### Centralize Configuration Management
If you see multiple methods for doing the same thing (e.g., setting colors, managing file paths, calculating costs), consider centralizing into a config manager or utility functions. We should aim to have a single source of truth for key configrations/management, calculations, data transformations, etc.

Before: 
 - Duplicated and Scattered Logic
```python
# Repeated default values and no central control
def get_data_path():
    return os.getenv("DATA_PATH") or "./data"


def get_log_path():
    return os.getenv("LOG_PATH") or "./logs"


def get_max_rows():
    return int(os.getenv("MAX_ROWS") or 1000)
```
After:
- Centralized Configuration
```python
# Single source of truth
CONFIG_DEFAULTS = {
    "data_path": "./data",
    "log_path": "./logs",
    "max_rows": 1000,
}


def get_config(key):
    return CONFIG_DEFAULTS[key]


def get_data_path():
    return get_config("data_path")


def get_log_path():
    return get_config("log_path")


def get_max_rows():
    return get_config("max_rows")
```


### Avoid uneccessary conditional logic
Aim to keep code straightforward, but also minimal in number of lines of code. Consider when helper functions, protocols/ABCs, or polymorphism can reduce conditional complexity and overall code we need to maintain.


Before:
```python
def build_agent(spec, save_to=None, overwrite=False):
    if save_to is None:
        save_to = Path(tempfile.mkstemp()[1])
    if save_to.exists() and not overwrite:
        raise FileExistsError("File exists")
    ...
```
After:
```python
def build_agent(spec, save_to, overwrite=False):
    if save_to.exists() and not overwrite:
        raise FileExistsError("File exists")
    ...
```

### Standardize Parameter Order

Choose consistent order across package:
- Required params first
- Optional params after
- Common params (like `save_to`, `overwrite`) last

```python
# Consistent
def new_thing_a(required1, required2, optional=None, save_to=None):
    ...


def new_thing_b(required1, optional=None, save_to=None):
    ...

# Inconsistent (bad)
def new_thing_a(save_to=None, required1=None, optional=None):
    ...


def new_thing_b(required1, optional=None, save_to=None):
    ...
```

### Use Common Protocols and Dispatch

```python
# Instead of scattered ad-hoc helpers, use shared contracts.
# Use Protocol / ABC / singledispatch where appropriate.

from abc import ABC, abstractmethod
from functools import singledispatch
from typing import Protocol


class SupportsSummary(Protocol):
    def summary(self) -> str:
        ...


class RuntimeObject(ABC):
    @abstractmethod
    def summary(self) -> str:
        ...


@singledispatch
def summarize(obj):
    raise TypeError(f"Unsupported type: {type(obj)!r}")


@summarize.register
def _(obj: AgentSpec):
    return obj.summary()


@summarize.register
def _(obj: ToolSpec):
    return obj.summary()
```


## File Organization

### Group Related Functions

```python
# config.py - Configuration management
get_config_dir()
set_config_dir()
list_agents()
list_tools()

# spec.py - Specification creation
new_agent()
new_py_tool()
new_rag_tool()
new_agent_tool()

# runtime.py - Runtime objects
build_agent()
use_agent()
reset_agent()
```

### Keep Files Focused
- One file per major feature
- < 500 lines per file (guideline)
- Related functions together
- Clear file naming


### Standardize Examples

```python
# Use realistic examples across docs
"""
Examples
--------
Create tool:
  new_tool(id="my_tool", ...)

Load and use:
    tool = load_tool("my_tool")
"""
```

## Breaking Changes

### When Breaking Changes Needed

1. **Add deprecation warning first**
```python
import warnings


def old_function(*args, **kwargs):
    warnings.warn(
        "old_function() is deprecated; use new_function()",
        category=DeprecationWarning,
        stacklevel=2,
    )
    return new_function(*args, **kwargs)
```

2. **Update in next major version**
- Remove old function
- Update CHANGELOG
- Update docs examples

### Avoiding Breaking Changes

 Ensuring documentation and test expecations is key above all else. We mainly want to avoid introducing new patterns that are not consistent with previous patterns unless there is a strong reason to do so (like expanding functionality to a new tool category type that wasnt previously supported).
 - An example of where we wouldnt want to drift is making a new Python function tool using an approach that isnt consistent with `build_examples.py`, `example_tools.py`, and tests.
 - We also want to avoid changing test expectations just to get a tool or agent to "pass" if the underlying logic is changing in a way that isnt consistent with previous patterns or expected behavior.

## Pre-Refactor Checklist

- [ ] Current tests all pass
- [ ] Test coverage checked for affected code
- [ ] Git working directory clean
- [ ] Created feature branch for refactoring
- [ ] Documented plan in issue/comment

## Post-Refactor Checklist

- [ ] All tests pass
- [ ] No new warnings in check()
- [ ] Documentation updated
- [ ] Examples updated and run
- [ ] Coverage same or better
- [ ] Git diff reviewed
- [ ] CHANGELOG updated (if applicable)

## Git Integration

### Make Small, Focused Commits

```bash
# Good commit history
git commit -m "Extract validation logic to .validate_config"
git commit -m "Update tests for new validation function"
git commit -m "Migrate build_agent to use .validate_config"
git commit -m "Remove old validation code"

# Bad commit history  
git commit -m "Refactor everything"
```


## Tools for Refactoring

```bash
# Lint and style
ruff check .
ruff format .

# Optional type checks
mypy src/
```