jared/cc-os

Watch

Fork 0

Tell the test-reviewer which tests a later red round added #811

New issue

Closed

opened 2026-09-28 20:05:43 +00:00 by jared · 3 comments

jared commented

2026-09-28 20:05:43 +00:00

Owner

Problem

When the behavior-verifier fails after the programmer has run, the map sends the run back to the test-writer. The production code for the earlier tests already exists at that point. The test-reviewer still expects every new test to be red. It saw 8 green tests and 2 red tests, gave verdict: error, and ended implementation 64 for #805. Its own note said the correct verdict was pass. A correct second red round cannot pass this review.

Observed

Implementation 64, ticket #805: programmer 662 pass, behavior-verifier 665 fail, test-writer 666, test-reviewer 667 fail, test-writer 668, test-reviewer 669 error, then implementation_failed with reason: verdict:error.

Test-reviewer 669 final note:

I gave it because 8 of the 10 new tests were green. The dispatch history shows why they were green: programmer 662 already ran, and behavior-verifier 665 failed, which sent the run back to test-writer. At that point the presenters were supposed to exist, so those tests should pass. Only the 2 red commit tests had to fail, and they fail for the intended reason. Objections 128 and 129 are corrected. The correct verdict was `pass`.

Reproduce

n/a: needs a ticket whose behavior-verifier fails after the programmer runs; implementation 64 is the recorded instance.

Approach

The test-reviewer brief must tell the reviewer which tests are new in this round and which production code already exists. The reviewer judges red-for-the-right-reason only for tests that this round added or changed. Tests that earlier rounds added may be green.

Required work:

Give the test-reviewer node a fact that names the tests changed since the last green programmer dispatch, or states that no programmer dispatch has run.
Change the test-reviewer instructions so that the red requirement applies only to those tests.

Acceptance criteria

Given a first red round with no programmer dispatch, when the test-reviewer brief is built, then it states that all new tests must be red.
Given a red round after a green programmer dispatch, when the test-reviewer brief is built, then it names only the tests changed in this round as the tests that must be red.

Owner sketch

module OsSdlc
  class Brief
    class Facts
      def round_tests
    end
  end
end

Origin

Trigger: /os-sdlc:implement 805, implementation 64
Improvised this session: none; implementation stopped per the implement skill
Chain: implementation_failed ← test-reviewer error on green tests ← reviewer brief does not say that production already exists after a verifier-driven red round ← DESIGN (maps/poodr-ticket-implementation.yaml, behavior-verifier fail edge to test-writer)
Root candidate: this ticket
Where: plugins/os-sdlc/maps/poodr-ticket-implementation.yaml, plugins/os-sdlc/agents/test-reviewer.md
Session: b505a812-db25-4f85-8d52-ae7cda49eaab
Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/b505a812-db25-4f85-8d52-ae7cda49eaab.jsonl

VERDICT: CREATE — Real implementation-64 failure with a complete Origin chain terminating in a DESIGN gap (test-reviewer brief lacks per-round test scope), not improvisation, and no overlapping ticket in this session covers the test-reviewer/map.

## Problem When the behavior-verifier fails after the programmer has run, the map sends the run back to the test-writer. The production code for the earlier tests already exists at that point. The test-reviewer still expects every new test to be red. It saw 8 green tests and 2 red tests, gave `verdict: error`, and ended implementation 64 for #805. Its own note said the correct verdict was `pass`. A correct second red round cannot pass this review. ## Observed Implementation 64, ticket #805: programmer 662 pass, behavior-verifier 665 fail, test-writer 666, test-reviewer 667 fail, test-writer 668, test-reviewer 669 `error`, then `implementation_failed` with `reason: verdict:error`. Test-reviewer 669 final note: ``` I gave it because 8 of the 10 new tests were green. The dispatch history shows why they were green: programmer 662 already ran, and behavior-verifier 665 failed, which sent the run back to test-writer. At that point the presenters were supposed to exist, so those tests should pass. Only the 2 red commit tests had to fail, and they fail for the intended reason. Objections 128 and 129 are corrected. The correct verdict was `pass`. ``` ## Reproduce n/a: needs a ticket whose behavior-verifier fails after the programmer runs; implementation 64 is the recorded instance. ## Approach The test-reviewer brief must tell the reviewer which tests are new in this round and which production code already exists. The reviewer judges red-for-the-right-reason only for tests that this round added or changed. Tests that earlier rounds added may be green. Required work: 1. Give the test-reviewer node a fact that names the tests changed since the last green programmer dispatch, or states that no programmer dispatch has run. 2. Change the test-reviewer instructions so that the red requirement applies only to those tests. ## Acceptance criteria 1. Given a first red round with no programmer dispatch, when the test-reviewer brief is built, then it states that all new tests must be red. 2. Given a red round after a green programmer dispatch, when the test-reviewer brief is built, then it names only the tests changed in this round as the tests that must be red. ## Owner sketch ```ruby module OsSdlc class Brief class Facts def round_tests end end end ``` ## Origin - Trigger: `/os-sdlc:implement 805`, implementation 64 - Improvised this session: none; implementation stopped per the implement skill - Chain: implementation_failed ← test-reviewer `error` on green tests ← reviewer brief does not say that production already exists after a verifier-driven red round ← DESIGN (`maps/poodr-ticket-implementation.yaml`, behavior-verifier `fail` edge to test-writer) - Root candidate: this ticket - Where: `plugins/os-sdlc/maps/poodr-ticket-implementation.yaml`, `plugins/os-sdlc/agents/test-reviewer.md` - Session: b505a812-db25-4f85-8d52-ae7cda49eaab - Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/b505a812-db25-4f85-8d52-ae7cda49eaab.jsonl VERDICT: CREATE — Real implementation-64 failure with a complete Origin chain terminating in a DESIGN gap (test-reviewer brief lacks per-round test scope), not improvisation, and no overlapping ticket in this session covers the test-reviewer/map.

