jared/cc-os

Watch

Fork 0

Sdlc/Structural/ForwardingClass: skip role players and third-party adapters #1360

New issue

Closed

opened 2026-10-25 21:23:17 +00:00 by jared · 2 comments

jared commented

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

Owner

Problem

Sdlc/Structural/ForwardingClass is a recording cop. It records a class that only builds a constant and forwards to it, and its message prescribes "give the constant a constructor that takes what this class takes, and call it where this class is called". OsSdlc::Detector::Holders reads the records, and bin/lint-sweep shows each record as an offense. Both records in plugins/ are classes where that fix is wrong.

Found by the #1325 zero-false-positive audit (docs/plans/class-length-family/sdlc-cop-audit.md). Shapes measured on main 4a0fc41d, 2026-10-26, over every plugins/**/*.rb file (2442 files, 2 records):

shape	all plugins	#1317 sweep	verdict	example
public def has a parameter its body never reads (role player: the caller sets the signature)	1	1	false positive: callers send the role message through an injected duck type, so they cannot call the constant instead	`plugins/os-aidd-lint/tests/support/fake_invoker.rb:3` `def call(_prompt) = Result.new(output: @output, error: @error)`, injected as `invoker:` at `plugins/os-aidd-lint/tests/tier2/judge_test.rb:25`
forwarded constant comes from a library the file requires	1	0	false positive: a library constant cannot take a new constructor	`plugins/os-vault/eval/fixture/project/lib/reportgen/charts.rb:7` `require 'chartkit'` then `Chartkit::SVG.render(series)`
forwarder of a project constant, every parameter read	0	0	true positive	test fixtures only: `MIDDLEMAN_SOURCE`, `DIRECT_NEW_SOURCE` in `plugins/os-sdlc/tests/cops/forwarding_class_test.rb`; `wrapper.rb` in `plugins/os-sdlc/tests/detector/holders_bin_test.rb:38`

The #1317 sweep (plugins/os-sdlc/bin/lint-sweep plugins/<p> for os-backlog, os, os-adr, os-aidd-lint, os-me) shows 1 offense: the FakeInvoker record.

Approach

Fix by shape in the cop matcher. Never use a config Exclude list or a test-path exclusion.

Rule 1 (role player): skip a class when any public def has a parameter that its body never reads. Rule 2 (library adapter): skip a class when its file calls require (not require_relative) with a string whose first path segment, with _ and - removed, equals the first segment of the forwarded constant without case (chartkit matches Chartkit, svg_kit matches SVGKit, json matches JSON). Read the first segment after a leading :: (cbase).

Measured effect (scratch monkey-patch of both rules on main 4a0fc41d): records over plugins/**/*.rb go from 2 to 0; lint-sweep plugins/os-aidd-lint goes from 1 to 0. The fixtures MIDDLEMAN_SOURCE and DIRECT_NEW_SOURCE, a role player that reads its parameter, and the Charts shape with require_relative "chartkit" all still fire. os-sdlc has no record, so bin/detector holders output does not change.

Accepted misses, as in ruling R2 (docs/plans/class-length-family/HANDOFF.md): the cop gives up recall to keep every record true. A forwarder that ignores a parameter (Lint/UnusedMethodArgument still flags that parameter), or that forwards to a project constant whose library name the file also requires through the load path (require "reportgen" then Reportgen::X), is no longer recorded. No such site exists in plugins/ today.

Required work:

In plugins/os-sdlc/lib/os_sdlc/cops/structural/forwarding_class/class_match.rb, make forwarding? false when any public def has a named parameter (positional, optional, keyword, rest, or block) that no lvar in its body reads. An anonymous *, **, &, or ... does not count. Add a must-pass fixture with def call(_prompt) that returns Result.new(output: @output, error: @error) to plugins/os-sdlc/tests/cops/forwarding_class_test.rb.
check: present def call(_prompt) in plugins/os-sdlc/tests/cops/forwarding_class_test.rb
In plugins/os-sdlc/lib/os_sdlc/cops/structural/forwarding_class.rb or a new forwarding_class/ helper, skip the class when the file requires a library whose first path segment matches the first segment of the forwarded constant by rule 2. Add must-pass fixtures (require "chartkit", then Chartkit::SVG.render(series); require "svg_kit", then SVGKit.render(series)) and a must-fire fixture (the same class with require_relative "chartkit") to plugins/os-sdlc/tests/cops/forwarding_class_test.rb.
check: present Chartkit::SVG.render(series) in plugins/os-sdlc/tests/cops/forwarding_class_test.rb
In the lesson plugins/os-sdlc/lint-lessons/sdlc-structural-forwarding-class.yml, state in signal that the class reads every parameter of its public methods and that the target is not a constant of a required library.
check: present required library in plugins/os-sdlc/lint-lessons/sdlc-structural-forwarding-class.yml
In docs/plans/class-length-family/sdlc-cop-audit.md, change the ForwardingClass row (:47) from fails to passes and give its current path plugins/os-sdlc/lib/os_sdlc/cops/structural/forwarding_class.rb.
check: present cops/structural/forwarding_class.rb in docs/plans/class-length-family/sdlc-cop-audit.md
Add one History row for this change to docs/implementation-status/os-sdlc.md, and run bin/refresh-plugins after the lesson edit.
check: present #1360 in docs/implementation-status/os-sdlc.md

