Add Sdlc/Dry cop for repeated method skeleton #1321

Closed
opened 2026-10-25 16:44:41 +00:00 by jared · 5 comments
Owner

Problem

Metrics/ClassLength fires on files whose real fault is the same method skeleton written many times with one literal changed. No Sdlc cop names that shape. The agent gets a generic length lesson and often splits the file the wrong way.
The pre-fix os-backlog file (git show 7c228093:plugins/os-backlog/lib/backlog/cli_command_wiring.rb, 279 lines) has this shape. The layered config does not catch it. Sdlc/Dry/DuplicatedStatementSequence does not fire, because it compares 2-statement windows by exact AST equality and the repeated skeleton differs by one string literal. Also, a def whose body is a method-level rescue has a body of one rescue node, so it has no 2-statement window at all (smell analysis section 7).
Jared's standard: "We want the solution to always be correct so we know the correction is warranted and we can reduce flags to zero. Otherwise we would always have unresolved violations. We need to know exactly what shape triggers that fix and that the fix is always correct for that shape, as well as exactly when the shape could trigger a false positive and why it is false so we can carve those out and avoid flagging them."

Reproduce

n/a: new cop. The red fixture is the pre-fix file above.

Approach

Build the cop from the smell analysis in docs/plans/class-length-family/smell-repeated-method-skeleton.md. The cop matches one shape. The shape, the fix, and each false-positive carve-out come from that file.
The cop name is Sdlc/Dry/RepeatedRescueClause (smell analysis section 8), file plugins/os-sdlc/lib/os_sdlc/cops/repeated_rescue_clause.rb. The exact matcher is section 9 (SCOPE, MEMBER M1-M6, SKELETON, GROUP, FIRE, MESSAGE). The carve-outs are section 13 (C1-C13). The fixtures are section 16 (red R1-R14, green G1-G21).
Carve-outs live in the cop matcher, by shape. Do not add config Exclude lists. Do not exclude test paths.
Jared reviewed the smell analysis on 2026-10-25 (section 17). Q1: accept that differing error lists become a wrapper argument. Q2: extend Sdlc/Structural/RescueSite to record rescue *errors sites before this cop ships. The ticket is not blocked.
Q2 reading: RescueSite records error constants (Detector::ErrorLineage::RescueIndex counts them). When a rescue list entry is *name and name is the rest parameter of the enclosing def W, record each constant argument at or after the rest-parameter position of every receiverless call to W in the same file, with form rescue and the handler sends. When that call forwards a rest parameter of its own enclosing def V (W(name, *errors, &)), apply the same rule to the calls to V. Real case on main: plugins/os-backlog/lib/backlog/cli/issue_commands.rb (tracker_command :75, issue_command :70).
The cop fires on main on day one: plugins/os-backlog/lib/backlog/cli/project_commands.rb has 4 members (ensure_labels :31, validate_issue_create :95, create_issue :102, list_issues :114; 2 different error lists, so fix form B). Fix them in this ticket with the section 10 fix.

