Calibrate mined Sdlc cops against their source test files #369
Labels
No labels
P0
P1
P2
P3
bug
create
delete
enhancement
filed-by/agent
filed-by/user
frozen
lint-rule
needs-info
needs-triage
next
plugin/cc-architect
plugin/os
plugin/os-adr
plugin/os-aidd-lint
plugin/os-backlog
plugin/os-context
plugin/os-doc-hygiene
plugin/os-sdlc
plugin/os-vault
project/cc-os
ready-for-agent
ready-for-human
recurring
review
update
waiting
wayfinder:grilling
wayfinder:map
wayfinder:map
wayfinder:research
wayfinder:task
wayfinder:task
wontfix
worklist/deviations
worklist/lint-rule
worklist/new-implement-build
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
jared/cc-os#369
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Context
Sdlc/Dry/DuplicatedStatementSequence (152 test-file offenses) and Sdlc/Structural/RawFileContentAssertion (31) were mined from the very test files they now flag. Audit whether findings are real test-quality defects or cop miscalibration before any mass fixing. This review gates decisions on #370 (test-lint-fixer).
Audit of 164 current offenses (39 files), sample of ~30 across 10 files reviewed. Verdict: cop miscalibration, mass-fix should NOT proceed as-is.
Breakdown: ~80% (b) miscalibration -- idiomatic minitest AAA pairs (arrange+act, or act+first assert) that recur because test methods share style, not logic: e.g. 'offenses = offenses_for(...)' + 'assert_equal 1, offenses.size', or 'assert_predicate status, :success?' + 'assert_match(...)'. ~15% (a) real defects -- construction boilerplate worth a helper, e.g. judge_test.rb's repeated 'invoker = FakeInvoker.new(...); judge = Judge.new(config, invoker: invoker)', and a genuine non-test hit: os-vault/eval/bin/check NegativeChecker#initialize duplicating PositiveChecker#initialize verbatim. ~5% borderline.
Signal: all 164 offenses fall back to the generic 'shared_method' suggestion -- none match the one hand-authored SUGGESTED_EXTRACTIONS pair the cop was mined from, i.e. WINDOW_SIZE=2 over-generalized past the original case.
Recommendation (not implemented): raise WINDOW_SIZE to 3-4, and/or exclude windows where any statement is an assert_/refute_ send (assertion idiom, not logic duplication). Gates #370: do not build test-lint-fixer against current findings without this recalibration.
Audit of Sdlc/Structural/RawFileContentAssertion (#377): 36 current offenses across 10 files (ticket said 31 -- some drift since #369 filed). Classification: 30 miscalibration, 6 real defects, 0 borderline. Miscalibrated (should be excluded/allowed): os-adr tests/{cli,index,migrator}_test.rb (15) assert on Index/MigrationReport markdown -- these classes' entire contract IS the rendered doc text, no richer accessor exists. os-aidd-lint tests/{aidd_lint_bin,report,hooks/post_tool_use_hook,hooks/session_end_hook}_test.rb (15) assert on Report#path / --fix output -- Report.md is a line-oriented text artifact by design (ADR-0027), and the --fix assertions check the actual rewritten file bytes, which is the CLI's job. Real defects: os-backlog tests/checkout_test.rb (4) and issues_test.rb (1) assert raw ## Bounced / Agent: / labels: strings via File.read even though Backlog::IssueBody (heading parser) and Issues.parse already exist and are used elsewhere in the same files -- should switch to those. cc-architect plugin_config test_cli.rb:143 (1) is a minor duplicate/loose check where a JSON.parse of settings_path would be tighter. Recommendation: don't mass-fix via #370 as-is -- first add a cop carve-out (skip when the file being read is produced by a class whose only public output is that exact document, e.g. classes named */Index|Report|MigrationReport/ or paths matching *report.md/index.md/README.md) OR require a marker comment on legitimate raw-content tests. Scope #370 to the 6 real-defect lines only until the carve-out lands.
DECISION (#378), both audits weighed.
Dry/DuplicatedStatementSequence -> RETUNE COP FIRST, then fix residual. #380: exclude any window containing an assert_/refute_ send, and delete the dead SUGGESTED_EXTRACTIONS map (0/164 matched). Diverging from the audit on WINDOW_SIZE: keep it at 2 for now. The noise the audit found was assertion recurrence, and the real hits (judge_test FakeInvoker/Judge construction) are exactly 2-statement arrange pairs that WINDOW_SIZE 3-4 would silence. One lever, then re-measure; #380 reports the residual count here and files a WINDOW_SIZE bump only if still noisy. The one non-test hit (os-vault/eval/bin/check duplicated #initialize) is #382.
Structural/RawFileContentAssertion -> FIX TESTS + CONFIG EXCLUDE, no cop code change. #381 adds rubocop Exclude for the 5 os-adr/os-aidd-lint document-renderer test files (30 offenses). Diverging from the audit: no class-name/path regex carve-out and no inline marker. Name-pattern matching (/Index|Report/) is guesswork inside the cop; the cop's own message already prescribes file exclusion as the escape hatch, so the standard rubocop mechanism is the right one and carries zero cop-logic risk. The 6 real defects stay reported.
#370 scope: narrows to the 6 real RawFileContentAssertion lines (os-backlog checkout_test.rb 96/109/158/168, issues_test.rb:367, cc-architect test_cli.rb:143), plus whatever DuplicatedStatementSequence residual survives #380. No mass fix against current findings.
New tickets: #380, #381, #382.
#380 done: residual Sdlc/Dry/DuplicatedStatementSequence offenses 164 -> 20 (12 files) after assertion-window exclusion + WINDOW_SIZE=2 kept. Commit
a15988e.Post-retune audit: 19/20 residuals are setup boilerplate; WINDOW_SIZE=3 measured at 20 -> 1. Follow-ups: #389 (window bump), #390 (config.rb extraction).
Frozen in the 2026-08-16 backlog reset — see #419 for the expiry procedure. Do not work unless a live run rediscovers this issue.