reusable-parts
Review code for functions that fuse logic instead of composing independent parts. Test whether convenience functions are thin orchestration (good) or god functions with trapped logic (bad).
SKILL.mdclaude-skills/reusable-parts/SKILL.md
GitHubReusable Parts
Principle
The UNIX philosophy says: make simple functions that do one thing, then compose them into complex behavior. Convenience functions that orchestrate simple parts are fine — they're just shell scripts piping tools together. God functions that fuse logic are not — they trap decisions inside and make the parts unusable independently.
The test: Can you use the parts without the whole? If yes, the convenience function is just orchestration. If no, it's a god function.
How to apply
Step 1: Identify the target code. Use the code the user points you to, or the recently changed code from git diff.
Step 2: Parse arguments. The user may pass optional arguments:
agents=N— number of reviewer agents to spawn (default: 3)model=sonnet|opus|haiku— Claude model for the agents (default: inherit from parent)
For example: /reusable-parts agents=5 model=haiku spawns 5 agents using Haiku.
Step 3: Spawn N independent reviewer agents in parallel using the Agent tool (subagent_type: general-purpose, model: as specified). Give each agent:
- The full source of the code to review (paste it directly into the prompt, do not ask the agent to read files)
- The five tests below
- Instructions to return a structured report: one section per function reviewed, verdict (PASS / FAIL / WARN), and specific line citations for any violations
The three agents should work independently — do not share intermediate results between them before they report back.
Step 4: Aggregate. Once all N agents have returned, synthesize their findings:
- Violations all/most agree on → confirmed, high confidence
- Violations a majority flags → likely, note the dissent
- Violations only one flags → possible, lower confidence, include but mark as minority view
- Functions all pass → clean, note briefly
Present the aggregated report to the user, grouped by function. For each confirmed or likely violation, cite the specific lines and explain why they fail the test. Then propose a refactor: extract the trapped logic into independent primitives and reduce the orchestrator to a thin calling sequence.
The five tests (give these verbatim to each agent)
For each function that calls multiple other functions or contains multiple logical steps, evaluate:
Is it just a calling sequence? The function should read like a recipe: call A, pass result to B, pass result to C. The orchestrator makes no decisions of its own beyond sequencing.
Does logic live in the leaves? Branching, retries, configuration interpretation, error recovery — these belong in the primitive functions, not in the orchestrator. If the orchestrator contains
if/elifchains,forloops that build data structures, ortry/exceptwith recovery logic, the logic has leaked upward.Can the parts be used independently? The ultimate proof. Can you call each primitive in a different context — a dry run, a single-shot redo, a test — without the orchestrator? If a primitive depends on state set up by the orchestrator, it's coupled.
Do steps communicate through shared mutable state? If step B's behavior depends on side effects from step A via a shared
statedict or mutated object, the steps are fused even if they look like separate functions.Count the inputs. Draw a circle around a block of inline code and count the arrows coming in from the surrounding scope. If the block only touches two or three variables out of the ten available in the function, it's telling you it doesn't belong to this scope. It's a self-contained computation sitting inside a larger function. Those few arrows are a function signature waiting to happen. This is especially common with inline math — a matrix inversion and decomposition that only needs two matrices, sitting inside a function that also has a robot handle, a camera, a config, and a list of images. The math doesn't know about any of that. It shouldn't have to.
Case studies
BAD: Logic trapped inside the orchestrator
def collect_dataset(robot, cameras, config, dataset):
# generates positions inline — can't reuse this logic
positions = []
for i in range(config.num_samples):
pos = [random.uniform(*config.bounds[axis]) for axis in 'xyz']
if config.avoid_collisions:
while robot.would_collide(pos):
pos = [random.uniform(*config.bounds[axis]) for axis in 'xyz']
positions.append(pos)
for pos in positions:
# movement logic with retries baked in
for attempt in range(3):
try:
robot.move(pos, velocity=0.3 if attempt == 0 else 0.1)
break
except MotionError:
if attempt == 2:
continue
# capture with inline calibration decisions
images = []
for cam in cameras:
if cam.type == 'zed':
img = cam.grab(resolution='2k', depth=True)
elif cam.type == 'realsense':
img = cam.grab(resolution='1080p', depth=True, laser_power=150)
else:
img = cam.grab()
images.append(img)
dataset.append({'pos': pos, 'images': images})
Why this fails:
- Position generation is inline. The collision-aware sampling logic is trapped inside the loop. You can't reuse it to visualize planned positions, test a different sampling strategy, or generate positions without a robot present.
- Retry logic is baked into the orchestrator. The velocity-backoff retry strategy is a movement policy decision, but it lives in the dataset collection function. You can't reuse this retry logic for a different task, and you can't move the robot without retries if you wanted to.
- Camera dispatch is inline. The
if cam.type == 'zed'branching means adding a new camera type requires editing the dataset collection function. Capture configuration is a camera concern, not a dataset concern. - Nothing is independently callable. You can't generate positions without collecting a dataset. You can't capture images without the full loop. Every piece of logic is fused into one monolith.
BAD: Steps coupled through shared mutable state
def collect_dataset(robot, cameras, config):
state = {'retries': 0, 'skipped': [], 'dataset': [], 'last_pos': None}
for i in range(config.num_samples):
pos = generate_next_pos(state, config) # reads state['last_pos']
state['last_pos'] = pos
success = try_move(robot, pos, state) # mutates state['retries']
if not success:
state['skipped'].append(i) # steps communicate through state
continue
images = capture(cameras, state) # behavior depends on state['retries']
state['dataset'].append({'pos': pos, 'images': images})
return state
Why this fails:
- The functions look independent but aren't. They're separated into
generate_next_pos,try_move, andcapture, which gives the illusion of modularity. But they all read and write a sharedstatedict, so they're coupled through a side channel. - Behavior depends on hidden context.
capture(cameras, state)changes behavior based onstate['retries']— a value set bytry_move. This means you can't callcaptureindependently without first setting up the state thattry_movewould have produced. generate_next_posdepends on prior iteration. It readsstate['last_pos'], so you can't generate all positions up front or test the generation in isolation.- The state dict is a god object in disguise. Instead of fusing logic into one function, the logic is fused through shared mutable state. The coupling is just harder to see.
GOOD: Thin orchestration over independent parts
# Simple primitives — each is self-contained
def generate_positions(bounds, n, collision_check=None) -> list[Pose]: ...
def move_robot(robot, pose) -> bool: ...
def capture_images(cameras) -> list[Image]: ...
def store_sample(dataset, images, pose): ...
# Convenience function — just a calling sequence
def collect_dataset(robot, cameras, config, dataset):
for pose in generate_positions(config.bounds, config.num_samples):
if move_robot(robot, pose):
images = capture_images(cameras)
store_sample(dataset, images, pose)
Why this succeeds:
- The orchestrator contains no logic of its own. It's a four-line loop that calls primitives in sequence. No branching, no retries, no configuration interpretation. It reads like a recipe.
- Each primitive is self-contained.
generate_positionstakes bounds and a count — it doesn't need a robot or a dataset.capture_imagestakes cameras — it doesn't need to know what position the robot is at. No function depends on side effects from another. - The retry/collision/calibration logic lives in the leaves.
move_robothandles its own retry policy internally.generate_positionshandles collision avoidance via the optionalcollision_checkcallback. Each camera knows how to configure itself insidecapture_images. The orchestrator doesn't need to know about any of this.
GOOD: Proof — same parts, different compositions
# Visualize planned positions without moving the robot
def dry_run(config):
poses = generate_positions(config.bounds, config.num_samples)
plot_poses(poses)
# Re-take images at a single known position
def recapture_at(robot, cameras, dataset, pose_index):
pose = dataset[pose_index]['pose']
move_robot(robot, pose)
images = capture_images(cameras)
store_sample(dataset, images, pose)
Why this succeeds:
- This composition was never planned by the original author.
dry_runandrecapture_atuse the same primitives in arrangements thecollect_datasetfunction never anticipated. This is only possible because the parts are truly independent. - Each new composition is also a thin calling sequence. No new logic is introduced. The functions are just wired together differently.
- This is the UNIX payoff. Independent tools composed via simple scripts. The value isn't in any one function — it's in the fact that you can rearrange them freely.
Real case study: split a god-export at its seam, share the pose decomposition
A dry-run debug export started as one function that baked debug cameras + grasp-frame axes
into a live USD stage and wrote a .usdz + .npz:
# BAD — decoration fused with persistence; you can't get the decorated scene without writing files
def export_debug_bundle(runtime, scene, grasp_points, world_poses, render_dir):
# ... bake cameras + axis gizmos into the stage ...
usdz = export_subtree_usdz(stage, "/World", render_dir / "debug", "scene")
np.savez(render_dir / "debug" / "dryrun.npz", world_poses=world_poses, ...)
return usdz
Apply test 3 (can the parts be used independently?): no — to obtain a decorated scene you are forced to write a USDZ and an npz to a specific directory. The natural seam is the moment the last gizmo is added: before it is decorate the scene, after it is persist it.
# GOOD — mechanism returns the decorated scene; a thin policy persists it
def decorate_debug_scene(scene, grasp_points, world_poses) -> dict: ... # no I/O
def export_debug_bundle(info, render_dir): ... # export + savez
# orchestrator is a calling sequence:
export_debug_bundle(decorate_debug_scene(scene, grasp_points, world_poses), render_dir)
Now decorate_debug_scene is reusable in arrangements the export author never planned —
render the scene in-process, open it in a viewer, or bake more onto it before exporting —
exactly the "same parts, different compositions" payoff above.
The same review also caught a hidden version of the problem via test 5 (count the inputs).
move_prims was the only place an SE3 became a prim pose; its decomposition
(p[:3,3], R.from_matrix(p[:3,:3]).as_euler('xyz')) only touched the pose matrix out of all
the Replicator state in scope — a self-contained computation wearing the function as a costume.
The dry run needed that identical math but without a Replicator graph. Extracting it paid off
immediately:
def se3_to_pos_euler(pose): # the one decomposition, reused by both callers
return (pose[:3, 3].tolist(),
R.from_matrix(pose[:3, :3]).as_euler('xyz', degrees=True).tolist())
def set_prim_pose(prim_path, pose): # static applier, prim-path addressed (dry run)
t, e = se3_to_pos_euler(pose)
set_transform(stage.GetPrimAtPath(prim_path), translation=t, rotation=e)
# move_prims (live capture) now consumes the same primitive instead of a private copy:
positions, rotations = zip(*(se3_to_pos_euler(p) for p in poses))
rep.modify.pose(position=rep.distribution.sequence(list(positions)),
rotation=rep.distribution.sequence(list(rotations)))
One extracted primitive, two compositions (a batched Replicator graph and a static per-prim setter) — and because both reuse the same decomposition, the dry-run cameras can't drift from where the real capture would place them.