Acceptance criteria

Given a class with initialize(output: nil, error: nil) storing both and def call(_prompt) returning Result.new(output: @output, error: @error), when the cop runs, then it records nothing.
Given the class def build(series:) = Chartkit::SVG.render(series), when the cop runs once on a file with require 'chartkit' and once on the same file with require_relative "chartkit", then it records nothing for the first and records Chartkit::SVG for the second (one contrast test: the kind of require decides).
Given the existing fixtures MIDDLEMAN_SOURCE and DIRECT_NEW_SOURCE, when the cop runs, then it records Thing as today. The existing tests in plugins/os-sdlc/tests/cops/forwarding_class_test.rb and plugins/os-sdlc/tests/detector/holders_bin_test.rb prove this unchanged; do not add a new test for it, because a test of existing behavior cannot be red in the red-first round.
Given rubocop with only Sdlc/Structural/ForwardingClass over every plugins/**/*.rb file, when it runs, then it reports 0 offenses (2 on 4a0fc41d), and plugins/os-sdlc/bin/lint-sweep plugins/os-aidd-lint reports no ForwardingClass offense.

Owner sketch

RuboCop::Cop::Sdlc::Structural::ForwardingClass::ClassMatch (existing) owns rule 1: it already holds the public defs and decides forwarding?.
RuboCop::Cop::Sdlc::Structural::ForwardingClass (existing) owns rule 2: only the cop has processed_source, so it reads the file's require calls and compares them with ClassMatch#constant. No new object is required; a small forwarding_class/ helper is acceptable if the cop grows past its size cops.

In scope / out of scope

In scope: the two skip rules, their fixtures, the lesson signal, and the audit row.
Out of scope (cut on 2026-10-26 as YAGNI, 0 sites in plugins/): a role player that reads all its parameters (needs cross-file knowledge of who injects it); a library constant loaded through Bundler.require or Zeitwerk without a require in the file; a project-wide constant index to tell project constants from library constants.
Out of scope: OsSdlc::Detector::Holders and other detector code; their output does not change.

Blocking edges

None. One implement run.

Origin

Trigger: #1325 zero-false-positive audit of the Sdlc cops
Improvised this session: none
Chain: false positive on valid code ← cop matcher tests a proxy shape, not the real condition ← DESIGN (cop source named in Where:)
Root candidate: none found; searched the cop name, its mechanism terms, and false positive
Where: plugins/os-sdlc/lib/os_sdlc/cops/structural/forwarding_class.rb (forwarding_class/class_match.rb:18-52, constant_chain.rb:22-50)
Session: eb98cdcd-6783-46a0-b7df-cf18a8fe2a56
Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os--claude-worktrees-ticket-1325/eb98cdcd-6783-46a0-b7df-cf18a8fe2a56.jsonl

VERDICT: CREATE