Required work:

  1. Add the cop under the Sdlc/Dry family — check: present RepeatedRescueClause in plugins/os-sdlc/lib/os_sdlc/cops/*.rb
  2. Add the cop to the os-sdlc rubocop config.
  3. Add a red fixture that has the pre-fix os-backlog file shape.
  4. Add one green fixture for each carve-out in the smell analysis.
  5. First, before the cop: extend RescueSite to record the constants of rescue *errors sites (Q2 reading above), with tests in plugins/os-sdlc/tests/cops/rescue_site_test.rb — check: present rescue *errors in plugins/os-sdlc/tests/cops/rescue_site_test.rb
  6. Add the red fixtures R1-R14 and green fixtures G1-G21 of smell analysis section 16 in plugins/os-sdlc/tests/cops/repeated_rescue_clause_test.rb.
  7. Fix the 4 members in plugins/os-backlog/lib/backlog/cli/project_commands.rb with one private form-B wrapper. Ruling 2026-10-25 (AFK run): the wrapper lives on the parent class Commands and tracker_command shares it (spec follow-up 1), because the revert of line-golf edits made Metrics/ClassLength fire on ProjectCommands — check: present rescue *errors in plugins/os-backlog/lib/backlog/cli/commands.rb

Acceptance criteria

  1. Given the red fixture, when the cop runs, then it reports one offense on the rescue clause of each member (14 offenses for the 7c228093 file shape).
  2. Given each green fixture, when the cop runs, then it reports no offense.
  3. Given the cc-os plugins tree, when the cop runs, then it reports no false hit.
  4. Given the cc-os plugins tree after the project_commands.rb fix, when the cop runs, then it reports no offense.
  5. Given a def w(name, *errors) with rescue *errors => e and a call w("x", A, B::C) in the same file, when RescueSite runs, then it records A and B::C.

Owner sketch

class RuboCop::Cop::Sdlc::Dry::RepeatedRescueClause < RuboCop::Cop::Base
  def on_new_investigation
end

class RuboCop::Cop::Sdlc::Structural::RescueSite < RecordingCop
  def on_resbody(node)
end

RepeatedRescueClause (plugins/os-sdlc/lib/os_sdlc/cops/repeated_rescue_clause.rb) walks every class, module, class << self, and block body as one scope, so a per-node on_class hook is not enough. Register it in the require: list and as Sdlc/Dry/RepeatedRescueClause: Enabled: true in plugins/os-sdlc/.rubocop.yml. RescueSite (plugins/os-sdlc/lib/os_sdlc/cops/rescue_site.rb) gains the rescue *errors record in on_resbody. Commands (plugins/os-backlog/lib/backlog/cli/commands.rb) gains one private wrapper, shared by ProjectCommands and tracker_command.

Origin

  • Trigger: A Metrics/ClassLength offense in os-backlog that no Sdlc cop explained.
  • Improvised this session: none
  • Chain: Wrong split guidance ← no cop names the shape ← DESIGN (plugins/os-sdlc/lint-lessons/metrics-class-length.yml covers one cause only)
  • Root candidate: none found; searched Sdlc/Dry, RepeatedMethodSkeleton, skeleton, DuplicatedStatementSequence, ClassLength, cli_command_wiring, duplicat, cop
    VERDICT: CREATE
  • Where: docs/plans/class-length-family/smell-repeated-method-skeleton.md
  • Session: f4187368-5e7e-459d-80c6-ddf7620389cd
  • Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/f4187368-5e7e-459d-80c6-ddf7620389cd.jsonl
## Problem Metrics/ClassLength fires on files whose real fault is the same method skeleton written many times with one literal changed. No Sdlc cop names that shape. The agent gets a generic length lesson and often splits the file the wrong way. The pre-fix os-backlog file (`git show 7c228093:plugins/os-backlog/lib/backlog/cli_command_wiring.rb`, 279 lines) has this shape. The layered config does not catch it. Sdlc/Dry/DuplicatedStatementSequence does not fire, because it compares 2-statement windows by exact AST equality and the repeated skeleton differs by one string literal. Also, a def whose body is a method-level rescue has a body of one rescue node, so it has no 2-statement window at all (smell analysis section 7). Jared's standard: "We want the solution to always be correct so we know the correction is warranted and we can reduce flags to zero. Otherwise we would always have unresolved violations. We need to know exactly what shape triggers that fix and that the fix is always correct for that shape, as well as exactly when the shape could trigger a false positive and why it is false so we can carve those out and avoid flagging them." ## Reproduce n/a: new cop. The red fixture is the pre-fix file above. ## Approach Build the cop from the smell analysis in docs/plans/class-length-family/smell-repeated-method-skeleton.md. The cop matches one shape. The shape, the fix, and each false-positive carve-out come from that file. The cop name is `Sdlc/Dry/RepeatedRescueClause` (smell analysis section 8), file plugins/os-sdlc/lib/os_sdlc/cops/repeated_rescue_clause.rb. The exact matcher is section 9 (SCOPE, MEMBER M1-M6, SKELETON, GROUP, FIRE, MESSAGE). The carve-outs are section 13 (C1-C13). The fixtures are section 16 (red R1-R14, green G1-G21). Carve-outs live in the cop matcher, by shape. Do not add config Exclude lists. Do not exclude test paths. Jared reviewed the smell analysis on 2026-10-25 (section 17). Q1: accept that differing error lists become a wrapper argument. Q2: extend `Sdlc/Structural/RescueSite` to record `rescue *errors` sites before this cop ships. The ticket is not blocked. Q2 reading: RescueSite records error constants (Detector::ErrorLineage::RescueIndex counts them). When a rescue list entry is `*name` and `name` is the rest parameter of the enclosing def W, record each constant argument at or after the rest-parameter position of every receiverless call to W in the same file, with form `rescue` and the handler sends. When that call forwards a rest parameter of its own enclosing def V (`W(name, *errors, &)`), apply the same rule to the calls to V. Real case on main: plugins/os-backlog/lib/backlog/cli/issue_commands.rb (`tracker_command` :75, `issue_command` :70). The cop fires on main on day one: plugins/os-backlog/lib/backlog/cli/project_commands.rb has 4 members (`ensure_labels` :31, `validate_issue_create` :95, `create_issue` :102, `list_issues` :114; 2 different error lists, so fix form B). Fix them in this ticket with the section 10 fix. Required work: 1. Add the cop under the Sdlc/Dry family — check: present `RepeatedRescueClause` in `plugins/os-sdlc/lib/os_sdlc/cops/*.rb` 2. Add the cop to the os-sdlc rubocop config. 3. Add a red fixture that has the pre-fix os-backlog file shape. 4. Add one green fixture for each carve-out in the smell analysis. 5. First, before the cop: extend RescueSite to record the constants of `rescue *errors` sites (Q2 reading above), with tests in plugins/os-sdlc/tests/cops/rescue_site_test.rb — check: present `rescue *errors` in `plugins/os-sdlc/tests/cops/rescue_site_test.rb` 6. Add the red fixtures R1-R14 and green fixtures G1-G21 of smell analysis section 16 in plugins/os-sdlc/tests/cops/repeated_rescue_clause_test.rb. 7. Fix the 4 members in plugins/os-backlog/lib/backlog/cli/project_commands.rb with one private form-B wrapper. Ruling 2026-10-25 (AFK run): the wrapper lives on the parent class Commands and `tracker_command` shares it (spec follow-up 1), because the revert of line-golf edits made Metrics/ClassLength fire on ProjectCommands — check: present `rescue *errors` in `plugins/os-backlog/lib/backlog/cli/commands.rb` ## Acceptance criteria 1. Given the red fixture, when the cop runs, then it reports one offense on the rescue clause of each member (14 offenses for the 7c228093 file shape). 2. Given each green fixture, when the cop runs, then it reports no offense. 3. Given the cc-os plugins tree, when the cop runs, then it reports no false hit. 4. Given the cc-os plugins tree after the project_commands.rb fix, when the cop runs, then it reports no offense. 5. Given a def `w(name, *errors)` with `rescue *errors => e` and a call `w("x", A, B::C)` in the same file, when RescueSite runs, then it records `A` and `B::C`. ## Owner sketch ```ruby class RuboCop::Cop::Sdlc::Dry::RepeatedRescueClause < RuboCop::Cop::Base def on_new_investigation end class RuboCop::Cop::Sdlc::Structural::RescueSite < RecordingCop def on_resbody(node) end ``` RepeatedRescueClause (plugins/os-sdlc/lib/os_sdlc/cops/repeated_rescue_clause.rb) walks every class, module, `class << self`, and block body as one scope, so a per-node `on_class` hook is not enough. Register it in the `require:` list and as `Sdlc/Dry/RepeatedRescueClause: Enabled: true` in plugins/os-sdlc/.rubocop.yml. RescueSite (plugins/os-sdlc/lib/os_sdlc/cops/rescue_site.rb) gains the `rescue *errors` record in `on_resbody`. Commands (plugins/os-backlog/lib/backlog/cli/commands.rb) gains one private wrapper, shared by ProjectCommands and `tracker_command`. ## Origin - Trigger: A Metrics/ClassLength offense in os-backlog that no Sdlc cop explained. - Improvised this session: none - Chain: Wrong split guidance ← no cop names the shape ← DESIGN (plugins/os-sdlc/lint-lessons/metrics-class-length.yml covers one cause only) - Root candidate: none found; searched Sdlc/Dry, RepeatedMethodSkeleton, skeleton, DuplicatedStatementSequence, ClassLength, cli_command_wiring, duplicat, cop VERDICT: CREATE - Where: docs/plans/class-length-family/smell-repeated-method-skeleton.md - Session: f4187368-5e7e-459d-80c6-ddf7620389cd - Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/f4187368-5e7e-459d-80c6-ddf7620389cd.jsonl
Author
Owner

Blocked by Jared's review of docs/plans/class-length-family/smell-repeated-method-skeleton.md (no ticket number): the cop is built from that analysis.

Blocked by Jared's review of docs/plans/class-length-family/smell-repeated-method-skeleton.md (no ticket number): the cop is built from that analysis.
Author
Owner

Rulings (Jared, 2026-10-25, accepted prediction): Q1 accept: differing error lists become a wrapper argument; constants are evaluated before the body runs. Q2 extend RescueSite to record rescue *errors sites before this cop ships.
Independent blind check (sonnet, own finder from section 9 only): 2 groups, 18 methods, all TRUE, 0 FALSE; matches the analysis.
Spec points to define before TDD (they change matches): (1) the key's 'reference kind' means empty versus listed error list, not the list itself; (2) how the key treats non-string literal arguments such as fail_soft(:a) vs fail_soft(:b); (3) public and private members form one group, the wrapper is private.
Add fixtures: early return inside a wrapped body; public/private member mix.

Rulings (Jared, 2026-10-25, accepted prediction): Q1 accept: differing error lists become a wrapper argument; constants are evaluated before the body runs. Q2 extend RescueSite to record `rescue *errors` sites before this cop ships. Independent blind check (sonnet, own finder from section 9 only): 2 groups, 18 methods, all TRUE, 0 FALSE; matches the analysis. Spec points to define before TDD (they change matches): (1) the key's 'reference kind' means empty versus listed error list, not the list itself; (2) how the key treats non-string literal arguments such as fail_soft(:a) vs fail_soft(:b); (3) public and private members form one group, the wrapper is private. Add fixtures: early `return` inside a wrapped body; public/private member mix.
Author
Owner

Spec ready for TDD. Section 9 now defines the three points (list kind only in the key; non-string literals stay in the key; public/private/protected form one group, wrapper private). Section 17 records Q1 (accept) and Q2 (extend RescueSite first). New fixtures R13, R14, G21. Commit f81efc62.

Spec ready for TDD. Section 9 now defines the three points (list kind only in the key; non-string literals stay in the key; public/private/protected form one group, wrapper private). Section 17 records Q1 (accept) and Q2 (extend RescueSite first). New fixtures R13, R14, G21. Commit f81efc62.
Author
Owner

Work started on branch ticket-1321 (os-sdlc implement, AFK run approved 2026-10-25).

Work started on branch ticket-1321 (os-sdlc implement, AFK run approved 2026-10-25).
Author
Owner

Resolution

Done: Cop Sdlc/Dry/RepeatedRescueClause (Member, Handler, Group) flags 3+ methods in one scope whose rescue skeleton differs only in literals or error lists and names the wrapper to extract; RescueSite records rescue *errors constants with one forwarding level (Q2). Fixed the 4 os-backlog hits with Commands#fail_soft_in shared with tracker_command (required-work item 7 updated on the ticket: wrapper on Commands, not ProjectCommands, after reverting line-golf edits made ClassLength fire). os-sdlc-lint exits 3 with usage when --root or --base is missing.

Evidence: main cb92a1e5. os-sdlc suite 2592 runs, 0 failures; os-backlog 327 runs, 0 failures; cop over plugins/ 2510 files 0 offenses, 18 true hits on pre-fix files; rubocop clean on os-sdlc files. Implementation 162 escalated (75, unexpected_pass); hand-finished with one design audit, its 3 must-fix findings applied. Known lint residue: one Sdlc/Structural/UnusedPrivateMethod false positive on Commands#tracker_command (cop reads one file; same mechanism already on main in that file) recorded on #1325.

Follow-ups: #1342 (verifier brief lacks cross-target diff, skeptic CREATE); UnusedPrivateMethod false positive recorded on #1325 (open, zero-FP audit). Dropped: itblock scope (no Ruby 3.4 it-blocks in repo); RescueSource vs CopHelpers duplication (test support, CRC pass will rule).

## Resolution **Done:** Cop Sdlc/Dry/RepeatedRescueClause (Member, Handler, Group) flags 3+ methods in one scope whose rescue skeleton differs only in literals or error lists and names the wrapper to extract; RescueSite records rescue *errors constants with one forwarding level (Q2). Fixed the 4 os-backlog hits with Commands#fail_soft_in shared with tracker_command (required-work item 7 updated on the ticket: wrapper on Commands, not ProjectCommands, after reverting line-golf edits made ClassLength fire). os-sdlc-lint exits 3 with usage when --root or --base is missing. **Evidence:** main cb92a1e5. os-sdlc suite 2592 runs, 0 failures; os-backlog 327 runs, 0 failures; cop over plugins/ 2510 files 0 offenses, 18 true hits on pre-fix files; rubocop clean on os-sdlc files. Implementation 162 escalated (75, unexpected_pass); hand-finished with one design audit, its 3 must-fix findings applied. Known lint residue: one Sdlc/Structural/UnusedPrivateMethod false positive on Commands#tracker_command (cop reads one file; same mechanism already on main in that file) recorded on #1325. **Follow-ups:** #1342 (verifier brief lacks cross-target diff, skeptic CREATE); UnusedPrivateMethod false positive recorded on #1325 (open, zero-FP audit). Dropped: itblock scope (no Ruby 3.4 it-blocks in repo); RescueSource vs CopHelpers duplication (test support, CRC pass will rule).
jared closed this issue 2026-10-25 20:38:13 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
jared/cc-os#1321
No description provided.