programmer diff-test gate silently runs the full suite when any changed file is unmapped #625

Closed
opened 2026-09-22 19:17:06 +00:00 by jared · 2 comments
Owner

Problem

GateCommands#diff_test_command returns the suite command whenever Changes#unmapped_code_paths is non-empty, before it looks at the tests that are already in the diff. A new production file whose test-writer test has a different basename is "unmapped" by construction, so every programmer gate run on such a ticket costs a full suite run, and the gate output does not say so. Verified still present on main at 599afa67 (2026-09-24).

Observed

Implementation 10 on #557 (session e8c75cf9): all five programmer gate runs (dispatch 125) took 118-139 s and reported 1365 runs / 3093 assertions under the step named diff-test. That is the full suite. suite-check (dispatch 127) took 123 s for the same suite.
#557 added lib/os_sdlc/model/timestamp_stamping.rb and tests/model/implementation_complete_stamp_test.rb (commit 52072ad5). Codebase#tests_for globs only <basename>_test.rb / <basename>_*_test.rb, so the production file is unmapped even though its test was in the diff.
Changes#test_paths (changes.rb:16-18) already unions changed test files with mapped tests. The unmapped guard at gate_commands.rb:27 fires first and discards that set.

Reproduce

# on a ticket branch that adds lib/x.rb plus tests/y_test.rb (different basename)
os-sdlc-runner programmer-gate   # diff-test step reports the whole suite's run count

Approach

Widen to the suite only when there is nothing else to run. Otherwise run the diff's own tests and name the unmapped paths in the gate reply.

plan = Pipeline::DiffTestPlan.new(project: project, changes: changes)
plan.command   # ruby -I... -e require... over changes.test_paths that exist on disk
               # suite command only when that set is empty and unmapped_code_paths.any?
plan.to_h      # { "unmapped" => ["lib/x.rb"], "widened" => false }

One DiffTestPlan instance per gate run, shared by GateCommands and BriefFacts#append_unmapped_note (brief_facts.rb:83-90), so the brief note and the gate reply cannot disagree about widened.

Required work:

  1. Add Pipeline::DiffTestPlan (paths, unmapped, widened?, command). GateCommands#diff_test_command delegates to it; the suite fallback applies only when the existing test set is empty.
  2. GateCommands#to_h today returns only command strings, and GateHandoff#gate_commands (gate_handoff.rb:52-54) consumes that. The DiffTestPlan must survive to GateHandoff#record (gate_handoff.rb:70-74), where GateReply is built. GateReply is a Struct with no field for it; add a keyword (e.g. diff_test_plan:) with a nil default so existing GateReply.new call sites, including the helper in tests/pipeline/gate_handoff_test.rb (~line 104), keep working. GateReply#to_h merges diff_test: plan.to_h only when the plan is present. This makes the diff-test step's reply carry unmapped and widened — check: os-sdlc-runner programmer-gate output on a branch with an unmapped file.
  3. BriefFacts#append_unmapped_note reads the plan and states what actually ran ("unmapped code files (no mapped test; diff-test ran changed tests only)" or "... widened to the suite").
  4. ADR: amend ADR-0202 clause 4, ADR-0210 line ~35 ("ADR-0208 D4 stands"), and ADR-0208 D4 itself via /os-adr:create — widening is now conditional and always named.
  5. History row in docs/implementation-status/os-sdlc.md.

Rejected: derive a convention-named test path for the new file and run it if present — tests_for already does that; it did not help #557 because the test had another basename.
Rejected: never widen — a bin/ or hooks/ only change would then run zero tests in the programmer gate.

