jared/cc-os

Watch

Fork 0

Sdlc/Structural/UnderscoredParameter: report only a def that the same file calls #1370

New issue

Open

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

jared commented

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

Owner

Problem

Sdlc/Structural/UnderscoredParameter tells the agent: "Delete it and stop passing it, or read it." That fix is correct only when the agent can change every caller. The cop sees one file. When the caller is in another file, the arity is often set by a framework, a protocol that sibling implementers read, or a dispatch table, and deletion breaks the caller. The cop runs only in plugins/os-sdlc/.rubocop-sweep.yml.

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, with the sweep config over every file in plugins/ (25 offenses):

shape	all plugins	#1317 sweep	verdict	example
def or defs whose name the same file sends with no receiver or with `self`	4	1	true positive: every caller is in view, so the fix is always correct	`plugins/cc-architect/scripts/plugin_config/operations/audit.rb:121` private `check_installed(_name, key)`, called only at `:72`; sweep hit `plugins/os-backlog/tests/project_index_test.rb:54`
framework hook (no caller in the file)	1	0	false positive: Thor sets the arity	`plugins/os-sdlc/lib/os_sdlc/boot/onboarding.rb:17` `def self.handle_no_command_error(command, _has_namespace = $thor_runner)`
test double of a production protocol	6	0	false positive: the production caller sends the full list	`plugins/os-sdlc/tests/daemon/lsp_session_reopen_test.rb:31` `def request(_method, params)`
callback lambda in a test, given to production code	2	0	false positive: the production recorder protocol passes the value	`plugins/os-sdlc/tests/gate_test.rb:62` `->(_outcome, classification:) { ... }`
production protocol method or dispatch-table entry (def, `method(:x)`, or lambda) called from another file	12	0	false positive: siblings read the value (`Takedown::StepList#steps(run, history)` beside `ProbePlan#steps(_run, history)`), and the fix needs new caller machinery that the cop cannot see	`plugins/os-sdlc/lib/os_sdlc/crc/shares/run.rb:10` `"verdicts-merge" => ->(path, _run) { VerdictMerge.new(path) }`; `plugins/os-sdlc/lib/os_sdlc/dispatch/rerouting.rb:10`

The #1317 sweep of os-backlog, os, os-adr, os-aidd-lint, and os-me finds 1 offense (plugins/os-backlog/tests/project_index_test.rb:54). It is a true positive.

Approach

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

Rule: the cop checks a def or defs only when the same file holds a send of its method name with no receiver or with a self receiver. It no longer checks lambdas. The existing parameter test does not change: report an underscored parameter when the definition reads another parameter.

Measured effect (scratch simulation that patches the cop, main 4a0fc41d): offenses over plugins/ go from 25 to 4. The 4 kept are the true positives above (audit.rb:121, plugins/os-adr/eval-c/bin/check:226, project_index_test.rb:54, plugins/os-sdlc/lib/os_sdlc/implementation/lifecycle.rb:42). The 21 removed are all false positives. The #1317 sweep count stays 1.

The lesson's dispatch-table example (from fast-batch-1, docs/crc/os-sdlc/fast-batch-1/finding-c-underscored-parameter.md) no longer fires. That shape has a correct fix only when the table's caller is in this codebase, and no AST shape in one file tells that case apart from a protocol-forced lambda. The cop gives up that recall to keep every offense true, as in ruling R2 (docs/plans/class-length-family/HANDOFF.md).

Required work:

In plugins/os-sdlc/lib/os_sdlc/cops/structural/underscored_parameter.rb, check a def or defs only when processed_source.ast holds a send or csend of the same method name whose receiver is nil or self. Delete the on_block lambda check.
check: absent def on_block in plugins/os-sdlc/lib/os_sdlc/cops/structural/underscored_parameter.rb
In plugins/os-sdlc/tests/cops/underscored_parameter_test.rb, change the two lambda fixtures that expect an offense (test_reports_an_underscored_lambda_parameter_beside_read_ones, test_reports_an_underscored_parameter_inside_a_destructured_lambda_parameter) to tests that expect no offense, and add an in-file call to the fixtures of test_reports_an_underscored_def_parameter_beside_read_ones and test_reports_an_underscored_keyword_rest_parameter so that they still fire. Add must-pass fixtures: a Thor hook def self.handle_no_command_error(command, _has_namespace = nil) with no caller in the file, a test double def call(_repo, target) = target, and a method that the file calls only on another receiver (obj.call(repo, target)). Add must-fire fixtures: a private helper called with no receiver, a method called through self., a destructured def parameter def show(io, (action, _id)) called in the file, and a call that comes before the def.
check: present handle_no_command_error in plugins/os-sdlc/tests/cops/underscored_parameter_test.rb
In plugins/os-sdlc/lint-lessons/sdlc-structural-underscored-parameter.yml, state in signal that the same file calls the method (today it says "A method or lambda"). Rewrite decision and correction for that shape: the callers are in the file, so delete the parameter and the argument at each call; drop the "second constructor on the table entry" advice. Replace the dispatch-table example with a method that the same file calls (before: check_installed(_name, key) called as check_installed(name, key); after: check_installed(key)). Run bin/refresh-plugins after the edit.
check: present same file calls in plugins/os-sdlc/lint-lessons/sdlc-structural-underscored-parameter.yml
In docs/plans/class-length-family/sdlc-cop-audit.md, update the UnderscoredParameter row (line 63) and line 15: the cop path is plugins/os-sdlc/lib/os_sdlc/cops/structural/underscored_parameter.rb, and the cop now checks only a def that the same file calls.
check: absent os_sdlc/cops/underscored_parameter.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.
check: present #1370 in docs/implementation-status/os-sdlc.md

Acceptance criteria

Given def self.handle_no_command_error(command, _has_namespace = nil) that reads command and that no line of the file calls, when the cop runs, then it reports no offense.
Given a lambda ->(path, _run) { VerdictMerge.new(path) }, as a hash value, a call argument, or a local variable, when the cop runs, then it reports no offense.
Given a private def check_installed(_name, key) that reads key, and check_installed(name, key) in the same file, when the cop runs, then it reports _name.
Given def record(_result, text) that reads text, and self.record(result, text) in the same file, when the cop runs, then it reports _result.
Given bundle exec rubocop -c .rubocop-sweep.yml --only Sdlc/Structural/UnderscoredParameter ../../plugins run from plugins/os-sdlc, when it runs after the change, then it reports exactly 4 offenses, at audit.rb:121, check:226, project_index_test.rb:54, and lifecycle.rb:42.

Owner sketch

RuboCop::Cop::Sdlc::Structural::UnderscoredParameter (existing) owns the decision. It already partitions parameters and checks reads; it adds one question, "does this file call the method by name?", before it reports. No new object.

In scope / out of scope

