Add Sdlc/Dry cop for repeated collaborator construction

jared commented

2026-10-25 16:44:42 +00:00

Owner

Problem

Metrics/ClassLength fires on files whose real fault is the same collaborator built again and again in many methods. 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. The construction svc = Backlog::Issues.new(runner: @runner, repo_root: Dir.pwd, client: @client) is identical in each method, but the statement after it differs, and a def whose body is a method-level rescue has a body of one rescue node, so it has no 2-statement window.
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-collaborator-construction.md. The cop matches one shape. The shape, the fix, and each false-positive carve-out come from that file.
The exact matcher is section 9 (conditions 1-10 and spec points a-f). The fix is section 10: a private build_<snake_case of the last constant segment> method, no memoization. The carve-outs are section 13. The fixtures are section 16 (red 1-13 and R-a to R-h; green 1-32 and G-a to G-c). docs/plans/class-length-family/smell-test-subject-construction.md supports ruling R1 for test classes.
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. R1: the cop fires only when the class builds the constant in one form; every construction of that constant in the class must be the same expression (section 9, condition 9). R2: accept the miss when many equal sites sit beside one outlier construction. Fix name: build_<class>; add a role noun only if the method is memoized. The ticket is not blocked.
The cop fires on main when it ships. The probe (section 15, main e7e8ecd4) found 29 groups on main: 8 production groups (os-adr/lib/adr/migration.rb, os-me/lib/os_me/cli.rb, os-sdlc detector/measures.rb, dispatch/brief/facts.rb, gate/commands.rb, map/check/node_rules.rb, map/check.rb, project/root_search.rb) and 21 test groups. The 30th group is the 7c228093 file, which is not on main. Fix every group the cop reports on main in this ticket, with the section 10 fix and its step 2 name check. When the name check finds a collision, do not apply the fix; file a false-positive ticket with /os-backlog:capture.

Required work:

Add the cop under the Sdlc/Dry family — check: present RepeatedCollaboratorConstruction in plugins/os-sdlc/lib/os_sdlc/cops/*.rb
Add the cop to the os-sdlc rubocop config.
Add a red fixture that has the pre-fix os-backlog file shape.
Add one green fixture for each carve-out in the smell analysis.
Add the section 16 red and green fixtures in plugins/os-sdlc/tests/cops/repeated_collaborator_construction_test.rb.
Fix every group that the cop reports on main (about 29) with the section 10 fix.

Acceptance criteria

Given the red fixture, when the cop runs, then it reports one offense on each construction site of the group (12 offenses for the 7c228093 file shape).
Given each green fixture, when the cop runs, then it reports no offense.
Given the cc-os plugins tree, when the cop runs, then it reports no false hit.
Given the cc-os plugins tree after the fixes, when the cop runs, then it reports no offense, except sites filed as false-positive tickets by the name check.

Owner sketch

class RuboCop::Cop::Sdlc::Dry::RepeatedCollaboratorConstruction < RuboCop::Cop::Base
  def on_class(node)
  def on_module(node)
end

File: plugins/os-sdlc/lib/os_sdlc/cops/repeated_collaborator_construction.rb. The group scope is a class or a module body (condition 6), so the cop needs both hooks. Register it in the require: list and as Sdlc/Dry/RepeatedCollaboratorConstruction: Enabled: true in plugins/os-sdlc/.rubocop.yml, with no Exclude. Each fixed class or module gains one private build_<name> method.

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 RepeatedCollaboratorConstruction, collaborator construction, Sdlc/Dry, DuplicatedStatementSequence, ClassLength, cli_command_wiring, skeleton
VERDICT: CREATE
Where: docs/plans/class-length-family/smell-repeated-collaborator-construction.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 collaborator built again and again in many methods. 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. The construction `svc = Backlog::Issues.new(runner: @runner, repo_root: Dir.pwd, client: @client)` is identical in each method, but the statement after it differs, and a def whose body is a method-level rescue has a body of one rescue node, so it has no 2-statement window. 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-collaborator-construction.md. The cop matches one shape. The shape, the fix, and each false-positive carve-out come from that file. The exact matcher is section 9 (conditions 1-10 and spec points a-f). The fix is section 10: a private `build_<snake_case of the last constant segment>` method, no memoization. The carve-outs are section 13. The fixtures are section 16 (red 1-13 and R-a to R-h; green 1-32 and G-a to G-c). docs/plans/class-length-family/smell-test-subject-construction.md supports ruling R1 for test classes. 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. R1: the cop fires only when the class builds the constant in one form; every construction of that constant in the class must be the same expression (section 9, condition 9). R2: accept the miss when many equal sites sit beside one outlier construction. Fix name: `build_<class>`; add a role noun only if the method is memoized. The ticket is not blocked. The cop fires on main when it ships. The probe (section 15, main e7e8ecd4) found 29 groups on main: 8 production groups (os-adr/lib/adr/migration.rb, os-me/lib/os_me/cli.rb, os-sdlc detector/measures.rb, dispatch/brief/facts.rb, gate/commands.rb, map/check/node_rules.rb, map/check.rb, project/root_search.rb) and 21 test groups. The 30th group is the 7c228093 file, which is not on main. Fix every group the cop reports on main in this ticket, with the section 10 fix and its step 2 name check. When the name check finds a collision, do not apply the fix; file a false-positive ticket with /os-backlog:capture. Required work: 1. Add the cop under the Sdlc/Dry family — check: present `RepeatedCollaboratorConstruction` 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. Add the section 16 red and green fixtures in plugins/os-sdlc/tests/cops/repeated_collaborator_construction_test.rb. 6. Fix every group that the cop reports on main (about 29) with the section 10 fix. ## Acceptance criteria 1. Given the red fixture, when the cop runs, then it reports one offense on each construction site of the group (12 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 fixes, when the cop runs, then it reports no offense, except sites filed as false-positive tickets by the name check. ## Owner sketch ```ruby class RuboCop::Cop::Sdlc::Dry::RepeatedCollaboratorConstruction < RuboCop::Cop::Base def on_class(node) def on_module(node) end ``` File: plugins/os-sdlc/lib/os_sdlc/cops/repeated_collaborator_construction.rb. The group scope is a `class` or a `module` body (condition 6), so the cop needs both hooks. Register it in the `require:` list and as `Sdlc/Dry/RepeatedCollaboratorConstruction: Enabled: true` in plugins/os-sdlc/.rubocop.yml, with no Exclude. Each fixed class or module gains one private `build_<name>` method. ## 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 RepeatedCollaboratorConstruction, collaborator construction, Sdlc/Dry, DuplicatedStatementSequence, ClassLength, cli_command_wiring, skeleton VERDICT: CREATE - Where: docs/plans/class-length-family/smell-repeated-collaborator-construction.md - Session: f4187368-5e7e-459d-80c6-ddf7620389cd - Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/f4187368-5e7e-459d-80c6-ddf7620389cd.jsonl

jared added the

labels

2026-10-25 16:44:42 +00:00

jared commented

2026-10-25 16:44:51 +00:00

Author

Owner

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

jared referenced this issue

2026-10-25 16:44:52 +00:00

Write lessons for the ClassLength family cops #1323

jared commented

2026-10-25 16:59:18 +00:00

Author

Owner

Ruling (Jared, 2026-10-25), naming: the cop prescribes build_<snake_case class noun> because its fix does not memoize and each call builds a new object. The lesson adds: if all three memoization checks in the smell analysis pass and the agent memoizes, rename the method to the noun for its role in this class (for example issue_service). A verb name means a new object on each call; a noun name means one object for each holder. Analysis: docs/plans/class-length-family/smell-repeated-collaborator-construction.md (a986d591).

Ruling (Jared, 2026-10-25), naming: the cop prescribes `build_<snake_case class noun>` because its fix does not memoize and each call builds a new object. The lesson adds: if all three memoization checks in the smell analysis pass and the agent memoizes, rename the method to the noun for its role in this class (for example `issue_service`). A verb name means a new object on each call; a noun name means one object for each holder. Analysis: docs/plans/class-length-family/smell-repeated-collaborator-construction.md (a986d591).

jared commented

2026-10-25 17:02:17 +00:00

Author

Owner

Independent blind check (sonnet, own finder from section 9 only): 33 groups, 119 sites, all TRUE, 0 FALSE. The analysis probe found 39 groups, 139 sites. The two finders disagree on 7 groups, so section 9 is ambiguous. The disputed groups are mostly tests that build their own subject under test: tier2/judge_test.rb (Judge), dispatch_budget_test.rb (DispatchBudget), loop_ledger_test.rb (LoopLedger), gated_run/ledger_test.rb (Ledger), residual_worklist_test.rb (ResidualWorklist), boot_unchanged_check_test.rb, plus ProjectScenario counts. Causes to pin down: nested Const.new in args, receiverless calls such as ledger_path counting as 'reads self' (Kernel calls like format/Integer too), array literals in args.
Other spec points: scope of 'C defines no def named N' (def self.N, attr_*, alias, define_method, included modules); overlapping sites; optarg-default sites; private def / protected forms; A::Report vs B::Report share N.
Open ruling for Jared: is extracting the subject-under-test construction in a test warranted? See session f4187368.

Independent blind check (sonnet, own finder from section 9 only): 33 groups, 119 sites, all TRUE, 0 FALSE. The analysis probe found 39 groups, 139 sites. The two finders disagree on 7 groups, so section 9 is ambiguous. The disputed groups are mostly tests that build their own subject under test: tier2/judge_test.rb (Judge), dispatch_budget_test.rb (DispatchBudget), loop_ledger_test.rb (LoopLedger), gated_run/ledger_test.rb (Ledger), residual_worklist_test.rb (ResidualWorklist), boot_unchanged_check_test.rb, plus ProjectScenario counts. Causes to pin down: nested Const.new in args, receiverless calls such as ledger_path counting as 'reads self' (Kernel calls like format/Integer too), array literals in args. Other spec points: scope of 'C defines no def named N' (def self.N, attr_*, alias, define_method, included modules); overlapping sites; optarg-default sites; private def / protected forms; A::Report vs B::Report share N. Open ruling for Jared: is extracting the subject-under-test construction in a test warranted? See session f4187368.

jared commented

2026-10-25 17:24:34 +00:00

Author

Owner

Rulings (Jared, 2026-10-25, accepted prediction): R1 accept: the single-construction-form rule replaces condition 9: fire only if every .new in the class whose constant maps to the prescribed name is the same expression as the group (AST only, no path or file name). Probe v5: 30 groups, 109 sites kept; 9 test groups dropped, including the 7 disputed; 0 production groups lost. R2 accept: many equal sites plus one outlier (Ledger 6+1, RecordingRunner 6+3) is an accepted miss. Correction: Metrics/ClassLength excludes tests today (plugins/os-sdlc/.rubocop.yml:73-78), so these misses have no backstop until #1317 removes that exclusion. Before TDD: a blind independent check of the new rule over kept and dropped groups. Analysis: docs/plans/class-length-family/smell-test-subject-construction.md (2e2c4ee0).

Rulings (Jared, 2026-10-25, accepted prediction): R1 accept: the single-construction-form rule replaces condition 9: fire only if every .new in the class whose constant maps to the prescribed name is the same expression as the group (AST only, no path or file name). Probe v5: 30 groups, 109 sites kept; 9 test groups dropped, including the 7 disputed; 0 production groups lost. R2 accept: many equal sites plus one outlier (Ledger 6+1, RecordingRunner 6+3) is an accepted miss. Correction: Metrics/ClassLength excludes tests today (plugins/os-sdlc/.rubocop.yml:73-78), so these misses have no backstop until #1317 removes that exclusion. Before TDD: a blind independent check of the new rule over kept and dropped groups. Analysis: docs/plans/class-length-family/smell-test-subject-construction.md (2e2c4ee0).

jared commented

2026-10-25 17:30:55 +00:00

Author

Owner

Spec ready for TDD. Blind check of the single-construction-form rule (sonnet, own rubocop-ast finder from sections 9-10 only): same 30 groups / 109 sites as the analysis; 30 TRUE, 0 FALSE; 9 of 9 dropped groups CORRECT DROP (Catalog 5+1 and Ledger 6+1 borderline, covered by R2). Section 9 now pins rubocop-ast for Ruby 3.4, adds itblock/numblock, puts the optarg rule in condition 5, makes two or more def N a no-fire, disqualifies any receiverless module_function, defines the underscore name rule, and separates ::Foo from Foo. New fixtures R-h, G-a, G-b, G-c. Commit f81efc62.

Spec ready for TDD. Blind check of the single-construction-form rule (sonnet, own rubocop-ast finder from sections 9-10 only): same 30 groups / 109 sites as the analysis; 30 TRUE, 0 FALSE; 9 of 9 dropped groups CORRECT DROP (Catalog 5+1 and Ledger 6+1 borderline, covered by R2). Section 9 now pins rubocop-ast for Ruby 3.4, adds itblock/numblock, puts the optarg rule in condition 5, makes two or more def N a no-fire, disqualifies any receiverless module_function, defines the underscore name rule, and separates ::Foo from Foo. New fixtures R-h, G-a, G-b, G-c. Commit f81efc62.

jared referenced this issue from a commit

2026-10-25 17:31:01 +00:00

docs(plans): smell analyses for RepeatedRescueClause and repeated collaborator construction (#1321, #1322)

jared referenced this issue from a commit

2026-10-25 17:31:01 +00:00

docs(plans): smell analysis for tests that build their subject (#1322)

jared referenced this issue from a commit

2026-10-25 17:31:01 +00:00

docs(plans): collaborator-construction spec adopts single-construction-form rule (#1322 R1, R2)

jared referenced this issue from a commit

2026-10-25 17:31:01 +00:00

docs(plans): close spec ambiguities found by blind checks; record Q1, Q2 rulings (#1321, #1322)

jared referenced this issue

2026-10-25 19:02:11 +00:00

Write lessons for the ClassLength family cops #1323

jared referenced this issue

2026-10-25 19:02:12 +00:00

jared referenced this issue

2026-10-25 19:02:13 +00:00

Audit every Sdlc cop against the zero-false-positive standard #1325

jared added the

ready-for-agent

plugin/os-backlog

plugin/os-adr

labels

2026-10-25 19:03:50 +00:00

jared commented

2026-10-25 20:37:12 +00:00

Author

Owner

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

jared referenced this issue

2026-10-25 21:09:25 +00:00

Find why unchecked_required_work did not stop unchecked required-work items #1340

jared referenced this issue

2026-10-25 21:09:47 +00:00

Make the new-offense gate ignore metric scores in offense messages #1343

jared referenced this issue

2026-10-25 21:09:48 +00:00

Add an end-to-end test for OsMe::CLI::ScoreRequest #1344

jared commented

2026-10-25 21:10:00 +00:00

Author

Owner

Resolution

Done: Cop Sdlc/Dry/RepeatedCollaboratorConstruction (Body, Construction, Group, Wiring) flags a collaborator built by one closed expression at 3+ sites in a class and prescribes a private build_ builder (R1, R2). Fixed all 34 groups on main: 8 production, 26 test, across os-sdlc, os-adr, os-me and the ticket-takedown eval. OsMe::CLI and Adr::Migration hit ClassLength after the fix and each gave up one job (CLI::ScoreRequest, Migration::Sections) instead of line-golf.

Evidence: main 9c348bb3 (caf79098, 9c348bb3). Cop over plugins/: 2518 files, 0 offenses; 12 offenses on the 7c228093 file (AC1). Suites: os-sdlc 2647, os-me 392, os-adr 110, ticket-takedown 615 runs, all 0 failures. New-offense gate: only Migration#convert AbcSize, a pre-existing offense whose score dropped (gate keys on message text, #1343). Implementation 163 failed at contract-auditor (WORK-MISSING R6); hand-finished with two design audits, no must-fix left.

Follow-ups: #1343 (new-offense gate counts a lower metric score as new, skeptic CREATE); #1344 (no end-to-end test for ScoreRequest, skeptic CREATE); occurrence on #1340 (item 6 had no typed check, skeptic OCCURRENCE). Dropped: Migration#convert AbcSize split (pre-existing, improved here); Sections#confidence recomputation (cheap, memoized).

## Resolution **Done:** Cop Sdlc/Dry/RepeatedCollaboratorConstruction (Body, Construction, Group, Wiring) flags a collaborator built by one closed expression at 3+ sites in a class and prescribes a private build_<class> builder (R1, R2). Fixed all 34 groups on main: 8 production, 26 test, across os-sdlc, os-adr, os-me and the ticket-takedown eval. OsMe::CLI and Adr::Migration hit ClassLength after the fix and each gave up one job (CLI::ScoreRequest, Migration::Sections) instead of line-golf. **Evidence:** main 9c348bb3 (caf79098, 9c348bb3). Cop over plugins/: 2518 files, 0 offenses; 12 offenses on the 7c228093 file (AC1). Suites: os-sdlc 2647, os-me 392, os-adr 110, ticket-takedown 615 runs, all 0 failures. New-offense gate: only Migration#convert AbcSize, a pre-existing offense whose score dropped (gate keys on message text, #1343). Implementation 163 failed at contract-auditor (WORK-MISSING R6); hand-finished with two design audits, no must-fix left. **Follow-ups:** #1343 (new-offense gate counts a lower metric score as new, skeptic CREATE); #1344 (no end-to-end test for ScoreRequest, skeptic CREATE); occurrence on #1340 (item 6 had no typed check, skeptic OCCURRENCE). Dropped: Migration#convert AbcSize split (pre-existing, improved here); Sections#confidence recomputation (cheap, memoized).

jared closed this issue

2026-10-25 21:10:00 +00:00

jared referenced this issue from a commit

2026-10-25 21:44:55 +00:00

feat(os-sdlc): add Sdlc/Dry/RepeatedCollaboratorConstruction cop (#1322)

jared referenced this issue from a commit

2026-10-25 21:44:55 +00:00

refactor: fix the 34 repeated collaborator constructions on main (#1322)

jared referenced this issue from a commit

2026-10-25 21:44:55 +00:00

docs(os-sdlc): lessons for the ClassLength family cops (#1323)