Acceptance criteria

  1. Given a branch that adds lib/x.rb and tests/y_test.rb (no basename match), when the programmer diff-test gate runs, then only the changed and mapped test files run, and the reply carries diff_test.unmapped == ["lib/x.rb"] and diff_test.widened == false.
  2. Given a branch whose changed and mapped test set is empty and that changes at least one unmapped Ruby file, when the gate runs, then the suite runs and the reply carries diff_test.widened == true with the unmapped paths.
  3. Given a branch with no unmapped Ruby files, when the gate runs, then the reply carries diff_test.unmapped == [] and diff_test.widened == false.
  4. Given any branch, when suite-check runs, then it still runs the full suite (unchanged).
  5. Given a brief for a branch as in (1), when the test-writer or programmer reads the test_changes fact, then the unmapped note says the diff-test ran changed tests only, not that it widened to the suite.
  6. Given a branch with both a mapped test file and an unmapped code file in the same diff, when the gate runs, then only the mapped test runs, and the reply carries diff_test.widened == false and diff_test.unmapped == [the unmapped file].

Owner sketch

module OsSdlc::Pipeline
  class DiffTestPlan
    def initialize(project:, changes:)
    def command
    def unmapped
    def widened?
    def to_h
  end
  class GateCommands
    def diff_test_command = plan.command
  end
  class GateHandoff
    def record   # threads plan into GateReply.new(diff_test_plan: plan, ...)
  end
  class GateHandoff::GateReply
    def to_h   # merges diff_test: plan.to_h when step == diff-test and plan present
  end
end

Non-goals

Changing Codebase#tests_for mapping rules (ADR-0210 clause 5). Making the test-writer name test files after the production basename.

Blast radius

  • coding.yaml, poodr-implementation.yaml, poodr-ticket-implementation.yaml gate the programmer on [diff-test, lint] and need no edit.
  • tests/support/dispatch_240_fixture.rb NAMES list is a captured fixture — do not rename existing tests in this ticket.
  • Tests that stay green under the new rule: tests/pipeline/gate_commands_project_dispatch_test.rb#test_diff_test_command_requires_the_mapped_test_file stays green.

Retired tests

Origin

  • Trigger: implementation 10 on #557, gate timing review; re-verified 2026-09-24 by reading gate_commands.rb:27 and changes.rb:16-21 on main 599afa67
  • Improvised this session: none
  • Chain: 120 s programmer gates ← diff-test returns suite command when any unmapped path ← ADR-0202 clause 4 / ADR-0208 D4 chose "widen to suite" over "error" without a "run what we have" option ← DESIGN (docs/adr/0202, docs/adr/0208)
  • Root candidate: this ticket
  • Where: plugins/os-sdlc/lib/os_sdlc/pipeline/gate_commands.rb, plugins/os-sdlc/lib/os_sdlc/model/project/changes.rb
  • Session: d37d9a9a-bb4c-456a-a0cf-294619c9128a
  • Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/d37d9a9a-bb4c-456a-a0cf-294619c9128a.jsonl