## Problem `Sdlc/Structural/ForwardingClass` is a recording cop. It records a class that only builds a constant and forwards to it, and its message prescribes "give the constant a constructor that takes what this class takes, and call it where this class is called". `OsSdlc::Detector::Holders` reads the records, and `bin/lint-sweep` shows each record as an offense. Both records in `plugins/` are classes where that fix is wrong. Found by the #1325 zero-false-positive audit (docs/plans/class-length-family/sdlc-cop-audit.md). Shapes measured on main 4a0fc41d, 2026-10-26, over every `plugins/**/*.rb` file (2442 files, 2 records): | shape | all plugins | #1317 sweep | verdict | example | |---|---|---|---|---| | public def has a parameter its body never reads (role player: the caller sets the signature) | 1 | 1 | false positive: callers send the role message through an injected duck type, so they cannot call the constant instead | `plugins/os-aidd-lint/tests/support/fake_invoker.rb:3` `def call(_prompt) = Result.new(output: @output, error: @error)`, injected as `invoker:` at `plugins/os-aidd-lint/tests/tier2/judge_test.rb:25` | | forwarded constant comes from a library the file requires | 1 | 0 | false positive: a library constant cannot take a new constructor | `plugins/os-vault/eval/fixture/project/lib/reportgen/charts.rb:7` `require 'chartkit'` then `Chartkit::SVG.render(series)` | | forwarder of a project constant, every parameter read | 0 | 0 | true positive | test fixtures only: `MIDDLEMAN_SOURCE`, `DIRECT_NEW_SOURCE` in `plugins/os-sdlc/tests/cops/forwarding_class_test.rb`; `wrapper.rb` in `plugins/os-sdlc/tests/detector/holders_bin_test.rb:38` | The #1317 sweep (`plugins/os-sdlc/bin/lint-sweep plugins/<p>` for os-backlog, os, os-adr, os-aidd-lint, os-me) shows 1 offense: the FakeInvoker record. ## Approach Fix by shape in the cop matcher. Never use a config Exclude list or a test-path exclusion. Rule 1 (role player): skip a class when any public def has a parameter that its body never reads. Rule 2 (library adapter): skip a class when its file calls `require` (not `require_relative`) with a string whose first path segment, with `_` and `-` removed, equals the first segment of the forwarded constant without case (`chartkit` matches `Chartkit`, `svg_kit` matches `SVGKit`, `json` matches `JSON`). Read the first segment after a leading `::` (cbase). Measured effect (scratch monkey-patch of both rules on main 4a0fc41d): records over `plugins/**/*.rb` go from 2 to 0; `lint-sweep plugins/os-aidd-lint` goes from 1 to 0. The fixtures `MIDDLEMAN_SOURCE` and `DIRECT_NEW_SOURCE`, a role player that reads its parameter, and the Charts shape with `require_relative "chartkit"` all still fire. os-sdlc has no record, so `bin/detector holders` output does not change. Accepted misses, as in ruling R2 (docs/plans/class-length-family/HANDOFF.md): the cop gives up recall to keep every record true. A forwarder that ignores a parameter (`Lint/UnusedMethodArgument` still flags that parameter), or that forwards to a project constant whose library name the file also requires through the load path (`require "reportgen"` then `Reportgen::X`), is no longer recorded. No such site exists in `plugins/` today. Required work: 1. In `plugins/os-sdlc/lib/os_sdlc/cops/structural/forwarding_class/class_match.rb`, make `forwarding?` false when any public def has a named parameter (positional, optional, keyword, rest, or block) that no `lvar` in its body reads. An anonymous `*`, `**`, `&`, or `...` does not count. Add a must-pass fixture with `def call(_prompt)` that returns `Result.new(output: @output, error: @error)` to `plugins/os-sdlc/tests/cops/forwarding_class_test.rb`. check: present `def call(_prompt)` in `plugins/os-sdlc/tests/cops/forwarding_class_test.rb` 2. In `plugins/os-sdlc/lib/os_sdlc/cops/structural/forwarding_class.rb` or a new `forwarding_class/` helper, skip the class when the file `require`s a library whose first path segment matches the first segment of the forwarded constant by rule 2. Add must-pass fixtures (`require "chartkit"`, then `Chartkit::SVG.render(series)`; `require "svg_kit"`, then `SVGKit.render(series)`) and a must-fire fixture (the same class with `require_relative "chartkit"`) to `plugins/os-sdlc/tests/cops/forwarding_class_test.rb`. check: present `Chartkit::SVG.render(series)` in `plugins/os-sdlc/tests/cops/forwarding_class_test.rb` 3. In the lesson `plugins/os-sdlc/lint-lessons/sdlc-structural-forwarding-class.yml`, state in `signal` that the class reads every parameter of its public methods and that the target is not a constant of a required library. check: present `required library` in `plugins/os-sdlc/lint-lessons/sdlc-structural-forwarding-class.yml` 4. In `docs/plans/class-length-family/sdlc-cop-audit.md`, change the ForwardingClass row (`:47`) from `fails` to `passes` and give its current path `plugins/os-sdlc/lib/os_sdlc/cops/structural/forwarding_class.rb`. check: present `cops/structural/forwarding_class.rb` in `docs/plans/class-length-family/sdlc-cop-audit.md` 5. Add one History row for this change to `docs/implementation-status/os-sdlc.md`, and run `bin/refresh-plugins` after the lesson edit. check: present `#1360` in `docs/implementation-status/os-sdlc.md` ## Acceptance criteria 1. Given a class with `initialize(output: nil, error: nil)` storing both and `def call(_prompt)` returning `Result.new(output: @output, error: @error)`, when the cop runs, then it records nothing. 2. Given the class `def build(series:) = Chartkit::SVG.render(series)`, when the cop runs once on a file with `require 'chartkit'` and once on the same file with `require_relative "chartkit"`, then it records nothing for the first and records `Chartkit::SVG` for the second (one contrast test: the kind of require decides). 3. Given the existing fixtures `MIDDLEMAN_SOURCE` and `DIRECT_NEW_SOURCE`, when the cop runs, then it records `Thing` as today. The existing tests in `plugins/os-sdlc/tests/cops/forwarding_class_test.rb` and `plugins/os-sdlc/tests/detector/holders_bin_test.rb` prove this unchanged; do not add a new test for it, because a test of existing behavior cannot be red in the red-first round. 4. Given `rubocop` with only `Sdlc/Structural/ForwardingClass` over every `plugins/**/*.rb` file, when it runs, then it reports 0 offenses (2 on 4a0fc41d), and `plugins/os-sdlc/bin/lint-sweep plugins/os-aidd-lint` reports no ForwardingClass offense. ## Owner sketch - `RuboCop::Cop::Sdlc::Structural::ForwardingClass::ClassMatch` (existing) owns rule 1: it already holds the public defs and decides `forwarding?`. - `RuboCop::Cop::Sdlc::Structural::ForwardingClass` (existing) owns rule 2: only the cop has `processed_source`, so it reads the file's `require` calls and compares them with `ClassMatch#constant`. No new object is required; a small `forwarding_class/` helper is acceptable if the cop grows past its size cops. ## In scope / out of scope - In scope: the two skip rules, their fixtures, the lesson signal, and the audit row. - Out of scope (cut on 2026-10-26 as YAGNI, 0 sites in `plugins/`): a role player that reads all its parameters (needs cross-file knowledge of who injects it); a library constant loaded through `Bundler.require` or Zeitwerk without a `require` in the file; a project-wide constant index to tell project constants from library constants. - Out of scope: `OsSdlc::Detector::Holders` and other detector code; their output does not change. ## Blocking edges None. One implement run. ## Origin - Trigger: #1325 zero-false-positive audit of the Sdlc cops - Improvised this session: none - Chain: false positive on valid code ← cop matcher tests a proxy shape, not the real condition ← DESIGN (cop source named in Where:) - Root candidate: none found; searched the cop name, its mechanism terms, and false positive - Where: plugins/os-sdlc/lib/os_sdlc/cops/structural/forwarding_class.rb (forwarding_class/class_match.rb:18-52, constant_chain.rb:22-50) - Session: eb98cdcd-6783-46a0-b7df-cf18a8fe2a56 - Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os--claude-worktrees-ticket-1325/eb98cdcd-6783-46a0-b7df-cf18a8fe2a56.jsonl VERDICT: CREATE

