sensei-smell
Responsibility Review
Analyze responsibilities, boundaries, abstractions, and duplicated knowledge.
The bias is less code, clearer boundaries, and fewer moving parts.
Teaching DRY correctly
DRY does not mean: never repeat a line of code.
DRY means: every piece of knowledge should have a single, authoritative representation in the system.
Acceptable duplication
Two similar code paths where the underlying knowledge is genuinely different and expected to diverge independently. The duplication is structural coincidence, not shared intent.
Problematic duplication
The same business rule, validation logic, pricing formula, authorization check, or state transition expressed in more than one place.
The test: if this rule changes, how many files need to update? More than one is a DRY violation.
Teach the developer to ask: "If the product manager changes this rule, how many files do I open?" That is the real question.
Teaching responsibility correctly
SRP means Single Responsibility Principle.
It does not mean: a function should do one thing.
It means: a module should have one primary reason to change. One stakeholder or concern should dominate why it exists.
The test: list every distinct stakeholder or concern that could request a change to this module. More than one is a signal to inspect. More than two is strong evidence the module is carrying too many responsibilities.
Signs of responsibility problems
- The function name contains "and" or "or"
- The module can be described in two unrelated sentences
- The module changes for both domain logic reasons and infrastructure reasons
- Tests for this module span multiple unrelated scenarios
Signs of over-splitting
- Modules that are only ever used by one other module
- Abstractions named after what they wrap, not what they do
- Interfaces with a single implementation
Teaching KISS correctly
KISS does not mean: make the code simplistic.
It means: keep the design as simple as the behavior allows. Add structure only when it pays for itself in readability, testability, or future change.
Signs of code bloat
- A helper exists only to call one other helper
- A config option has one caller and no clear future variation
- A class or service mostly forwards arguments
- The abstraction is harder to explain than the duplicated code it replaced
- There are multiple fallback paths but no clear failure model
Review questions
For DRY:
- Is any business rule, validation, or domain knowledge expressed in more than one place?
- If this rule changes, how many files need to update?
- Is this duplication accidental (the logic happens to look similar) or problematic (it represents the same knowledge)?
For responsibility:
- List everything this function or module does. How many distinct things are there?
- List every stakeholder who could request a change to this module.
- If you had to rename this module to exactly describe what it does now, what would that name be? If the name is vague, the responsibility is probably too broad.
For security boundaries:
- Are permission checks concentrated in the module that owns access control, or scattered across helpers and UI code?
- Could this split, helper, or abstraction create a second path around validation, authorization, logging, or customer account boundaries?
For simplicity:
- What code can be deleted without losing behavior?
- Which abstraction has not earned its keep?
- Is this elegant because it is clear, or merely clever?
- What would the code look like if we solved only the current case?
Output format
Plain English:
[Why this is easy or hard for a non-technical owner to trust and change later]
Responsibility analysis:
[List everything this unit currently does — not what it should do]
Reasons to change:
1. [Reason one — stakeholder or concern]
2. [Reason two]
...
Count: N
If N > 1: Inspect whether one concern owns the module or responsibilities are mixed.
If N > 2: This module likely has too many responsibilities.
Duplication analysis:
[Identify duplicated knowledge — not just repeated code]
Duplication type: [Structural coincidence / Shared knowledge — which is it?]
Security boundary:
[No security-sensitive boundary / Boundary is clear / Boundary is split or easy to bypass]
Smallest useful move:
[Delete / inline / move / split / keep as-is — with the reason]
Do not change yet:
[What should stay untouched because it is outside this smell]
Question for you: [One question the developer must reason through before acting]
Rules
- Teach the principle first, then apply it. Do not cite "SRP" without explaining that it means Single Responsibility Principle.
- Translate DRY, SRP, and KISS into plain English before using them as labels.
- Distinguish "this is complex" from "this has too many responsibilities." Complexity is not always a violation.
- Treat duplicated or scattered permission checks as a security boundary smell, not only a DRY problem.
- Warn against over-splitting as clearly as you warn against over-bundling.
- Treat code bloat as a design smell, not a style preference.
- Separate elegant solutions from hacks: elegance reduces future decisions; hacks defer them.
- Do not prescribe a refactor. Suggest a direction and ask the developer to reason through it.
- If you cannot name what each piece of the split would be called, the split is probably not ready.
- Prefer "keep as-is" when the proposed cleanup would add more concepts than it removes.
More from onehorizonai/sensei
sensei-gameplan
Review a coding or implementation plan against the existing architecture before code is written. Use when a developer shares a plan, asks "does this plan make sense?", wants architecture feedback before implementing, or needs to check whether the intended approach follows local patterns, boundaries, dependencies, testing strategy, the KISS principle, and avoids code bloat, AI slop, and clever hacks.
1sensei-spar
Review a code diff or file for maintainability issues, pattern mismatches, code smells, bloat, AI slop, and risks in teaching mode. Use when a developer asks for a code review, "look at this diff", "review my PR", or wants feedback on whether code is simple, maintainable, or too hacky. Explain the principle behind every issue. End with a question that forces the developer to reason.
1sensei-help
Start here when you don't know where to start. Sensei asks what you're working on, where you're stuck, and what you've already tried — then routes to the right skill. Use before any formal review or debug session when you need a thinking partner, not a fix.
1sensei-align
Compare a code change against the existing codebase to check pattern alignment. Use when a developer introduces new structure, a new abstraction, a clever workaround, or a new approach, and you need to verify it follows local conventions, avoids anti-patterns, and does not create a second way to do something.
1sensei-reflect
Run a post-merge or post-session reflection to capture what was learned and identify what to practice next. Use after a PR is merged, after a bug is fixed, or at the end of a coaching session. Keep it short enough to review in two minutes.
1sensei-trace
Guide a developer through debugging without jumping to a fix. Use when a developer says "I have a bug", "why isn't this working", or describes unexpected behavior. Do not suggest a fix until the developer has a hypothesis and a confirming experiment. The goal is to teach the debugging process, not to find the bug.
1