## Problem `GateCommands#diff_test_command` returns the suite command whenever `Changes#unmapped_code_paths` is non-empty, before it looks at the tests that are already in the diff. A new production file whose test-writer test has a different basename is "unmapped" by construction, so every programmer gate run on such a ticket costs a full suite run, and the gate output does not say so. Verified still present on main at 599afa67 (2026-09-24). ## Observed Implementation 10 on #557 (session e8c75cf9): all five programmer gate runs (dispatch 125) took 118-139 s and reported 1365 runs / 3093 assertions under the step named `diff-test`. That is the full suite. suite-check (dispatch 127) took 123 s for the same suite. #557 added `lib/os_sdlc/model/timestamp_stamping.rb` and `tests/model/implementation_complete_stamp_test.rb` (commit 52072ad5). `Codebase#tests_for` globs only `<basename>_test.rb` / `<basename>_*_test.rb`, so the production file is unmapped even though its test was in the diff. `Changes#test_paths` (changes.rb:16-18) already unions changed test files with mapped tests. The unmapped guard at gate_commands.rb:27 fires first and discards that set. ## Reproduce ``` # on a ticket branch that adds lib/x.rb plus tests/y_test.rb (different basename) os-sdlc-runner programmer-gate # diff-test step reports the whole suite's run count ``` ## Approach Widen to the suite only when there is nothing else to run. Otherwise run the diff's own tests and name the unmapped paths in the gate reply. ```ruby plan = Pipeline::DiffTestPlan.new(project: project, changes: changes) plan.command # ruby -I... -e require... over changes.test_paths that exist on disk # suite command only when that set is empty and unmapped_code_paths.any? plan.to_h # { "unmapped" => ["lib/x.rb"], "widened" => false } ``` One `DiffTestPlan` instance per gate run, shared by `GateCommands` and `BriefFacts#append_unmapped_note` (`brief_facts.rb:83-90`), so the brief note and the gate reply cannot disagree about `widened`. Required work: 1. Add `Pipeline::DiffTestPlan` (paths, unmapped, widened?, command). `GateCommands#diff_test_command` delegates to it; the suite fallback applies only when the existing test set is empty. 2. `GateCommands#to_h` today returns only command strings, and `GateHandoff#gate_commands` (`gate_handoff.rb:52-54`) consumes that. The `DiffTestPlan` must survive to `GateHandoff#record` (`gate_handoff.rb:70-74`), where `GateReply` is built. `GateReply` is a Struct with no field for it; add a keyword (e.g. `diff_test_plan:`) with a nil default so existing `GateReply.new` call sites, including the helper in `tests/pipeline/gate_handoff_test.rb` (~line 104), keep working. `GateReply#to_h` merges `diff_test: plan.to_h` only when the plan is present. This makes the diff-test step's reply carry `unmapped` and `widened` — check: `os-sdlc-runner programmer-gate` output on a branch with an unmapped file. 3. `BriefFacts#append_unmapped_note` reads the plan and states what actually ran ("unmapped code files (no mapped test; diff-test ran changed tests only)" or "... widened to the suite"). 4. ADR: amend ADR-0202 clause 4, ADR-0210 line ~35 ("ADR-0208 D4 stands"), and ADR-0208 D4 itself via `/os-adr:create` — widening is now conditional and always named. 5. History row in `docs/implementation-status/os-sdlc.md`. Rejected: derive a convention-named test path for the new file and run it if present — `tests_for` already does that; it did not help #557 because the test had another basename. Rejected: never widen — a `bin/` or `hooks/` only change would then run zero tests in the programmer gate. ## Acceptance criteria 1. Given a branch that adds `lib/x.rb` and `tests/y_test.rb` (no basename match), when the programmer diff-test gate runs, then only the changed and mapped test files run, and the reply carries `diff_test.unmapped == ["lib/x.rb"]` and `diff_test.widened == false`. 2. Given a branch whose changed and mapped test set is empty and that changes at least one unmapped Ruby file, when the gate runs, then the suite runs and the reply carries `diff_test.widened == true` with the unmapped paths. 3. Given a branch with no unmapped Ruby files, when the gate runs, then the reply carries `diff_test.unmapped == []` and `diff_test.widened == false`. 4. Given any branch, when suite-check runs, then it still runs the full suite (unchanged). 5. Given a brief for a branch as in (1), when the test-writer or programmer reads the `test_changes` fact, then the unmapped note says the diff-test ran changed tests only, not that it widened to the suite. 6. Given a branch with both a mapped test file and an unmapped code file in the same diff, when the gate runs, then only the mapped test runs, and the reply carries `diff_test.widened == false` and `diff_test.unmapped == [the unmapped file]`. ## Owner sketch ```ruby module OsSdlc::Pipeline class DiffTestPlan def initialize(project:, changes:) def command def unmapped def widened? def to_h end class GateCommands def diff_test_command = plan.command end class GateHandoff def record # threads plan into GateReply.new(diff_test_plan: plan, ...) end class GateHandoff::GateReply def to_h # merges diff_test: plan.to_h when step == diff-test and plan present end end ``` ## Non-goals Changing `Codebase#tests_for` mapping rules (ADR-0210 clause 5). Making the test-writer name test files after the production basename. ## Blast radius - `coding.yaml`, `poodr-implementation.yaml`, `poodr-ticket-implementation.yaml` gate the programmer on `[diff-test, lint]` and need no edit. - `tests/support/dispatch_240_fixture.rb` NAMES list is a captured fixture — do not rename existing tests in this ticket. - Tests that stay green under the new rule: `tests/pipeline/gate_commands_project_dispatch_test.rb#test_diff_test_command_requires_the_mapped_test_file` stays green. ## Retired tests ## Origin - Trigger: implementation 10 on #557, gate timing review; re-verified 2026-09-24 by reading gate_commands.rb:27 and changes.rb:16-21 on main 599afa67 - Improvised this session: none - Chain: 120 s programmer gates ← diff-test returns suite command when any unmapped path ← ADR-0202 clause 4 / ADR-0208 D4 chose "widen to suite" over "error" without a "run what we have" option ← DESIGN (docs/adr/0202, docs/adr/0208) - Root candidate: this ticket - Where: plugins/os-sdlc/lib/os_sdlc/pipeline/gate_commands.rb, plugins/os-sdlc/lib/os_sdlc/model/project/changes.rb - Session: d37d9a9a-bb4c-456a-a0cf-294619c9128a - Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/d37d9a9a-bb4c-456a-a0cf-294619c9128a.jsonl
Author
Owner