jared added the

plugin/os-sdlc

filed-by/agent

category/code-change

labels

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

jared referenced this issue

2026-10-26 14:54:59 +00:00

os-sdlc: move Metrics/ClassLength and Metrics/ModuleLength to the sweep tier with zero Excludes, drain, promote #1317

jared added the

ready-for-agent

label

2026-10-26 15:20:29 +00:00

jared referenced this issue

2026-10-26 16:02:28 +00:00

diff-test gate accepts a round as red when any test is red but test-reviewer requires all new tests red #953

jared commented

2026-10-26 16:02:32 +00:00

Author

Owner

Work started on branch ticket-1360 via /os-sdlc:implement. Before the run, acceptance criterion 3 was folded into criterion 2 as one contrast test, and the preserved-behavior criterion now says the existing tests prove it (see #953 occurrence from #1370).

jared commented

2026-10-26 16:17:09 +00:00

Author

Owner

Resolution

Done: ForwardingClass skips a class whose public def ignores a named parameter (role player) or whose forwarded constant comes from a library the file requires; MethodMatch, RequireCall and RequiredLibraries own the decisions. Lesson, audit row and History row updated.

Evidence: Commit 356eb506 on main. Implementation 173 escalated (programmer found test defect sub->gsub at forwarding_class_test.rb:158); hand-finished: full suite 2686 tests green, rubocop clean on 9 files, ForwardingClass over plugins/**/*.rb reports 0 (was 2), design audit pass.

Follow-ups: none: optional audit findings accepted under ruling R2 (0 sites in plugins/): mlhs parameters not handled, a class mixing project and library forwards is skipped by its last constant, RequiredLibraries rescans per class.

## Resolution **Done:** ForwardingClass skips a class whose public def ignores a named parameter (role player) or whose forwarded constant comes from a library the file requires; MethodMatch, RequireCall and RequiredLibraries own the decisions. Lesson, audit row and History row updated. **Evidence:** Commit 356eb506 on main. Implementation 173 escalated (programmer found test defect sub->gsub at forwarding_class_test.rb:158); hand-finished: full suite 2686 tests green, rubocop clean on 9 files, ForwardingClass over plugins/**/*.rb reports 0 (was 2), design audit pass. **Follow-ups:** none: optional audit findings accepted under ruling R2 (0 sites in plugins/): mlhs parameters not handled, a class mixing project and library forwards is skipped by its last constant, RequiredLibraries rescans per class.

jared closed this issue

2026-10-26 16:17:10 +00:00

jared referenced this issue from a commit

2026-10-26 16:17:38 +00:00

feat(os-sdlc): ForwardingClass skips role players and required-library adapters (#1360)