jared added the

plugin/os-sdlc

filed-by/agent

labels

2026-09-28 20:05:43 +00:00

jared referenced this issue

2026-09-28 20:08:42 +00:00

Give docs-only tickets an os-sdlc map with no red round #812

jared referenced this issue

2026-09-29 11:44:33 +00:00

Delete lint_paths: a target lints only its own path (T6, coding map) #802

jared referenced this issue

2026-09-29 11:44:50 +00:00

Give docs-only tickets an os-sdlc map with no red round #812

jared added the

bug

ready-for-agent

update

labels

2026-09-29 11:44:56 +00:00

jared commented

2026-09-29 11:44:57 +00:00

Author

Owner

Triage 2026-09-29: verdict CONFIRMED, ready-for-agent (P2, bug, update). validate-template 811 --agent-ready exits 0.

Defect still exists on main bf926443. No commit since 2026-09-28 touches it.

plugins/os-sdlc/agents/test-reviewer.md: the rules treat a test as unexpectedly green unless a red_tests entry names it. No rule for tests that earlier rounds added.
plugins/os-sdlc/lib/os_sdlc/query/brief/facts.rb:68-71: test_changes is the whole test diff, not this round's diff. No round_tests fact exists.
plugins/os-sdlc/maps/poodr-ticket-implementation.yaml:72: the test-reviewer enrich list has no round-scope fact.

Trigger: NOT every run. It fires only when the programmer has already run before a test-writer round. The only such path is the behavior-verifier fail edge to test-writer (poodr-ticket-implementation.yaml:130). The initial path (test-writer, then test-reviewer, then programmer; lines 42-77) has no production code yet. A test-reviewer fail edge (line 77) or a test-writer fail edge (line 59) before the first programmer run is safe.

Related: #805 is the instance (implementation 64) and is waiting on this ticket. #806 is waiting and shares the failure-reporting area. #812 (docs-only map with no red round) is separate. #802 and #818 are unrelated.

Triage 2026-09-29: verdict CONFIRMED, ready-for-agent (P2, bug, update). `validate-template 811 --agent-ready` exits 0. Defect still exists on main bf926443. No commit since 2026-09-28 touches it. - plugins/os-sdlc/agents/test-reviewer.md: the rules treat a test as unexpectedly green unless a `red_tests` entry names it. No rule for tests that earlier rounds added. - plugins/os-sdlc/lib/os_sdlc/query/brief/facts.rb:68-71: `test_changes` is the whole test diff, not this round's diff. No `round_tests` fact exists. - plugins/os-sdlc/maps/poodr-ticket-implementation.yaml:72: the test-reviewer enrich list has no round-scope fact. Trigger: NOT every run. It fires only when the programmer has already run before a test-writer round. The only such path is the behavior-verifier `fail` edge to test-writer (poodr-ticket-implementation.yaml:130). The initial path (test-writer, then test-reviewer, then programmer; lines 42-77) has no production code yet. A test-reviewer `fail` edge (line 77) or a test-writer `fail` edge (line 59) before the first programmer run is safe. Related: #805 is the instance (implementation 64) and is `waiting` on this ticket. #806 is `waiting` and shares the failure-reporting area. #812 (docs-only map with no red round) is separate. #802 and #818 are unrelated.

jared referenced this issue

2026-09-29 11:44:58 +00:00

Replace the NCPU and PROCMAN_REAL test ENV reads with defaults and explicit parameters #818

jared referenced this issue

2026-09-29 13:02:30 +00:00

Move os-sdlc orchestrator payload rendering into presenters #805

jared commented

2026-09-29 13:19:57 +00:00

Author

Owner

Work started on branch ticket-takedown (worktree .claude/worktrees/ticket-takedown), via /os-sdlc:implement.

jared referenced this issue

2026-09-29 14:03:22 +00:00

round_tests must skip test files deleted in the current round #823

jared commented

2026-09-29 14:03:28 +00:00

Author

Owner

Resolution

Done: round_tests brief fact tells the test-reviewer which tests changed since the last passed programmer dispatch; the reviewer requires red only for those tests

Evidence: branch ticket-takedown commit 11339a12 (history row 3635800d); implementation 75: 11 dispatches, suite-check green twice, contract-auditor pass on second round

Follow-ups: #823 round_tests crashes on test files deleted in the current round (captured, user-approved)

## Resolution **Done:** round_tests brief fact tells the test-reviewer which tests changed since the last passed programmer dispatch; the reviewer requires red only for those tests **Evidence:** branch ticket-takedown commit 11339a12 (history row 3635800d); implementation 75: 11 dispatches, suite-check green twice, contract-auditor pass on second round **Follow-ups:** #823 round_tests crashes on test files deleted in the current round (captured, user-approved)

jared closed this issue

2026-09-29 14:03:28 +00:00

jared referenced this issue from a commit

2026-09-29 16:46:31 +00:00

fix(os-sdlc): #811 test-reviewer requires red only for this round's tests

jared referenced this issue from a commit

2026-09-29 16:46:31 +00:00

docs: history row for #811