Skip to content

Latest commit

 

History

8 Commits

Folders and files

NameName
Last commit message
Last commit date
 
 
 
 
 
 
 
 
 
 

Repository files navigation

Disciplined Code Changes

A Claude Code plugin containing one skill: engineering guardrails against the ways code assistance commonly goes wrong.

What it does

The skill loads when Claude is about to write, modify, or refactor code, and holds the work to ten rules:

  • Prefer small, verifiable changes
  • Keep changes scoped to the requested task
  • Validate behavior, not just compilation
  • Check assumptions
  • Prefer the simplest correct solution
  • Preserve unrelated code
  • Keep the codebase clean
  • Surface uncertainty and tradeoffs
  • Do not optimize prematurely
  • Do not overdo documentation

Each rule is paired with a concrete description of what violating it looks like, so the model can catch itself rather than merely agreeing with the principle. The skill ends with a five-item checklist to clear before handing work back.

A note on cost

This skill adds a set of standing constraints to the context of every coding task it applies to, and it asks for real validation rather than a claim that something compiles. Both cost tokens, and the validation step costs time.

That trade is worth it on changes that touch behavior, span more than one file, or land in code other people maintain. It is not worth it on a one-line typo fix, a rename, or a throwaway script. Turn it off, or leave it uninvoked, for trivial work.

The rules, with examples in Kotlin

Prefer small, verifiable changes

Task: add a retry to fetchUser.

// ❌ Replaces the call site with a general-purpose retrying HTTP layer, changes
//    the return type, and touches every caller — all in one unverified step.
class RetryingApiClient<T>(private val delegate: ApiClient, private val policy: RetryPolicy) { /* ... */ }
suspend fun fetchUser(id: String): Result<User> = retryingClient.call { getUser(id) }

// ✅ One change, at one call site, with a test that fails without it.
suspend fun fetchUser(id: String): User = retry(times = 3) { api.getUser(id) }

Keep changes scoped to the requested task

Task: fix the null timeout.

// ❌ Fixes the timeout, and also renames the parameter, converts the body to an
//    expression, and reorders the file's imports. The real fix is now one line
//    inside a forty-line diff.

// ✅ The fix, and nothing else.
val timeout = config.requestTimeout ?: DEFAULT_TIMEOUT

Unrelated improvements you spot are worth mentioning separately. They are not worth folding into this diff.

Validate behavior, not just compilation

@Test
fun `falls back to the default when the configured timeout is null`() {
    val client = buildClient(config = Config(requestTimeout = null))
    assertEquals(DEFAULT_TIMEOUT, client.callTimeout)
}
./gradlew :core:test --tests "*ClientConfigTest*"
BUILD SUCCESSFUL — 4 tests, 0 failures

A green build proves the code parses. Only the test proves the fallback fires.

Check assumptions

// ❌ Assumes Instant.parse accepts a plain date. It does not — this throws
//    DateTimeParseException at runtime on every request.
val day = Instant.parse(request.date)   // request.date == "2026-09-02"

// ✅ Checked the format the caller actually sends, then used the matching type.
val day = LocalDate.parse(request.date)

The cost of checking is one minute. The cost of not checking is a production stack trace.

Prefer the simplest correct solution

// ❌ An interface, an implementation, and a factory — for one call site with one
//    implementation that will never have a second.
interface PriceFormatter { fun format(cents: Long): String }
class DefaultPriceFormatter : PriceFormatter { override fun format(cents: Long) = "$%.2f".format(cents / 100.0) }
class PriceFormatterFactory { fun create(): PriceFormatter = DefaultPriceFormatter() }

//
fun formatPrice(cents: Long): String = "$%.2f".format(cents / 100.0)

Add the interface when the second implementation arrives, not in anticipation of it.

Preserve unrelated code

// ❌ Deleted the comment and the fallback, because the code "reads fine" and the
//    configured value "is obviously never null".
val timeout = config.requestTimeout

// ✅ Left in place. The comment is the only record of why the fallback exists.
// Fall back to the default timeout because the configured value can be null for
// newly created environments, and OkHttp requires a positive timeout value.
val timeout = config.requestTimeout ?: DEFAULT_TIMEOUT

Error handling you do not understand is error handling someone added for a reason.

Keep the codebase clean

// ❌ Scaffolding and editing-session narration left behind.
// Refactored to coroutines per request — was previously a callback
class UserRepository(private val api: ApiClient) {
    private fun legacyLoad(id: String) = api.getUserBlocking(id)  // no longer used

    suspend fun load(id: String): User {
        println("DEBUG loading $id")
        return api.getUser(id)
    }
}

//
class UserRepository(private val api: ApiClient) {
    suspend fun load(id: String): User = api.getUser(id)
}

Comments explain the code as it stands, to a reader who was not there when it changed.

Surface uncertainty and tradeoffs

Task: "make the export endpoint faster."

// ❌ Silently picks streaming, and presents it as the only possible answer.

// ✅ States the fork before writing either one:
//
//    Two approaches, and they differ in more than speed:
//
//    a) Stream rows as they are read. Constant memory, works for any export size,
//       but the response can no longer carry a Content-Length or fail cleanly
//       after the first byte is sent.
//    b) Page the query and buffer. Keeps the current error semantics, but memory
//       still scales with export size.
//
//    (a) if exports can exceed a few hundred MB. Which is it?

Do not optimize prematurely

// ❌ A thread-safe memoization cache around a pure function over a list that
//    holds, at most, a dozen elements.
private val cache = ConcurrentHashMap<String, BigDecimal>()
fun totalFor(order: Order): BigDecimal = cache.getOrPut(order.id) { order.lines.sumOf { it.amount } }

// ✅ Correct first. Add the cache when a profile shows this line matters.
fun totalFor(order: Order): BigDecimal = order.lines.sumOf { it.amount }

The cache above is also a memory leak and a stale-data bug, bought in exchange for an unmeasured gain.

Do not overdo documentation

// ❌ Three lines restating the signature.
/**
 * Gets the user id.
 *
 * @return the user id
 */
val userId: String

// ✅ Documents what the type cannot say: the unit, and the reason for null.
/** Request timeout in milliseconds. Null for environments created before v2.4. */
val requestTimeout: Long?

Documentation that restates the code goes stale silently and helps nobody.

Install

/plugin marketplace add egkz/disciplined-code-changes-plugin
/plugin install disciplined-code-changes

To try it before installing:

claude --plugin-dir /path/to/disciplined-code-changes-plugin

Contents

.claude-plugin/plugin.json
.claude-plugin/marketplace.json
skills/disciplined-code-changes/SKILL.md

License

MIT

About

Engineering guardrails against the ways AI code assistance commonly goes wrong.

Topics

Resources

Stars

2 stars

Watchers

0 watching

Forks

Releases

Packages

Contributors