In scope: the matcher rule, its tests, and the lesson.
Out of scope: fixing the 4 true positives. The sweep drain owns them (#1317 for the os-backlog hit). lifecycle.rb:42 needs plugins/os-sdlc/lib/os_sdlc/implementation/landing.rb:51 to stop passing root:.
Out of scope: moving the cop from .rubocop-sweep.yml to .rubocop.yml. The tier change belongs to the sweep-promotion process (#1317), not to this false-positive fix.
Out of scope: docs/crc/os-sdlc/fast-batch-1/finding-c-underscored-parameter.md. It is a dated record of fast batch 1; the History row of this ticket records that the two Boot lambdas it fixed are no longer in the cop's shape.
Out of scope (cut on 2026-10-26, no site in plugins/): a lambda that its own file calls, and calls through send(:name) or public_send.

Blocking edges

None. One implement run.

Origin

Trigger: #1325 zero-false-positive audit of every Sdlc cop.
Improvised this session: none
Chain: false positives on forced arity ← matcher reports a definition whose callers it cannot see ← DESIGN (plugins/os-sdlc/lib/os_sdlc/cops/structural/underscored_parameter.rb)
Root candidate: none found; searched UnderscoredParameter, underscored, underscore, unused param, arity, protocol, Structural, carve out
Where: plugins/os-sdlc/lib/os_sdlc/cops/structural/underscored_parameter.rb
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/UnderscoredParameter` tells the agent: "Delete it and stop passing it, or read it." That fix is correct only when the agent can change every caller. The cop sees one file. When the caller is in another file, the arity is often set by a framework, a protocol that sibling implementers read, or a dispatch table, and deletion breaks the caller. The cop runs only in `plugins/os-sdlc/.rubocop-sweep.yml`. 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, with the sweep config over every file in `plugins/` (25 offenses): | shape | all plugins | #1317 sweep | verdict | example | |---|---|---|---|---| | def or defs whose name the same file sends with no receiver or with `self` | 4 | 1 | true positive: every caller is in view, so the fix is always correct | `plugins/cc-architect/scripts/plugin_config/operations/audit.rb:121` private `check_installed(_name, key)`, called only at `:72`; sweep hit `plugins/os-backlog/tests/project_index_test.rb:54` | | framework hook (no caller in the file) | 1 | 0 | false positive: Thor sets the arity | `plugins/os-sdlc/lib/os_sdlc/boot/onboarding.rb:17` `def self.handle_no_command_error(command, _has_namespace = $thor_runner)` | | test double of a production protocol | 6 | 0 | false positive: the production caller sends the full list | `plugins/os-sdlc/tests/daemon/lsp_session_reopen_test.rb:31` `def request(_method, params)` | | callback lambda in a test, given to production code | 2 | 0 | false positive: the production recorder protocol passes the value | `plugins/os-sdlc/tests/gate_test.rb:62` `->(_outcome, classification:) { ... }` | | production protocol method or dispatch-table entry (def, `method(:x)`, or lambda) called from another file | 12 | 0 | false positive: siblings read the value (`Takedown::StepList#steps(run, history)` beside `ProbePlan#steps(_run, history)`), and the fix needs new caller machinery that the cop cannot see | `plugins/os-sdlc/lib/os_sdlc/crc/shares/run.rb:10` `"verdicts-merge" => ->(path, _run) { VerdictMerge.new(path) }`; `plugins/os-sdlc/lib/os_sdlc/dispatch/rerouting.rb:10` | The #1317 sweep of os-backlog, os, os-adr, os-aidd-lint, and os-me finds 1 offense (`plugins/os-backlog/tests/project_index_test.rb:54`). It is a true positive. ## Approach Fix by shape in the cop matcher. Never use a config Exclude list or a test-path exclusion. Rule: the cop checks a `def` or `defs` only when the same file holds a send of its method name with no receiver or with a `self` receiver. It no longer checks lambdas. The existing parameter test does not change: report an underscored parameter when the definition reads another parameter. Measured effect (scratch simulation that patches the cop, main 4a0fc41d): offenses over `plugins/` go from 25 to 4. The 4 kept are the true positives above (`audit.rb:121`, `plugins/os-adr/eval-c/bin/check:226`, `project_index_test.rb:54`, `plugins/os-sdlc/lib/os_sdlc/implementation/lifecycle.rb:42`). The 21 removed are all false positives. The #1317 sweep count stays 1. The lesson's dispatch-table example (from fast-batch-1, docs/crc/os-sdlc/fast-batch-1/finding-c-underscored-parameter.md) no longer fires. That shape has a correct fix only when the table's caller is in this codebase, and no AST shape in one file tells that case apart from a protocol-forced lambda. The cop gives up that recall to keep every offense true, as in ruling R2 (docs/plans/class-length-family/HANDOFF.md). Required work: 1. In `plugins/os-sdlc/lib/os_sdlc/cops/structural/underscored_parameter.rb`, check a `def` or `defs` only when `processed_source.ast` holds a `send` or `csend` of the same method name whose receiver is nil or `self`. Delete the `on_block` lambda check. check: absent `def on_block` in `plugins/os-sdlc/lib/os_sdlc/cops/structural/underscored_parameter.rb` 2. In `plugins/os-sdlc/tests/cops/underscored_parameter_test.rb`, change the two lambda fixtures that expect an offense (`test_reports_an_underscored_lambda_parameter_beside_read_ones`, `test_reports_an_underscored_parameter_inside_a_destructured_lambda_parameter`) to tests that expect no offense, and add an in-file call to the fixtures of `test_reports_an_underscored_def_parameter_beside_read_ones` and `test_reports_an_underscored_keyword_rest_parameter` so that they still fire. Add must-pass fixtures: a Thor hook `def self.handle_no_command_error(command, _has_namespace = nil)` with no caller in the file, a test double `def call(_repo, target) = target`, and a method that the file calls only on another receiver (`obj.call(repo, target)`). Add must-fire fixtures: a private helper called with no receiver, a method called through `self.`, a destructured def parameter `def show(io, (action, _id))` called in the file, and a call that comes before the `def`. check: present `handle_no_command_error` in `plugins/os-sdlc/tests/cops/underscored_parameter_test.rb` 3. In `plugins/os-sdlc/lint-lessons/sdlc-structural-underscored-parameter.yml`, state in `signal` that the same file calls the method (today it says "A method or lambda"). Rewrite `decision` and `correction` for that shape: the callers are in the file, so delete the parameter and the argument at each call; drop the "second constructor on the table entry" advice. Replace the dispatch-table `example` with a method that the same file calls (before: `check_installed(_name, key)` called as `check_installed(name, key)`; after: `check_installed(key)`). Run `bin/refresh-plugins` after the edit. check: present `same file calls` in `plugins/os-sdlc/lint-lessons/sdlc-structural-underscored-parameter.yml` 4. In `docs/plans/class-length-family/sdlc-cop-audit.md`, update the UnderscoredParameter row (line 63) and line 15: the cop path is `plugins/os-sdlc/lib/os_sdlc/cops/structural/underscored_parameter.rb`, and the cop now checks only a def that the same file calls. check: absent `os_sdlc/cops/underscored_parameter.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`. check: present `#1370` in `docs/implementation-status/os-sdlc.md` ## Acceptance criteria 1. Given `def self.handle_no_command_error(command, _has_namespace = nil)` that reads `command` and that no line of the file calls, when the cop runs, then it reports no offense. 2. Given a lambda `->(path, _run) { VerdictMerge.new(path) }`, as a hash value, a call argument, or a local variable, when the cop runs, then it reports no offense. 3. Given a private `def check_installed(_name, key)` that reads `key`, and `check_installed(name, key)` in the same file, when the cop runs, then it reports `_name`. 4. Given `def record(_result, text)` that reads `text`, and `self.record(result, text)` in the same file, when the cop runs, then it reports `_result`. 5. Given `bundle exec rubocop -c .rubocop-sweep.yml --only Sdlc/Structural/UnderscoredParameter ../../plugins` run from `plugins/os-sdlc`, when it runs after the change, then it reports exactly 4 offenses, at `audit.rb:121`, `check:226`, `project_index_test.rb:54`, and `lifecycle.rb:42`. ## Owner sketch - `RuboCop::Cop::Sdlc::Structural::UnderscoredParameter` (existing) owns the decision. It already partitions parameters and checks reads; it adds one question, "does this file call the method by name?", before it reports. No new object. ## In scope / out of scope - In scope: the matcher rule, its tests, and the lesson. - Out of scope: fixing the 4 true positives. The sweep drain owns them (#1317 for the os-backlog hit). `lifecycle.rb:42` needs `plugins/os-sdlc/lib/os_sdlc/implementation/landing.rb:51` to stop passing `root:`. - Out of scope: moving the cop from `.rubocop-sweep.yml` to `.rubocop.yml`. The tier change belongs to the sweep-promotion process (#1317), not to this false-positive fix. - Out of scope: `docs/crc/os-sdlc/fast-batch-1/finding-c-underscored-parameter.md`. It is a dated record of fast batch 1; the History row of this ticket records that the two Boot lambdas it fixed are no longer in the cop's shape. - Out of scope (cut on 2026-10-26, no site in `plugins/`): a lambda that its own file calls, and calls through `send(:name)` or `public_send`. ## Blocking edges None. One implement run. ## Origin - Trigger: #1325 zero-false-positive audit of every Sdlc cop. - Improvised this session: none - Chain: false positives on forced arity ← matcher reports a definition whose callers it cannot see ← DESIGN (`plugins/os-sdlc/lib/os_sdlc/cops/structural/underscored_parameter.rb`) - Root candidate: none found; searched UnderscoredParameter, underscored, underscore, unused param, arity, protocol, Structural, carve out - Where: plugins/os-sdlc/lib/os_sdlc/cops/structural/underscored_parameter.rb - 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:23 +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 changed title from ~~Sdlc/Structural/UnderscoredParameter: carve out arity forced by a protocol or framework~~ to Sdlc/Structural/UnderscoredParameter: report only a def that the same file calls

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

jared added the

ready-for-agent

label

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

jared commented

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

Author

Owner

Decomposed AFK on 2026-10-26 (ticket-decompose). Measured on main 4a0fc41d with a scratch patch of the cop: 25 offenses over plugins/ become 4, all true positives; the 21 removed are all false positives. #1317 sweep count stays 1 (true positive).

AFK decisions:

Rule "report a def only when the same file calls it by name (no receiver or self); stop checking lambdas", in place of the original carve-out list (hook names, callback lambdas, test doubles). Reason: one AST question covers every false-positive shape and keeps every true positive. Confidence 0.75.
Dispatch-table lambdas (the fast-batch-1 target shape) no longer fire. Reason: no single-file shape tells an in-codebase table apart from a protocol-forced lambda; ruling R2 accepts lost recall for zero false positives. Confidence 0.7. Rejected option: also report a lambda that is a direct hash value (7 offenses; adds crc/shares/run.rb:9-11, whose fix needs new caller machinery).
The tier move to .rubocop.yml and the fixes of the 4 true positives are out of scope (sweep-promotion process, #1317). Confidence 0.8.

Decomposed AFK on 2026-10-26 (ticket-decompose). Measured on main 4a0fc41d with a scratch patch of the cop: 25 offenses over `plugins/` become 4, all true positives; the 21 removed are all false positives. #1317 sweep count stays 1 (true positive). AFK decisions: - Rule "report a def only when the same file calls it by name (no receiver or `self`); stop checking lambdas", in place of the original carve-out list (hook names, callback lambdas, test doubles). Reason: one AST question covers every false-positive shape and keeps every true positive. Confidence 0.75. - Dispatch-table lambdas (the fast-batch-1 target shape) no longer fire. Reason: no single-file shape tells an in-codebase table apart from a protocol-forced lambda; ruling R2 accepts lost recall for zero false positives. Confidence 0.7. Rejected option: also report a lambda that is a direct hash value (7 offenses; adds `crc/shares/run.rb:9-11`, whose fix needs new caller machinery). - The tier move to `.rubocop.yml` and the fixes of the 4 true positives are out of scope (sweep-promotion process, #1317). Confidence 0.8.

jared commented

2026-10-26 15:55:50 +00:00

Author

Owner

Work started on branch ticket-1370 via /os-sdlc:implement.

jared commented

2026-10-26 16:01:22 +00:00

Author

Owner

Implementation 172 failed at the test-reviewer node (dispatch 1503, reason verdict:error). Cause: required work R2 and acceptance criteria 3-4 ask for 4 tests of behavior the cop already has (private helper, self receiver, destructured def, call before def). They are green before any production change, and the red-first round requires every new test to be red; the pipeline has no exemption for preservation tests. Reviewer 1501 objected (objection 161); test-writer 1502 could not fix it with a test-only edit; reviewer 1503 settled error. Worktree ticket-1370 is kept: 11 new tests in tests/cops/underscored_parameter_test.rb, 7 red as intended, 4 green; no production change. Recovery: hand-finish (production change makes all 11 pass) or move the 4 preservation tests out of the red round. Labeled waiting.

jared added the

waiting

label

2026-10-26 16:01:22 +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 referenced this issue

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

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

jared referenced this issue from a commit

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