Accept edited existing test methods in takedown final-red #1687

New issue

Open

opened 2026-11-11 07:04:41 +00:00 by jared · 0 comments

jared commented

2026-11-11 07:04:41 +00:00

Owner

Problem

The takedown step final-red rejects a design whose failing tests edit an existing test method.
RedCheck counts a failing-test file as red only when the file adds a new test method.
A ticket that changes a kept behavior needs an edit of the existing test, so the run ends with FAILED final-red and the design is lost, after about $3.5 and 19 minutes.

Observed

Takedown run docs/takedown/1682-20261110-162833 (ticket #1682) ended with:

FAILED final-red: final design revise-2: failing test not red at base (not red at base: plugins/os-sdlc/test/fast_gate/fast_gate_bin_smoke_test.rb names no new test method)
...
total: 16 steps, 1 failed, 0 skipped, 4872066 tokens, $3.5263, 1131.8s
exit=1

The design revise-2 held two failing-test files.
plugins/os-sdlc/test/os_sdlc/project_collaborators_test.rb added a new method that was red at base (NoMethodError: undefined method 'shell' for an instance of OsSdlc::Project).
plugins/os-sdlc/test/fast_gate/fast_gate_bin_smoke_test.rb edited an existing method, so it added none.
Run files: revise-2.red-check.json ("red": false, the note above) and 16-final-red.json.
Step revise-1 failed the same check earlier (not red at base: OsSdlc::ProjectCollaboratorsTest#test_shell_defaults_to_a_project_shell, ...).

Code that produces this:

plugins/os-sdlc/eval/ticket-takedown/lib/takedown/model/red_check.rb:23-29 (run_file): fresh = TestMethods.ids(file.fetch("content")) - bases.fetch(path); when fresh is empty it returns UnnamedTest.new(file: path).
plugins/os-sdlc/eval/ticket-takedown/lib/takedown/model/unnamed_test.rb:9: def red? = false.
plugins/os-sdlc/eval/ticket-takedown/lib/takedown/model/red_check.rb:35: def red? = !@results.empty? && @results.all?(&:red?). One UnnamedTest makes the whole design not red.
plugins/os-sdlc/eval/ticket-takedown/lib/takedown/command/final_red_step.rb:7-14: a not-red check fails the step.
An edit of an existing method gives fresh empty because the method id is already in the base file, even when the edit makes that method fail at base.
Related, not the same: #1002 asks the design cards to say that every new failing_tests method must be red at base; it does not cover edited methods.

Reproduce

n/a: a run costs minutes and money. A unit test of Takedown::RedCheck.run with a failing_tests entry whose content changes one assertion of an existing method shows the UnnamedTest result.

Approach

Decide how RedCheck treats an edited existing method.
Option A: compare method bodies, treat a method whose body changed as a candidate, and run it at base.
Option B: keep the rule and state it in the designer, critic, and reviser cards, so that a design moves each change of an existing test into the "Retired tests" and "Kept behavior" work.
State the chosen option in the ticket decomposition.

Required work:

Treat a changed existing test method as a failing-test candidate in RedCheck.run_file — check: present changed in plugins/os-sdlc/eval/ticket-takedown/lib/takedown/model/red_check.rb

Acceptance criteria

Given a failing-test file that changes the body of an existing test method so that it fails at base, when RedCheck runs, then that method is run and its result is red.
Given a failing-test file that changes nothing in any test method, when RedCheck runs, then the result is UnnamedTest and the check is not red.
Given a design with one new red test and one changed existing test that is red, when final-red runs, then the step is ok.

Owner sketch

class Takedown::RedCheck
  def self.run_file(tests, repo, file, bases)
end

Non-goals (optional)

The wording of the design cards (#1002).

Origin

Trigger: bin/takedown-run for ticket #1682 (run docs/takedown/1682-20261110-162833) ended at final-red.
Improvised this session: none
Chain: run ends at final-red ← RedCheck.run_file treats a file with no new method id as UnnamedTest (red_check.rb:26) ← the check was written for new test files and new methods (#995)
Root candidate: none found (ticket-skeptic)
Where: plugins/os-sdlc/eval/ticket-takedown/lib/takedown/model/red_check.rb
Session: a48658d5-b566-40ce-8257-ebeabf56f7e1
Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os-plugins-os-sdlc/a48658d5-b566-40ce-8257-ebeabf56f7e1.jsonl

VERDICT: CREATE
REASON: Observed failure on the #1682 takedown run; #1002 covers the design card wording, not the check code.

## Problem The takedown step `final-red` rejects a design whose failing tests edit an existing test method. `RedCheck` counts a failing-test file as red only when the file adds a new test method. A ticket that changes a kept behavior needs an edit of the existing test, so the run ends with `FAILED final-red` and the design is lost, after about $3.5 and 19 minutes. ## Observed Takedown run `docs/takedown/1682-20261110-162833` (ticket #1682) ended with: ``` FAILED final-red: final design revise-2: failing test not red at base (not red at base: plugins/os-sdlc/test/fast_gate/fast_gate_bin_smoke_test.rb names no new test method) ... total: 16 steps, 1 failed, 0 skipped, 4872066 tokens, $3.5263, 1131.8s exit=1 ``` The design `revise-2` held two failing-test files. `plugins/os-sdlc/test/os_sdlc/project_collaborators_test.rb` added a new method that was red at base (`NoMethodError: undefined method 'shell' for an instance of OsSdlc::Project`). `plugins/os-sdlc/test/fast_gate/fast_gate_bin_smoke_test.rb` edited an existing method, so it added none. Run files: `revise-2.red-check.json` (`"red": false`, the note above) and `16-final-red.json`. Step `revise-1` failed the same check earlier (`not red at base: OsSdlc::ProjectCollaboratorsTest#test_shell_defaults_to_a_project_shell, ...`). Code that produces this: - `plugins/os-sdlc/eval/ticket-takedown/lib/takedown/model/red_check.rb:23-29` (`run_file`): `fresh = TestMethods.ids(file.fetch("content")) - bases.fetch(path)`; when `fresh` is empty it returns `UnnamedTest.new(file: path)`. - `plugins/os-sdlc/eval/ticket-takedown/lib/takedown/model/unnamed_test.rb:9`: `def red? = false`. - `plugins/os-sdlc/eval/ticket-takedown/lib/takedown/model/red_check.rb:35`: `def red? = !@results.empty? && @results.all?(&:red?)`. One `UnnamedTest` makes the whole design not red. - `plugins/os-sdlc/eval/ticket-takedown/lib/takedown/command/final_red_step.rb:7-14`: a not-red check fails the step. An edit of an existing method gives `fresh` empty because the method id is already in the base file, even when the edit makes that method fail at base. Related, not the same: #1002 asks the design cards to say that every new `failing_tests` method must be red at base; it does not cover edited methods. ## Reproduce n/a: a run costs minutes and money. A unit test of `Takedown::RedCheck.run` with a `failing_tests` entry whose content changes one assertion of an existing method shows the `UnnamedTest` result. ## Approach Decide how `RedCheck` treats an edited existing method. Option A: compare method bodies, treat a method whose body changed as a candidate, and run it at base. Option B: keep the rule and state it in the designer, critic, and reviser cards, so that a design moves each change of an existing test into the "Retired tests" and "Kept behavior" work. State the chosen option in the ticket decomposition. Required work: 1. Treat a changed existing test method as a failing-test candidate in `RedCheck.run_file` — check: present `changed` in `plugins/os-sdlc/eval/ticket-takedown/lib/takedown/model/red_check.rb` ## Acceptance criteria 1. Given a failing-test file that changes the body of an existing test method so that it fails at base, when `RedCheck` runs, then that method is run and its result is red. 2. Given a failing-test file that changes nothing in any test method, when `RedCheck` runs, then the result is `UnnamedTest` and the check is not red. 3. Given a design with one new red test and one changed existing test that is red, when `final-red` runs, then the step is ok. ## Owner sketch ```ruby class Takedown::RedCheck def self.run_file(tests, repo, file, bases) end ``` ## Non-goals (optional) The wording of the design cards (#1002). ## Origin - Trigger: `bin/takedown-run` for ticket #1682 (run `docs/takedown/1682-20261110-162833`) ended at `final-red`. - Improvised this session: none - Chain: run ends at `final-red` ← `RedCheck.run_file` treats a file with no new method id as `UnnamedTest` (`red_check.rb:26`) ← the check was written for new test files and new methods (#995) - Root candidate: none found (ticket-skeptic) - Where: plugins/os-sdlc/eval/ticket-takedown/lib/takedown/model/red_check.rb - Session: a48658d5-b566-40ce-8257-ebeabf56f7e1 - Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os-plugins-os-sdlc/a48658d5-b566-40ce-8257-ebeabf56f7e1.jsonl VERDICT: CREATE REASON: Observed failure on the #1682 takedown run; #1002 covers the design card wording, not the check code.

jared added the

plugin/os-sdlc

filed-by/agent

category/code-change

labels

2026-11-11 07:04:41 +00:00

jared referenced this issue

2026-11-12 18:28:29 +00:00

os-sdlc entry: decide and build Thor for argument parsing behind the fixed os-sdlc-* script names (ADR-0232) #1646