Plan paying down one piece of technical debt as a refactor that serves the goal it is blocking — characterisation tests on the riskiest behaviour first, then small steps each merged to main with main releasable after every one, pure refactors kept apart from the behaviour changes the goal needs, a scope fence, a stop point, a technique per step (extract, branch by abstraction, parallel change, strangler fig, mikado) and the clashes with in-flight feature work. Use when the user asks for a refactor plan, how to untangle, break up, replace or clean up a module without a rewrite, how to pay down a tech-debt item, "how do we refactor X without stopping feature work", "how do we get off X without a big bang", or wants a strangler-fig or mikado plan.
Pack dev-teamTier standardJob Build and review softwarev0.2.0
In Claude Code, with the dev-team plugin installed, invoke it as:
/dev-team:refactor-plan
Try it
A realistic prompt from the skill's acceptance evals:
This 1,800-line OrderService is where every change goes wrong; plan how we untangle it without stopping feature work. The code is order_service.py, the register row is debt-register-row.md, coverage is in test-coverage.txt, and what's in flight is in-flight-prs.md.
What a good result looks like
The first two lines give the approach and the first step. A chat question gets a chat-sized plan; a document only when asked or large.
Every step serves the stated goal, and the last step reaches the structure that goal needs.
Every step is labelled refactor or behaviour change; behaviour changes stand alone, after the refactor that makes them safe.
Characterisation tests come first, on the incident-prone behaviour, and no open PR leaves that behaviour unpinned.
Caller contracts, copies outside the module and real test coverage are reported from the files, cited file:function.
Every in-flight clash has a decision, and a must-land feature PR is never blocked.
The scope fence and stop point exist; new findings go to the register.
Every file, function, metric and number is from the input. Unknowns are checks to run.
Example output
search ranker with a flag pile · the skill's own worked example, source
Input
TD-07 from our register got the go-ahead. search/ranker.py has one 600-line function, rank(query, docs, **flags), driven by 14 boolean flags listed in RANK_FLAGS at the top of the file. Register row: "5 of the last 8 search PRs changed rank(); INC-540 (Jul, legacy_tiebreak reversed the order of equal scores) and INC-552 (Aug, personalise is ignored when boost_recent is on — not fixed; the web client sends personalise only when boost_recent is off)." tests/search/test_ranker.py covers 4 of the 14 flags. PR #212 "Add boost_local flag" is open and edits rank(). Next quarter the mobile app needs its own ranking with different weights. Owner: Search team.
Output
Reply in chat (the default: the user asked how, not for a document)
Turn each flag into its own scoring function and build a ranking from a list of them, so the mobile ranking is a new list rather than an edit to rank(). First step: approval tests for every flag in a new test file, this week, because rank() has tests for only 4 of 14 flags.
What the code shows
search/ranker.py:rank reads all 14 RANK_FLAGS in one body; tests/search/test_ranker.py covers 4 of them.
INC-552 is live behaviour a client depends on: the web client sends personalise only when boost_recent is off, so the test pins it as-is.
#212 is editing rank() now; moves wait for it, tests don't.
Steps (each one PR to main; you can stop after any of them)
[refactor] Approval tests in tests/search/test_ranker_characterisation.py: personalise + boost_recent together (INC-552, as-is), legacy_tiebreak on equal scores (INC-540), then each flag alone.
[refactor] After #212 merges: extract the tie-break block into _tiebreak, unchanged; it caused INC-540.
[refactor] Add rank_with_profile(query, docs, profile); rank(**flags) builds a profile and delegates. The mobile team can start from here.
[refactor] Move the 15 flags (with boost_local) into scoring functions, five per PR, three PRs.
[refactor] Move callers to rank_with_profile, delete **flags, add a CI check that forbids new flags.
Target: search PRs that edit rank(), 5 of the last 8 → the mobile ranking ships without editing it. Stop point: every flag is a scoring function (step 4); step 5 is scheduled but optional. Not touched: INC-552 behaviour, RANK_FLAGS names, weights, the API response. Anything found during the work that isn't a step goes to the debt register. To confirm: does anything outside this repo call rank() with flags? It decides whether step 5 deletes **flags or keeps a shim.
The same plan as a document (when the user asks for one)
Refactor plan: take the scoring rules out of rank() so new rankings stop editing it
Target: search PRs that edit rank() — baseline 5 of the last 8 (TD-07) → the mobile ranking ships without editing rank(). Stop point: every flag in RANK_FLAGS is its own scoring function, and a ranking is built from a list of them. Removing **flags from rank() is scheduled but outside the stop point (step 7). **First step: characterisation tests for all 14 flags, then the expand step for a profile-based rank — first because the mobile ranking next quarter is the work this unblocks. Type: every step is a refactor; ranking output is identical throughout. The mobile weights are feature work built on step 3. Not touched:** INC-552 behaviour, RANK_FLAGS names, the search API response shape. [assumption] rank() is called only from inside this repo; see To confirm.
Steps
#
Change
Technique
Verified by
Rollback (live side)
Risk
Size
Owner
1
Approval tests in a new tests/search/test_ranker_characterisation.py: personalise + boost_recent together, legacy_tiebreak on equal scores, then each flag alone
characterisation tests
new tests pass on current code
revert PR
none, tests only
M
Search team
—
Wait for #212 to merge
—
—
—
—
—
—
2
Extract the tie-break block into _tiebreak(docs, legacy) in ranker.py, unchanged
extract
step 1 tests
revert PR
ordering of equal scores (INC-540)
S
Search team
3
Add rank_with_profile(query, docs, profile); rank(**flags) builds a profile from the flags and delegates
parallel change (expand)
step 1 tests; rank output identical
revert PR; rank is the only caller
flag-to-profile mapping misses a flag
M
Search team
4–6
Move flags into scoring functions, five, five and five per PR (15 with boost_local)
extract, behind step 3
step 1 tests + #212's tests
revert that batch's PR
interaction between flags in one batch
M each
Search team
7
Move callers to rank_with_profile; delete **flags; CI check forbids new flags
parallel change (contract)
suite + check
revert PR
caller outside the repo
S
Search team
Stop after any step: each leaves rank() shorter and better tested. Two entry points exist from step 3 to step 7 and nowhere else; step 7 is scheduled for the sprint after step 6.
Step detail (step 3 shown)
Change: new rank_with_profile in search/ranker.py; rank keeps its signature and builds a profile from **flags.
Technique: parallel change — callers move over later, so the old entry point stays live.
Verified by: every step 1 approval test, run through rank (unchanged callers).
Rollback: revert the PR; nothing calls rank_with_profile yet.
Risk: a flag dropped from the profile mapping changes ranking silently; the per-flag approval tests catch it.
Size: M. Owner: Search team.
Clashes: none once #212 has merged.
Characterisation tests
Function
Pin
Includes current bug?
Lives in
rank
each of the 14 flags alone over a recorded query set
no
tests/search/test_ranker_characterisation.py (new, so it can't conflict with #212)
rank
equal scores with legacy_tiebreak on and off
no (INC-540 is fixed)
same
rank
personalise + boost_recent together
yes: personalise ignored, pinned as-is (INC-552)
same
Clashes with feature work
In-flight work
Shared files / functions
Decision
Effect on the plan
#212 Add boost_local
search/ranker.py::rank
wait
step 2 starts after #212 merges; boost_local joins batch 4–6
Mobile ranking (next quarter)
rank
fold in
the mobile team builds its profile on rank_with_profile once step 6 lands
Scope fence
Not touched: search API response shape, RANK_FLAGS names, weights.
Pulled out of this plan: fixing INC-552 is a behaviour change; it is a separate ticket, and the web client workaround has to be removed in the same change.
Rule: anything found during the work that isn't in a step goes to the debt register as a new item, not into this plan.
To confirm
Does anything outside this repo call rank() with flags? It decides whether step 7 can delete **flags or must keep a shim.
How it's checked
Measured against plain Claude Code (2026-10-23): it met 57% of its checks, against 38% without it, and a blind judge preferred it. How skills are checked
55 assertions across 7 acceptance evals, and 11 trigger queries. Every skill's checks
What the output must do
Opens with the approach and the first step in its first lines, and states the target with its baseline taken from the register row (9 of the last 12 order PRs touching order_service.py, or the 3.1-day against 1.2-day review time) and a checkable stop point
Characterisation tests come before any step that touches calculate_tax, refund or send_confirmation_email (0% coverage), and they pin the per-line tax rounding and the zero-price negative-quantity lines as they behave today rather than fixing them
No step is a rewrite or a long-lived branch: every step is one PR merged to main, and moves are separated from changes
Each step says what changes, why (naming the technique where it matters), which tests verify it, and whether it is a pure refactor or a behaviour change; any step that adds a flag or an abstraction says which side is live for rollback
Names the clash with #481 (apply_discounts, charge_payment) and #486 (send_confirmation_email) and decides how each is handled (land first, wait, fold in, route through the new seam, or a short freeze window), rather than moving that code under the open PRs; characterisation tests are not delayed by either PR
The scope fence keeps the confirmation-email-on-failed-payment behaviour (INC-715) and the rounding out of the refactor as behaviour changes, and says new findings go to the debt register
Says the plan can stop after any step with the code better than before, and gives a reason the first real step goes first (for example the planned partial refunds or the in-flight gift-card work)
Does not invent modules, metrics or numbers not in the input; every file and function cited exists in the four files
Answers at the size of the ask: the plan is in the reply as a numbered sequence of steps of a line or two each, not a long multi-section document, since no document was requested
Produces the plan without asking questions first, with its assumptions tagged [assumption] at the top
Pulls the env var rename out of the refactor as a behaviour change, and either sends it to its own later parallel change (read both names, then drop the old) or to a separate ticket, instead of folding it into the steps
Uses 'about 40 direct os.environ reads' as the baseline and does not invent other measured numbers such as test times or file counts; anything else to measure is marked not measured
Introduces the settings object first and moves call sites over in batches of one PR each, so main stays releasable between batches
Characterisation tests pin how values are parsed today (defaults when unset, how strings like 'false' or '0' are read) before call sites move
The stop point is zero direct os.environ reads outside the settings object, enforced by a check in CI or a test
Does not invent file names, module names or settings that the prompt did not mention
Builds the order from imports.txt as a prerequisite graph (mikado-style or explicit dependency order): the goal, the import edges blocking it, and which edges change cleanly first, with the reason
States the target as the count of billing-to-web imports in imports.txt (seven) going to zero, or the four in invoices.py and dunning.py that block the worker as the first milestone, with the final stop point enforced by an import check in CI
Puts characterisation tests for billing/proration.py before any step that changes it, because the register row says it has no tests, and relies on tests/billing/ for invoices and dunning
Sequences the edges that block worker/nightly.py (in billing/invoices.py and billing/dunning.py) before the proration and tax edges the worker doesn't need, and says the plan can stop with the worker unblocked and the code better
Pulls the invoices.total_cents column change out of the refactor as a schema change with data to migrate, to be planned separately as a migration, and does not fold it into any step
Every step cites the specific import line or file from imports.txt it removes, and states its rollback
Does not invent modules, functions, metrics or numbers beyond imports.txt and register.md
Spots that src/partners/statement.py depends on build_invoice mutating the caller-supplied line_items list (it reads the appended tax line back from its own lines list, not from the returned invoice), and the plan either keeps that side effect working or moves that caller off it in its own step before the mutation is removed. It does not quietly make build_invoice pure.
Spots that reports/monthly_revenue.py has its own copied TAX_RATES and tax logic, which has already drifted (no US-TX, no EU reverse-charge rule). The plan puts it explicitly inside or outside the scope fence, and does not claim the refactor removes duplicated tax logic if the report is left out.
Puts characterisation tests for build_invoice first, before any extraction, and says they must cover the existing branches (short-period proration, annual anniversary month, included seats/annual add-on discount, usage tiers, each coupon type including expiry, tax-exempt and reverse charge, non-GBP conversion, credit notes, rounding). They pin current output, float-rounding quirks included. Notes that test_payments.py gives no coverage of invoice.py.
Deals with the clash with PR #412 (tax block and TAX_RATES, committed to customers for 1 Nov): it sequences the tax-block work after #412 merges, or otherwise avoids conflicting edits to that block while #412 is open. It also mentions draft PR #420 touching the add-ons block.
Keeps DEBT-15 (float to Decimal money) and any change to rounding or tax results out of scope as separate work, and states that every step preserves behaviour, i.e. invoice output is unchanged.
Breaks the work into small, separately mergeable PRs, each leaving main releasable. Each step names a specific technique that fits it, e.g. extract function per section (plan, add-ons, usage, coupons, tax, currency, credit note), or parallel change / expand-contract for the line_items parameter or the signature with 9 parameters.
Sets a measurable target with a baseline taken from the files (e.g. build_invoice is about 165 lines with 9 parameters and 0 tests; or the register's '2 incidents in Q3' / 'most of a sprint per tax region') or marked as 'to measure'. It also defines an explicit stop point where the refactor is finished.
Invents no facts: no made-up cyclomatic-complexity numbers, coverage percentages, incident counts, team sizes or dates beyond those in the inputs, and no claims that callers, modules or tests exist beyond those shown. New modules it proposes are clearly labelled as proposed. Unknowns, such as other callers of build_invoice, appear as checks to run (e.g. grep for build_invoice and line_items=), not as stated facts.
Flags that LegacyMailer.send returns true even when sending fails (the catch block returns true), and that src/orders/onOrderShipped.ts has a retryQueue branch on !sent that has never fired as a result. Warns that a new client which honestly returns false (or throws) would change behaviour, with possible retries or duplicate emails. Keeps the current semantics at the seam and treats surfacing errors as a separate, deliberate change.
Finds that scripts/nightly-digest.js has its own copy of the SMTP send and renderer, bypasses LegacyMailer, and skips the suppression list. Brings it into the plan, since the relay can't be switched off while it exists, and does not treat deleting legacyMailer.ts as finishing the migration.
Proposes an incremental migration through a seam (branch by abstraction and/or strangler fig): an interface the callers depend on, the new HTTP-provider implementation behind it, a per-template or per-caller switch via flag/config with a quick rollback, and legacy code deleted only once traffic on it is zero.
Lists the LegacyMailer behaviours that need characterisation tests before the swap: comma-splitting and lowercasing recipients, suppression-file filtering (and returning true when all recipients are suppressed), bccOps, per-template subjects including the 'there' fallback, missing vars rendering as empty, sendBulk's sequential 200ms pacing. Notes the existing test only covers render().
Handles each in-flight PR that matters: #88 (adds a template and subject inside legacyMailer.ts; must land for peak) is allowed to land first or is accommodated, not blocked. #91 (rewrites loadTemplate/templates to MJML) is flagged as a clash with the template-rendering step. #93 (new mailer.send caller for magic_link) is told to go through the new seam.
Sizes and sequences the steps against ci-notes.md (about 38 min e2e that flakes about 1 in 6, manual prod approval, about 2 deploys a week) and the end-of-Q1 deadline. Each step is its own small PR leaving main releasable. Includes a measurable target and stop point, e.g. templates or call sites still on the relay going from a baseline counted in the files to 0, or relay send volume reaching 0, before the contract ends.
Invents no facts: no named email vendor or specific API endpoints, no email volumes, error rates, team size or calendar dates beyond 'end of Q1', and no claim of call sites beyond those in the files. Unknowns (other callers of mailer, the provider's suppression handling, which templates are highest volume) are raised as questions or checks.
Delivers a usable plan in this reply despite the vague request, asking at most a few targeted questions, and recommends neither a big-bang cutover nor a feature freeze.
Flags that DunningJob.run() mutates invoices returned by findOverdue (status, dunningLevel, lastDunnedOn) without saving them, relying on Hibernate dirty checking inside @Transactional, and makes the plan handle it (an explicit save step or a test pinning that the changes persist) before findOverdue moves behind a non-JPA interface or the ORM changes.
Notices that InvoicePdfRenderer.taxFor is a copy of computeLineTax but rounds HALF_UP instead of HALF_EVEN, and does not silently merge the two as a behaviour-preserving step: it either fences the renderer out of scope or raises the mismatch as a decision for finance/the team (ADR-007 says tax rules are not to change).
Puts characterisation tests ahead of moving the untested logic, naming at least applyLateFees (grace days, exemption, the daysLate/30 multiplier, only raising the fee) and per-line tax rounding, since the existing tests only cover createInvoice; and says these tests should not need a database once the rules are extracted, or explains how they move from @SpringBootTest to plain unit tests.
Sequences around PR #412: does not start moving applyLateFees until #412 (approved, expected to merge this week) has landed, or explicitly rebases onto it; and names PR #415 (credit notes, changes void, about 3 weeks out) as a clash to coordinate with, e.g. by agreeing where issueCreditNote should live.
Sets a target taken from ADR-007's precondition, such as: invoice rule code has no jakarta.persistence imports and no EntityManager dependency, with the baseline read from the current file and a stop point at that precondition; the jOOQ migration itself is explicitly outside the scope fence, as are tax and late-fee rule changes.
Each step is a separate, small PR that leaves main releasable and behaviour unchanged, and each names a concrete technique that fits it (e.g. extract a pure InvoiceCalculator / late-fee policy, introduce a repository port in front of the EntityManager queries as branch by abstraction, parallel change for the findOverdue callers). It does not propose a rewrite or a long-lived branch.
Treats markPaid's native customer balance update (Customer.balance is mapped read-only) as persistence that has to stay atomic with the invoice save, and keeps it on the persistence side rather than moving it into the rules code.
Invents no modules, files, metrics, dates or numbers that are not in the provided files (no made-up coverage percentages, line counts, team sizes or PR numbers); anything new it proposes, such as a class name, is clearly labelled as a proposal.
The answer is short and easy to scan, matching a casual one-line question: a brief plan of a handful of steps, not a long formal document with many sections, and it does not ask a string of clarifying questions before giving a plan.
Respects the release: nothing beyond adding tests or trivial, zero-risk changes goes into CheckoutPanel before the 30 Oct code freeze / 3 Nov v4.8 release, and the structural refactor starts after release (or the plan explains why a specific step is safe to land before freeze).
Puts gift cards PR #231 (in v4.8, rewrites the totals block and submit(), expected merge by 28 Oct) ahead of any refactor of the same code, so the refactor does not conflict with it; and mentions #236 (address autocomplete, parked until after release) as touching the address section the plan would move.
Spots that MiniCart.tsx duplicates the totals maths but differs from CheckoutPanel (it rounds the discount per line instead of on the subtotal, and its free-shipping test is >= 5000 where CheckoutPanel uses > 5000), links this to WEB-512's 1p mismatch, and treats unifying them as a deliberate behaviour change needing a decision on which is correct, not a behaviour-preserving step.
Spots that OrderReview.tsx reads the 'checkout_draft' localStorage entry that CheckoutPanel writes (including total), and keeps that contract unchanged (or pins it with a test) when draft or totals logic is moved into a hook or module.
Asks for characterisation tests before moving logic, because the only existing test is a single snapshot: at least totals with a promo and the free-shipping threshold, and the draft write, as plain tests on behaviour rather than more snapshots.
Gives a target and stop point tied to something real in the input, for example WEB-530 (pay-by-bank in Q1 needs the payment step to stop assuming card fields) or pulling the totals and validation into pure functions, and says what is out of scope (e.g. not a redesign of the checkout flow).
Invents no files, metrics, dates or numbers not in the provided files; any count it quotes (e.g. how many useState calls there are or the file's length) matches the actual file, and proposed new hooks or modules are clearly labelled as proposals.
When it should and shouldn't activate
Should activate:
This 1,800-line OrderService is where every change goes wrong; plan how we untangle it without stopping feature work
TD-04 got approved for this quarter. Plan the refactor as small PRs we can ship one at a time
How do we break billing's dependency on the web package without a big-bang rewrite?
Give me a strangler-fig plan for replacing our legacy report builder module piece by piece
We want to stop calling the SMTP client from 14 places — plan the refactor with characterisation tests first
Should leave to another skill:
Go through our TODOs and incident notes and tell me which tech debt to pay down this quarter
Plan the cutover from MySQL to Postgres with a backfill and no downtime
Implement step 2 of the refactor plan: move calculate_tax into its own module with tests
Write an ADR for splitting the monolith into an orders service and a billing service
Review this refactoring PR before I merge it
Split the gift-card checkout epic into user stories
Hands off to
feature-implement-loop — when the plan is agreed and a step is ready to build. Implement one step, using its Change and Verified-by fields as the acceptance criteria.
tech-debt-register — when planning or a step turns up debt outside the scope fence, or the stop point is reached. Record new debt as its own item and update the paid-down item's cost and decision.