os-sdlc: test-writer/test-reviewer loop — fixture defects repeat and the two charters judge by different rules #644

New issue

Open

opened 2026-09-23 15:36:52 +00:00 by jared · 0 comments

jared commented

2026-09-23 15:36:52 +00:00

Owner

Problem

Ticket #638 implementation 13 and prior implementations surfaced repeat test-reviewer rejections. The user asked for a ticket to discuss the test-writer/test-reviewer loop: recurring fixture defects and a mismatch between the two charters' rules.

Observed

Data from .sdlc/pipeline.db, 13 implementations, 30 test-reviewer dispatches, 14 non-pass.

Fail reason	Count	Example
Fixture/setup makes the test red for the wrong reason	8	impl 6 d49/d51/d53: helper deletes tmpdir before Daemon.stop runs, same root cause 3 rounds; impl 8 d89 Hash passed instead of implementation record; impl 11 d132/d134 fixture never creates a distinct open dispatch; impl 13 d157 unseeded bin project
Case has no test	3	impl 1 d4 (cases 7,8); impl 11 d132 (case 6); impl 13 d155 (case 7)
Assertion checks more than the ticket specifies	3	impl 12 d144/d146 unspecified `brief` key
Unrelated test changed	1	impl 13 d155 state_dump includes→equals

Retries fixed the reviewer's literal complaint, but the new test added in the retry was red for a new fixture defect. This happened in impl 6, 11, and 13.

Ruled out: reviewer objections reach the retry test-writer verbatim and untruncated.

plugins/os-sdlc/lib/os_sdlc/pipeline/objections_digest.rb:16
model/objection/instance_methods.rb:16
BriefFacts#truncate (brief_facts.rb:111) applies only to gate_results and lint_offense_history, not to objections.

Charter mismatch:

Rule	test-writer	test-reviewer
Requirement source	"Read the ticket's acceptance criteria. Test exactly those criteria." (`test-writer.md:36,38`)	"cover each case in ticket_brief" (`test-reviewer.md:14`)
One behavior per test, no duplicates	absent	`test-reviewer.md:15`
No private-method tests / implementation coupling	absent	`test-reviewer.md:16`

Both agents receive the full ticket body (ticket_brief = implementations.ticket_body verbatim, brief_facts.rb:62), including ## Approach. Neither charter names ## Approach or instructs the agent to reason over the whole ticket.

Reproduce

n/a: this is a pattern observed across past pipeline runs recorded in .sdlc/pipeline.db, not a single reproducible command.

Expected

No fix is proposed yet. This ticket is for discussion; see the open questions below before any charter or pipeline change.

Questions for discussion (open, not decided)

Why limit either agent to "acceptance criteria" or "each case"? Should both reason over the entire ticket (Context, Observed, Owner sketch, Cases, Approach) to understand the problem, while Cases stay the unit of test coverage?
Why does the test-writer make the same fixture mistakes? Mine the recurring failure classes (tmpdir lifetime vs daemon stop, wrong record type passed, fixture that never reaches the state under test, unseeded project dir) and decide which should become explicit test-writer instructions or a pre-assertion step.
Starting-point proposal (for discussion, not decided): give the test-writer the reviewer's rules and the same requirement source; add a step that runs the arrange/setup once and proves it reaches the needed state before writing the assertion.

Related: #643 (Approach has no owner in the pipeline), #638 impl 13.

Origin

Trigger: user-requested capture, discussing #638 impl 13 and test-writer/test-reviewer pipeline data
Improvised this session: none
Chain: repeat fixture defects and inconsistent test coverage across pipeline rounds ← charter mismatch between test-writer.md and test-reviewer.md (DESIGN, plugins/os-sdlc test-writer.md/test-reviewer.md) ← repeated fixture failure classes not encoded as test-writer instructions (IMPROVISATION, no written rule covers them)
Root candidate: this ticket
Where: plugins/os-sdlc/agents/test-writer.md, plugins/os-sdlc/agents/test-reviewer.md
Session: d1aa78d1-c840-471e-bdd0-2be657c7d950
Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/d1aa78d1-c840-471e-bdd0-2be657c7d950.jsonl

