Sdlc/Structural/QueryMethodReachesWrite: anchor WRITE_NAME and QUERY_NAME to verb positions #1380

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

Problem

The cop Sdlc/Structural/QueryMethodReachesWrite matches write and query names anywhere in a method name, so pure computations get flagged.
Its one hit is a false positive.

Found by the #1325 zero-false-positive audit (docs/plans/class-length-family/sdlc-cop-audit.md).

Observed

Counterexample: plugins/os-context/audit/bin/extract:237. build reaches read_write_profile (:256), which only computes a hash. The unanchored WRITE_NAME matches the noun write_profile and rewrite. QUERY_NAME also matches format and force.

Reproduce

The cop matcher is at plugins/os-sdlc/lib/os_sdlc/cops/structural/query_method_reaches_write.rb (QUERY_NAME, WRITE_NAME); the chain walk is in plugins/os-sdlc/lib/os_sdlc/cops/structural/query_method_reaches_write/write_chain.rb. From plugins/os-sdlc, run bundle exec rubocop --only Sdlc/Structural/QueryMethodReachesWrite ../os-context/audit/bin/extract: 1 offense today, false.

Approach

Implement the rule in docs/lint-lessons/analyses/sdlc-structural-query-method-reaches-write.md step 6. Carve-outs go in the cop matcher, never in a config Exclude list or a test-path exclusion.

  • Query def: name matches \A(for|find|fetch|get|build)(_|\z) and does not end with ? or !.
  • Write def: name matches \A(save|persist|write|update)(_|!|\z), OR its body sends a method whose name matches that same anchored pattern (for example project.save_tracker, File.write).
  • Reach stays as today: calls with no receiver or receiver self, to defs in the same file. Do not follow calls into other classes (#1449 holds that widening). Do not check sends in the query def's own body.
  • The offense message keeps its current shape.

Required work:

  1. Anchor both name patterns and count a def whose body sends an anchored write verb as a write def — check: present (_|!|\z) in plugins/os-sdlc/lib/os_sdlc/cops/structural/query_method_reaches_write.rb

Owner sketch

  • QueryMethodReachesWrite (cop, structural/query_method_reaches_write.rb) owns the two name patterns QUERY_NAME and WRITE_NAME.
  • QueryMethodReachesWrite::WriteChain (structural/query_method_reaches_write/write_chain.rb) owns the decision "is this called def a write def": its name matches WRITE_NAME, or its body sends a method whose name matches WRITE_NAME.
  • QueryMethodReachesWrite::WriteChainOffense keeps the message; it already names the concrete write send through concrete_write_call.
  • Tests go through the cop public seam in plugins/os-sdlc/tests/cops/query_method_reaches_write_test.rb.

Acceptance criteria

Must not fire:

  1. Given build that calls read_write_profile, which only computes a hash, when the cop runs, then it reports no offense.
  2. Given format that calls write_report, when the cop runs, then it reports no offense.
  3. Given force_reload that calls update_cache, when the cop runs, then it reports no offense.
  4. Given find that calls rewrite, which only computes a string, when the cop runs, then it reports no offense.

Must still fire (near shapes the new rule decides; each is red before the change):
5. Given find_user that calls store_user, whose body sends File.write(path, data), when the cop runs, then it reports an offense.
6. Given fetch_config that calls apply_config, whose body sends config.update!(values), when the cop runs, then it reports an offense.
7. The existing test test_flags_a_query_named_method_whose_same_file_chain_reaches_a_write (self.for calls resolve_and_persist, which sends project.save_tracker) stays green with its message unchanged.

Regression guards (green before and after; not part of the red round; the orchestrator probes them before merge): build calls write_report which calls File.write; find_user calls save_user; fetch_config calls update! defined in the same file. Each still reports an offense.

Origin

  • Trigger: #1325 zero-false-positive audit of the Sdlc cops
  • Improvised this session: none
  • Chain: false positive ← name patterns are unanchored ← DESIGN (plugins/os-sdlc/lib/os_sdlc/cops/structural/query_method_reaches_write.rb)
  • Root candidate: none found; searched QueryMethodReachesWrite
  • Where: plugins/os-sdlc/lib/os_sdlc/cops/structural/query_method_reaches_write.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 The cop `Sdlc/Structural/QueryMethodReachesWrite` matches write and query names anywhere in a method name, so pure computations get flagged. Its one hit is a false positive. Found by the #1325 zero-false-positive audit (docs/plans/class-length-family/sdlc-cop-audit.md). ## Observed Counterexample: `plugins/os-context/audit/bin/extract:237`. `build` reaches `read_write_profile` (:256), which only computes a hash. The unanchored `WRITE_NAME` matches the noun `write_profile` and `rewrite`. `QUERY_NAME` also matches `format` and `force`. ## Reproduce The cop matcher is at `plugins/os-sdlc/lib/os_sdlc/cops/structural/query_method_reaches_write.rb` (`QUERY_NAME`, `WRITE_NAME`); the chain walk is in `plugins/os-sdlc/lib/os_sdlc/cops/structural/query_method_reaches_write/write_chain.rb`. From `plugins/os-sdlc`, run `bundle exec rubocop --only Sdlc/Structural/QueryMethodReachesWrite ../os-context/audit/bin/extract`: 1 offense today, false. ## Approach Implement the rule in `docs/lint-lessons/analyses/sdlc-structural-query-method-reaches-write.md` step 6. Carve-outs go in the cop matcher, never in a config Exclude list or a test-path exclusion. - Query def: name matches `\A(for|find|fetch|get|build)(_|\z)` and does not end with `?` or `!`. - Write def: name matches `\A(save|persist|write|update)(_|!|\z)`, OR its body sends a method whose name matches that same anchored pattern (for example `project.save_tracker`, `File.write`). - Reach stays as today: calls with no receiver or receiver self, to defs in the same file. Do not follow calls into other classes (#1449 holds that widening). Do not check sends in the query def's own body. - The offense message keeps its current shape. Required work: 1. Anchor both name patterns and count a def whose body sends an anchored write verb as a write def — check: present `(_|!|\z)` in `plugins/os-sdlc/lib/os_sdlc/cops/structural/query_method_reaches_write.rb` ## Owner sketch - `QueryMethodReachesWrite` (cop, `structural/query_method_reaches_write.rb`) owns the two name patterns `QUERY_NAME` and `WRITE_NAME`. - `QueryMethodReachesWrite::WriteChain` (`structural/query_method_reaches_write/write_chain.rb`) owns the decision "is this called def a write def": its name matches `WRITE_NAME`, or its body sends a method whose name matches `WRITE_NAME`. - `QueryMethodReachesWrite::WriteChainOffense` keeps the message; it already names the concrete write send through `concrete_write_call`. - Tests go through the cop public seam in `plugins/os-sdlc/tests/cops/query_method_reaches_write_test.rb`. ## Acceptance criteria Must not fire: 1. Given `build` that calls `read_write_profile`, which only computes a hash, when the cop runs, then it reports no offense. 2. Given `format` that calls `write_report`, when the cop runs, then it reports no offense. 3. Given `force_reload` that calls `update_cache`, when the cop runs, then it reports no offense. 4. Given `find` that calls `rewrite`, which only computes a string, when the cop runs, then it reports no offense. Must still fire (near shapes the new rule decides; each is red before the change): 5. Given `find_user` that calls `store_user`, whose body sends `File.write(path, data)`, when the cop runs, then it reports an offense. 6. Given `fetch_config` that calls `apply_config`, whose body sends `config.update!(values)`, when the cop runs, then it reports an offense. 7. The existing test `test_flags_a_query_named_method_whose_same_file_chain_reaches_a_write` (`self.for` calls `resolve_and_persist`, which sends `project.save_tracker`) stays green with its message unchanged. Regression guards (green before and after; not part of the red round; the orchestrator probes them before merge): `build` calls `write_report` which calls `File.write`; `find_user` calls `save_user`; `fetch_config` calls `update!` defined in the same file. Each still reports an offense. ## Origin - Trigger: #1325 zero-false-positive audit of the Sdlc cops - Improvised this session: none - Chain: false positive ← name patterns are unanchored ← DESIGN (plugins/os-sdlc/lib/os_sdlc/cops/structural/query_method_reaches_write.rb) - Root candidate: none found; searched QueryMethodReachesWrite - Where: plugins/os-sdlc/lib/os_sdlc/cops/structural/query_method_reaches_write.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
Author
Owner

Work started on branch ticket-1380 (os-sdlc implement, AFK unit).

Work started on branch ticket-1380 (os-sdlc implement, AFK unit).
Author
Owner

Lens proposal (contract-auditor A12, challenge verdict: code defect found) not filed; ticket-skeptic DROP: speculative hardening. WriteChain#body_mutates? and WriteChainOffense#concrete_write_call both scan a def body for a WRITE_NAME send; one owner (for example WriteSends.first) would stop drift if a carve-out is added later.

Lens proposal (contract-auditor A12, challenge verdict: code defect found) not filed; ticket-skeptic DROP: speculative hardening. WriteChain#body_mutates? and WriteChainOffense#concrete_write_call both scan a def body for a WRITE_NAME send; one owner (for example WriteSends.first) would stop drift if a carve-out is added later.
Author
Owner

Resolution

Done: Anchored QUERY_NAME to \A(for|find|fetch|get|build)(|\z) and WRITE_NAME to \A(save|persist|write|update)(|!|\z); WriteChain now counts a def as a write when its name matches or its body sends an anchored write verb (keeps resolve_and_persist -> project.save_tracker firing). Lesson signal updated. Three new true hits fixed by rename: os-backlog config_locator tests build_linked_worktree -> create_linked_worktree (writes a .git file); takedown eval run_cli_test build_cli -> create_cli (default paths write grader-calibration.json).

Evidence: Implementation 205 implementation_complete (commit b02ed132); fixes 1ea1559b, 311ab69a; main pushed at 311ab69a. os-sdlc suite 2861 runs 0 failures (31.6s); os-backlog suite 344 runs 0 failures. Plugin-wide cop hits over 2748 Ruby files (incl. extensionless scripts): before 1 (false, os-context/audit/bin/extract), after 0. Regression guards probed on main: build->write_report->File.write, find_user->save_user, fetch_config->update! each still fire.

Follow-ups: #1449 widening is unblocked. Red-round loop recorded as occurrence on #859. Lens proposal (A12 duplicate body-send scan) dropped by ticket-skeptic as speculative; noted in a comment here.

## Resolution **Done:** Anchored QUERY_NAME to \A(for|find|fetch|get|build)(_|\z) and WRITE_NAME to \A(save|persist|write|update)(_|!|\z); WriteChain now counts a def as a write when its name matches or its body sends an anchored write verb (keeps resolve_and_persist -> project.save_tracker firing). Lesson signal updated. Three new true hits fixed by rename: os-backlog config_locator tests build_linked_worktree -> create_linked_worktree (writes a .git file); takedown eval run_cli_test build_cli -> create_cli (default paths write grader-calibration.json). **Evidence:** Implementation 205 implementation_complete (commit b02ed132); fixes 1ea1559b, 311ab69a; main pushed at 311ab69a. os-sdlc suite 2861 runs 0 failures (31.6s); os-backlog suite 344 runs 0 failures. Plugin-wide cop hits over 2748 Ruby files (incl. extensionless scripts): before 1 (false, os-context/audit/bin/extract), after 0. Regression guards probed on main: build->write_report->File.write, find_user->save_user, fetch_config->update! each still fire. **Follow-ups:** #1449 widening is unblocked. Red-round loop recorded as occurrence on #859. Lens proposal (A12 duplicate body-send scan) dropped by ticket-skeptic as speculative; noted in a comment here.
jared closed this issue 2026-10-28 06:06:10 +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#1380
No description provided.