Work started on branch ticket-625 via /os-sdlc:implement (session 91151d77). #626 shipped first on main 481425cf.

Work started on branch ticket-625 via /os-sdlc:implement (session 91151d77). #626 shipped first on main 481425cf.
Author
Owner

Resolution

Done: Pipeline::DiffTestPlan owns the diff-test rule: run changes.test_paths when non-empty; widen to the suite only when that set is empty and an unmapped code file exists; nil when both are empty. plan.to_h ({unmapped, widened}) is merged into the gate reply as diff_test for diff-test nodes via a nil-default GateReply field. One plan per gate run shared by GateCommands and GateHandoff. BriefFacts unmapped note words by widened?. ADR-0215 amends ADR-0202, ADR-0208, ADR-0210; status leaf row added.

Evidence: Branch ticket-625 commits 7ea704e7, 8fafa566. Pipeline implementation 30 reached programmer green then escalated (bound_exhausted:programmer-repair/fail) on an unrelated red test inherited from main (#633 fork-guard allowlist, filed as #701). Hand-finished per escalation-procedure step 4: allowlist fix, AbcSize extraction, single shared plan instance, nil command for empty/empty. Full suite 1425 runs, 3271 assertions, 0 failures; rubocop clean on touched files.

Follow-ups: #701 filed (fork guard ignores untracked test files, skeptic CREATE). The suite-red-from-a-test-file routing gap is already #665. Live verification of the narrowed widening happens on the next implement run.

## Resolution **Done:** Pipeline::DiffTestPlan owns the diff-test rule: run changes.test_paths when non-empty; widen to the suite only when that set is empty and an unmapped code file exists; nil when both are empty. plan.to_h ({unmapped, widened}) is merged into the gate reply as diff_test for diff-test nodes via a nil-default GateReply field. One plan per gate run shared by GateCommands and GateHandoff. BriefFacts unmapped note words by widened?. ADR-0215 amends ADR-0202, ADR-0208, ADR-0210; status leaf row added. **Evidence:** Branch ticket-625 commits 7ea704e7, 8fafa566. Pipeline implementation 30 reached programmer green then escalated (bound_exhausted:programmer-repair/fail) on an unrelated red test inherited from main (#633 fork-guard allowlist, filed as #701). Hand-finished per escalation-procedure step 4: allowlist fix, AbcSize extraction, single shared plan instance, nil command for empty/empty. Full suite 1425 runs, 3271 assertions, 0 failures; rubocop clean on touched files. **Follow-ups:** #701 filed (fork guard ignores untracked test files, skeptic CREATE). The suite-red-from-a-test-file routing gap is already #665. Live verification of the narrowed widening happens on the next implement run.
jared closed this issue 2026-09-24 20:11:45 +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#625
No description provided.