## Problem Ticket #638 implementation 13 and prior implementations surfaced repeat test-reviewer rejections. The user asked for a ticket to discuss the test-writer/test-reviewer loop: recurring fixture defects and a mismatch between the two charters' rules. ## Observed Data from `.sdlc/pipeline.db`, 13 implementations, 30 test-reviewer dispatches, 14 non-pass. | Fail reason | Count | Example | | --- | --- | --- | | Fixture/setup makes the test red for the wrong reason | 8 | impl 6 d49/d51/d53: helper deletes tmpdir before Daemon.stop runs, same root cause 3 rounds; impl 8 d89 Hash passed instead of implementation record; impl 11 d132/d134 fixture never creates a distinct open dispatch; impl 13 d157 unseeded bin project | | Case has no test | 3 | impl 1 d4 (cases 7,8); impl 11 d132 (case 6); impl 13 d155 (case 7) | | Assertion checks more than the ticket specifies | 3 | impl 12 d144/d146 unspecified `brief` key | | Unrelated test changed | 1 | impl 13 d155 state_dump includes→equals | Retries fixed the reviewer's literal complaint, but the new test added in the retry was red for a new fixture defect. This happened in impl 6, 11, and 13. Ruled out: reviewer objections reach the retry test-writer verbatim and untruncated. - `plugins/os-sdlc/lib/os_sdlc/pipeline/objections_digest.rb:16` - `model/objection/instance_methods.rb:16` - `BriefFacts#truncate` (`brief_facts.rb:111`) applies only to `gate_results` and `lint_offense_history`, not to objections. Charter mismatch: | Rule | test-writer | test-reviewer | | --- | --- | --- | | Requirement source | "Read the ticket's acceptance criteria. Test exactly those criteria." (`test-writer.md:36,38`) | "cover each case in ticket_brief" (`test-reviewer.md:14`) | | One behavior per test, no duplicates | absent | `test-reviewer.md:15` | | No private-method tests / implementation coupling | absent | `test-reviewer.md:16` | Both agents receive the full ticket body (`ticket_brief` = `implementations.ticket_body` verbatim, `brief_facts.rb:62`), including `## Approach`. Neither charter names `## Approach` or instructs the agent to reason over the whole ticket. ## Reproduce n/a: this is a pattern observed across past pipeline runs recorded in `.sdlc/pipeline.db`, not a single reproducible command. ## Expected No fix is proposed yet. This ticket is for discussion; see the open questions below before any charter or pipeline change. ## Questions for discussion (open, not decided) 1. Why limit either agent to "acceptance criteria" or "each case"? Should both reason over the entire ticket (Context, Observed, Owner sketch, Cases, Approach) to understand the problem, while Cases stay the unit of test coverage? 2. Why does the test-writer make the same fixture mistakes? Mine the recurring failure classes (tmpdir lifetime vs daemon stop, wrong record type passed, fixture that never reaches the state under test, unseeded project dir) and decide which should become explicit test-writer instructions or a pre-assertion step. 3. Starting-point proposal (for discussion, not decided): give the test-writer the reviewer's rules and the same requirement source; add a step that runs the arrange/setup once and proves it reaches the needed state before writing the assertion. Related: #643 (Approach has no owner in the pipeline), #638 impl 13. ## Origin - Trigger: user-requested capture, discussing #638 impl 13 and test-writer/test-reviewer pipeline data - Improvised this session: none - Chain: repeat fixture defects and inconsistent test coverage across pipeline rounds ← charter mismatch between test-writer.md and test-reviewer.md (DESIGN, plugins/os-sdlc test-writer.md/test-reviewer.md) ← repeated fixture failure classes not encoded as test-writer instructions (IMPROVISATION, no written rule covers them) - Root candidate: this ticket - Where: plugins/os-sdlc/agents/test-writer.md, plugins/os-sdlc/agents/test-reviewer.md - Session: d1aa78d1-c840-471e-bdd0-2be657c7d950 - Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/d1aa78d1-c840-471e-bdd0-2be657c7d950.jsonl

jared added the

plugin/os-sdlc

filed-by/user

labels

2026-09-23 15:36:52 +00:00

jared referenced this issue

2026-09-23 16:46:03 +00:00

Ticket sections have no shared model, and pipeline roles work from silos of the ticket #643

jared referenced this issue

2026-09-23 17:00:32 +00:00

os-sdlc: blast-radius agent for any planned change; os-backlog: critique + loop-back step in ticket-decompose before ready-for-agent #647

jared referenced this issue