cleanup

Cleanup

This skill covers code quality patterns and common pitfalls when writing, changing, refactoring or reviewing HoloViz code. When you're writing, apply the sections below as you go and run the scan in step 2 of the Review before you finish. Keeping slop out matters more with agents in the loop: every line costs tokens each time an agent reads it, and the agent treats what it reads as the standard and copies it into the next change.

Contents

• Review
• Reuse and Duplication
• Errors and Guards
• Code Style
• Comments

Review

Review the change against the whole repo, not just the diff. The diff can't show a helper that already exists in another module, or a regex that's already parsed somewhere else, and those are the copies an agent adds most often. Writing something new is cheaper for it than finding what exists.

1. Run git diff main...HEAD to see the change, then check it against the rest of the repo:
   • For each new function, search the repo for one that already does the same job. The existing helper may have a name you wouldn't guess, so search for what it does as well as for likely names, using a semantic codebase index if one is available.
   • For each comment, check that what it claims is true.
   • For each try/except, say how the code inside could fail and what the caller sees when it does. If it can't fail, remove the except.
   • List the constants and helpers that are used only once, and for each, whether its name says something its value and call site don't.
   • For each fix, say whether it's where the problem starts or where it showed up. When a traceback points at a caller, an agent tends to patch the caller, like adding .strip() wherever a helper's output is used, instead of fixing the helper once. A PR that touches five files to work around a problem may have a two-line fix elsewhere.
2. Run python3 scripts/cleanup_scan.py <package dir> over the whole package, since it counts a constant's reads across every file it's given. It flags the findings a parser can see: file order, single-use constants, standard-library imports inside functions, docstrings that only repeat the name, blind excepts, and handlers that turn a failure into None. Fix each hit, or mark one that's right as written with # cleanup: ignore[rule-id] and the reason.
3. Delete first: dead guards, single-use constants whose name adds nothing, and wrappers that only reword an error. Each deletion shrinks what the rest of the review has to cover.
4. Then review the change as a whole:
   • Explain why this approach over the alternatives (mixin vs. inheritance vs. duplication); reviewers consistently ask for that rationale.
   • Don't change an existing default or signature — that breaks users — unless a breaking change is the explicit goal of the PR.
   • Treat every new # noqa as a review question: ask what it works around, and whether the workaround is the real problem. A # noqa: B904 explaining that a retry helper would otherwise show the raw database error to the model points at a retry helper that reads the wrong error.
   • Keep each PR to one change a reviewer can hold in their head and revert on its own; a commit that bundles eight fixes can't be rolled back one fix at a time. If fixing the review comments would touch most of the diff, recommend splitting the PR or starting over from a smaller request rather than a cleanup pass.
   • Scrutinize AI-assisted code like any other; flag it and verify the behavior yourself.