{"id":"8c1d001e-1eed-4306-a9c3-b78a3daa5ced","shortId":"8jbHEg","kind":"skill","title":"code-quality","tagline":"Load when reviewing a diff, naming a code smell or anti-pattern, deciding refactoring direction, or grading review-comment severity. Required reading when the architecture or code-review capabilities cite a named pattern — load explicitly rather than paraphrasing from memory.","description":"Reference content for naming and addressing structural and hygiene problems in code. Consumers cite the relevant pattern or rule by name.\n\n## Discipline\n\nWhen citing any pattern or rule from this catalog:\n\n- **Name** the pattern (e.g. \"Feature Envy\", \"Premature Abstraction\"); cite Fowler or Evans where applicable.\n- **Refactoring direction** — propose a concrete move (e.g. \"Move Method to X\", \"Replace Conditional with Polymorphism\", \"Extract Policy object\").\n- **Severity** — `blocker` if the change entrenches a pre-existing scattered pattern by adding a callsite; `suggestion` if it introduces a new pattern or a prerequisite refactor would simplify the work.\n\n## Pre-existing pattern rule\n\n**\"Make the change easy, then make the easy change.\"** — Kent Beck. When working with code that has pre-existing problems:\n\n- **Doesn't worsen it** (reads, passes through, no new callsite): skip — never flag ambient patterns the work doesn't interact with.\n- **Adds another callsite** to a scattered pattern: flag as **blocker** — stop and extract.\n- **Depends on it** (would be simpler if fixed first): flag as **suggestion** — \"Consider extracting [X] first as a prerequisite.\" Suggest a separate issue.\n\n---\n\n## System-level anti-patterns\n\nFor evaluating a *proposed approach* before code exists.\n\n- **Premature abstraction** — interface/layer before 2+ concrete implementations.\n- **Wrong layer** — business logic in DB, presentation in service layer, persistence in domain.\n- **Leaky abstraction** — forces callers to know its internals.\n- **Distributed monolith** — services that must deploy together or share a database.\n- **Config as code** — logic that belongs in code lives in feature flags or env vars.\n- **Speculative generality** — building for scale or flexibility not needed yet.\n\n## Code-level smells\n\nFor scanning a *concrete diff* for structural problems.\n\n### Tell, Don't Ask / Feature Envy\n\nCallsites pulling state out of an object to make a decision the object should make itself.\n\nSignals: multiple files checking the same attribute chain (`record.status == :locked`, `record.full_lock?`); same conditional duplicated across services/controllers/views/helpers; new method replicating a check already in another layer.\n\nWhen found: name the violated object, list every callsite, suggest where the logic belongs (policy, model method, service). Adding *another* callsite to an already-scattered pattern is a **blocker**.\n\n### Scattered Enforcement\n\nA specific Tell-Don't-Ask: authorization or validation enforced in multiple places independently. Access decisions in controller AND service AND view; policy methods partially redundant with service-layer guards; lock/state checks in helpers and views duplicating service rules.\n\nFlag: \"Enforced in N places — [list]. Any new callsite should go through [central point].\"\n\n### Shotgun Surgery\n\nOne logical change requires edits across 5+ unrelated files. The rule isn't encapsulated.\n\n### Primitive Obsession / Data Clumps\n\nPrimitives passed together repeatedly where a value object would clarify intent. Domain concepts as raw strings or magic numbers. A parameter hash that grows beyond 3-4 keys and always travels together.\n\n### Layering Violations\n\nBusiness logic in a view or controller. Persistence logic in domain objects (queries in presenters/helpers). View conditionals encoding business rules.\n\n### Anemic Domain Model\n\nRich service classes doing all the work, model is a data bag. Methods that only do `self.field = value; save`. New service files reconstructing domain knowledge already implicit in the model. Only flag when the diff *adds* to this pattern.\n\n### Anti-Extensibility Conditionals\n\nNew `if/elsif/case` on type/state/role where polymorphism or strategy would eliminate future N+1 branching. Existing `case` extended with another arm — flag \"this case has N arms now; consider a table or strategy pattern.\"\n\n### Coupling Introduced\n\nNew cross-module dependency where modules had no prior relationship. New parameter threaded through multiple layers. New shared mutable structure (global config, class-level state) creating hidden coupling.\n\n---\n\n## Hygiene rules\n\nFor diff review beyond named patterns.\n\n- **Single responsibility** — flag functions/classes doing too many things.\n- **Method placement** — method with one call site in a different domain belongs closer to caller.\n- **Naming** — flag imprecise (`data`, `info`, `handle`, `process`, `temp`) or domain-opaque (`internal`, `app`, `type`, `status` without qualification).\n- **Dead code** — for every new function, method, scope, constant, or route in the diff, search for callers; zero callers = dead.\n- **Orphaned code** — for every call the diff REMOVES, check if the target still has other callers; last caller removed = dead.\n- **DRY violations** — duplicated logic where existing patterns could be extended. Confirm the pattern exists elsewhere via code search before flagging.\n- **UX pattern drift** — different interactions than established patterns on similar pages (inline vs. modal editing, URL-synced vs. non-synced filters).\n- **Job granularity** — N individual jobs in a loop where one batch job would work, or vice versa.\n- **Propagation** — when the diff renames, removes, or semantically changes a column, attribute, enum, scope, association, route, or method, search the entire codebase (source, views, JS, JSON, YAML, fixtures, exports, serializers, mailers, jobs, seed files, admin pages, marketing pages). Any surviving reference not updated by the diff is a blocker.\n- **Caching correctness** — when the diff reads from caches (in-process memoization, Redis, framework cache stores), verify the cached value can't be stale relative to the operation's requirements, especially after writes.\n- **Minimize diff** — unnecessary whitespace changes (blank lines added/removed, trailing whitespace, re-indentation), unnecessary formatting changes, unrelated refactors, scope creep.\n\n## Tests\n\n- **Test coverage** — every changed behavior has tests. Missing coverage on a happy path or a documented edge case is a finding.\n- **Test validity** — stubs/mocks must target methods actually called in the code path being tested. A stub on a method outside the execution path means the test asserts nothing.\n- **Testing pyramid** — tests sit at the right tier. Most coverage is unit (fast, isolated, exercises one module). Integration tests are for cross-module contracts. End-to-end tests are sparse, expensive, used for critical user flows. Flag when:\n  - Logic that could be unit-tested is only exercised through integration or end-to-end tests (slow feedback, hides regressions).\n  - Logic that needs real component interaction is only unit-tested with mocks (false confidence).\n  - Pyramid is inverted: many e2e tests, few unit tests.\n- **Test placement** — a test belongs at the lowest tier that can validate the behavior. If a unit test covers the requirement, an integration or e2e test for the same logic is duplication.\n\n## Auditing pre-existing issues\n\nCheck version-control history to confirm an issue is from the current diff, not pre-existing. Apply the pre-existing pattern rule above.","tags":["code","quality","dotfiles","athal7","agent-skills"],"capabilities":["skill","source-athal7","skill-code-quality","topic-agent-skills"],"categories":["dotfiles"],"synonyms":[],"warnings":[],"endpointUrl":"https://skills.sh/athal7/dotfiles/code-quality","protocol":"skill","transport":"skills-sh","auth":{"type":"none","details":{"cli":"npx skills add athal7/dotfiles","source_repo":"https://github.com/athal7/dotfiles","install_from":"skills.sh"}},"qualityScore":"0.453","qualityRationale":"deterministic score 0.45 from registry signals: · indexed on github topic:agent-skills · 6 github stars · SKILL.md body (7,158 chars)","verified":false,"liveness":"unknown","lastLivenessCheck":null,"agentReviews":{"count":0,"score_avg":null,"cost_usd_avg":null,"success_rate":null,"latency_p50_ms":null,"narrative_summary":null,"summary_updated_at":null},"enrichmentModel":"deterministic:skill-github:v1","enrichmentVersion":1,"enrichedAt":"2026-05-18T19:14:34.187Z","embedding":null,"createdAt":"2026-05-18T13:22:28.779Z","updatedAt":"2026-05-18T19:14:34.187Z","lastSeenAt":"2026-05-18T19:14:34.187Z","tsv":"'+1':582 '-4':496 '2':242 '3':495 '5':458 'abstract':85,239,259 'access':410 'across':351,457 'actual':914 'ad':123,380 'add':188,562 'added/removed':873 'address':52 'admin':818 'alreadi':358,386,552 'already-scatt':385 'alway':499 'ambient':180 'anem':524 'anoth':189,360,381,588 'anti':15,228,567 'anti-extens':566 'anti-pattern':14,227 'app':679 'appli':1077 'applic':91 'approach':234 'architectur':30 'arm':589,595 'ask':317,401 'assert':934 'associ':798 'attribut':342,795 'audit':1054 'author':402 'bag':538 'batch':777 'beck':156 'behavior':891,1035 'belong':282,375,662,1026 'beyond':494,640 'blank':871 'blocker':111,197,391,832 'branch':583 'build':294 'busi':247,504,522 'cach':833,840,847,851 'call':656,708,915 'caller':261,665,700,702,719,721 'callsit':125,176,190,320,370,382,444 'capabl':35 'case':585,592,904 'catalog':77 'central':448 'chain':343 'chang':114,148,154,454,792,870,881,890 'check':339,357,428,712,1059 'cite':36,60,70,86 'clarifi':479 'class':529,629 'class-level':628 'closer':663 'clump':469 'code':2,11,33,58,160,236,279,284,303,685,705,740,918 'code-level':302 'code-qu':1 'code-review':32 'codebas':805 'column':794 'comment':24 'compon':1002 'concept':482 'concret':96,243,309 'condit':104,349,520,569 'confid':1012 'config':277,627 'confirm':734,1065 'consid':213,597 'constant':692 'consum':59 'content':48 'contract':960 'control':413,510,1062 'correct':834 'could':731,978 'coupl':603,634 'cover':1040 'coverag':888,895,945 'creat':632 'creep':885 'critic':971 'cross':607,958 'cross-modul':606,957 'current':1071 'data':468,537,669 'databas':276 'db':250 'dead':684,703,723 'decid':17 'decis':330,411 'depend':201,609 'deploy':271 'diff':8,310,561,638,697,710,787,829,837,867,1072 'differ':660,747 'direct':19,93 'disciplin':68 'distribut':266 'document':902 'doesn':167,184 'domain':257,481,514,525,550,661,676 'domain-opaqu':675 'dri':724 'drift':746 'duplic':350,433,726,1053 'e.g':81,98 'e2e':1017,1046 'easi':149,153 'edg':903 'edit':456,758 'elimin':579 'elsewher':738 'encapsul':465 'encod':521 'end':962,964,990,992 'end-to-end':961,989 'enforc':393,405,437 'entir':804 'entrench':115 'enum':796 'env':290 'envi':83,319 'especi':863 'establish':750 'evalu':231 'evan':89 'everi':369,687,707,889 'execut':929 'exercis':950,985 'exist':119,143,165,237,584,729,737,1057,1076,1081 'expens':968 'explicit':41 'export':812 'extend':586,733 'extens':568 'extract':107,200,214 'fals':1011 'fast':948 'featur':82,287,318 'feedback':995 'file':338,460,548,817 'filter':766 'find':907 'first':209,216 'fix':208 'fixtur':811 'flag':179,195,210,288,436,558,590,645,667,743,974 'flexibl':298 'flow':973 'forc':260 'format':880 'found':363 'fowler':87 'framework':846 'function':689 'functions/classes':646 'futur':580 'general':293 'global':626 'go':446 'grade':21 'granular':768 'grow':493 'guard':426 'handl':671 'happi':898 'hash':491 'helper':430 'hidden':633 'hide':996 'histori':1063 'hygien':55,635 'if/elsif/case':571 'implement':244 'implicit':553 'imprecis':668 'in-process':841 'indent':878 'independ':409 'individu':770 'info':670 'inlin':755 'integr':953,987,1044 'intent':480 'interact':186,748,1003 'interface/layer':240 'intern':265,678 'introduc':129,604 'invert':1015 'isn':463 'isol':949 'issu':223,1058,1067 'job':767,771,778,815 'js':808 'json':809 'kent':155 'key':497 'know':263 'knowledg':551 'last':720 'layer':246,254,361,425,502,621 'leaki':258 'level':226,304,630 'line':872 'list':368,441 'live':285 'load':4,40 'lock':345,347 'lock/state':427 'logic':248,280,374,453,505,512,727,976,998,1051 'loop':774 'lowest':1029 'magic':487 'mailer':814 'make':146,151,328,334 'mani':649,1016 'market':820 'mean':931 'memoiz':844 'memori':46 'method':100,354,378,419,539,651,653,690,801,913,926 'minim':866 'miss':894 'mock':1010 'modal':757 'model':377,526,534,556 'modul':608,611,952,959 'monolith':267 'move':97,99 'multipl':337,407,620 'must':270,911 'mutabl':624 'n':439,581,594,769 'name':9,38,50,67,78,364,641,666 'need':300,1000 'never':178 'new':131,175,353,443,546,570,605,616,622,688 'non':764 'non-sync':763 'noth':935 'number':488 'object':109,326,332,367,477,515 'obsess':467 'one':452,655,776,951 'opaqu':677 'oper':860 'orphan':704 'outsid':927 'page':754,819,821 'paramet':490,617 'paraphras':44 'partial':420 'pass':172,471 'path':899,919,930 'pattern':16,39,63,72,80,121,132,144,181,194,229,388,565,602,642,730,736,745,751,1082 'persist':255,511 'place':408,440 'placement':652,1023 'point':449 'polici':108,376,418 'polymorph':106,575 'pre':118,142,164,1056,1075,1080 'pre-exist':117,141,163,1055,1074,1079 'prematur':84,238 'prerequisit':135,219 'present':251 'presenters/helpers':518 'primit':466,470 'prior':614 'problem':56,166,313 'process':672,843 'propag':784 'propos':94,233 'pull':321 'pyramid':937,1013 'qualif':683 'qualiti':3 'queri':516 'rather':42 'raw':484 're':877 're-indent':876 'read':27,171,838 'real':1001 'reconstruct':549 'record.full':346 'record.status':344 'redi':845 'redund':421 'refactor':18,92,136,883 'refer':47,824 'regress':997 'relat':857 'relationship':615 'relev':62 'remov':711,722,789 'renam':788 'repeat':473 'replac':103 'replic':355 'requir':26,455,862,1042 'respons':644 'review':6,23,34,639 'review-com':22 'rich':527 'right':942 'rout':694,799 'rule':65,74,145,435,462,523,636,1083 'save':545 'scale':296 'scan':307 'scatter':120,193,387,392 'scope':691,797,884 'search':698,741,802 'seed':816 'self.field':543 'semant':791 'separ':222 'serial':813 'servic':253,268,379,415,424,434,528,547 'service-lay':423 'services/controllers/views/helpers':352 'sever':25,110 'share':274,623 'shotgun':450 'signal':336 'similar':753 'simpler':206 'simplifi':138 'singl':643 'sit':939 'site':657 'skill' 'skill-code-quality' 'skip':177 'slow':994 'smell':12,305 'sourc':806 'source-athal7' 'spars':967 'specif':395 'specul':292 'stale':856 'state':322,631 'status':681 'still':716 'stop':198 'store':848 'strategi':577,601 'string':485 'structur':53,312,625 'stub':923 'stubs/mocks':910 'suggest':126,212,220,371 'surgeri':451 'surviv':823 'sync':761,765 'system':225 'system-level':224 't-ask':399 'tabl':599 'target':715,912 'tell':314,397 'tell-don':396 'temp':673 'test':886,887,893,908,921,933,936,938,954,965,982,993,1008,1018,1021,1022,1025,1039,1047 'thing':650 'thread':618 'tier':943,1030 'togeth':272,472,501 'topic-agent-skills' 'trail':874 'travel':500 'type':680 'type/state/role':573 'unit':947,981,1007,1020,1038 'unit-test':980,1006 'unnecessari':868,879 'unrel':459,882 'updat':826 'url':760 'url-sync':759 'use':969 'user':972 'ux':744 'valid':404,909,1033 'valu':476,544,852 'var':291 'verifi':849 'versa':783 'version':1061 'version-control':1060 'via':739 'vice':782 'view':417,432,508,519,807 'violat':366,503,725 'vs':756,762 'whitespac':869,875 'without':682 'work':140,158,183,533,780 'worsen':169 'would':137,204,478,578,779 'write':865 'wrong':245 'x':102,215 'yaml':810 'yet':301 'zero':701","prices":[{"id":"1521729a-c211-4e21-a94d-5a3bbe23971a","listingId":"8c1d001e-1eed-4306-a9c3-b78a3daa5ced","amountUsd":"0","unit":"free","nativeCurrency":null,"nativeAmount":null,"chain":null,"payTo":null,"paymentMethod":"skill-free","isPrimary":true,"details":{"org":"athal7","category":"dotfiles","install_from":"skills.sh"},"createdAt":"2026-05-18T13:22:28.779Z"}],"sources":[{"listingId":"8c1d001e-1eed-4306-a9c3-b78a3daa5ced","source":"github","sourceId":"athal7/dotfiles/code-quality","sourceUrl":"https://github.com/athal7/dotfiles/tree/main/skills/code-quality","isPrimary":false,"firstSeenAt":"2026-05-18T13:22:28.779Z","lastSeenAt":"2026-05-18T19:14:34.187Z"}],"details":{"listingId":"8c1d001e-1eed-4306-a9c3-b78a3daa5ced","quickStartSnippet":null,"exampleRequest":null,"exampleResponse":null,"schema":null,"openapiUrl":null,"agentsTxtUrl":null,"citations":[],"useCases":[],"bestFor":[],"notFor":[],"kindDetails":{"org":"athal7","slug":"code-quality","github":{"repo":"athal7/dotfiles","stars":6,"topics":["agent-skills"],"license":null,"html_url":"https://github.com/athal7/dotfiles","pushed_at":"2026-05-18T18:53:57Z","description":null,"skill_md_sha":"18100e5c0c31800e1fa2971da461dc6cf825688d","skill_md_path":"skills/code-quality/SKILL.md","default_branch":"main","skill_tree_url":"https://github.com/athal7/dotfiles/tree/main/skills/code-quality"},"layout":"multi","source":"github","category":"dotfiles","frontmatter":{"name":"code-quality","license":"MIT","description":"Load when reviewing a diff, naming a code smell or anti-pattern, deciding refactoring direction, or grading review-comment severity. Required reading when the architecture or code-review capabilities cite a named pattern — load explicitly rather than paraphrasing from memory."},"skills_sh_url":"https://skills.sh/athal7/dotfiles/code-quality"},"updatedAt":"2026-05-18T19:14:34.187Z"}}