Retire the pipeline integration testing subsystem (#435)

[jeremymanning]

Closes #435. Deletes 2,223 lines that never ran; replaces the intent with six tests against the canonical compiler and orchestrator. Rationale recorded in docs/adr/0002-retire-pipeline-integration-infrastructure.md.

1. rg proof: no external consumers

$ rg 'pipeline_integration_infrastructure|pipeline_integration_demo' \
     --glob '!<the three files>' .
  (no matches)

$ rg 'PipelineTestModel|PipelineTestProvider|PipelineIntegrationResult|PipelineIntegrationValidator' \
     --glob '!<the three files>' .
  (no matches)

src/orchestrator/testing/__init__.py does not import them either.

2. Why deleted, not repaired

PipelineTestModel could not be constructed. Four keyword arguments, none of which exist:

Line	Passed	Actual field
89	ModelCapabilities(max_output_tokens=)	max_tokens
96–97	ModelRequirements(gpu_memory_gb=0, network_access=)	network_access doesn't exist
102	ModelMetrics(tokens_per_second=)	not a field
112–114	ModelCost(input_cost_per_token=, fixed_cost_per_request=)	*_per_1k_tokens, base_cost_per_request

Fixing one revealed the next. As instructed, I did not mechanically rename them — that would make the first layer pass while preserving a second testing architecture (its own model, provider, validator, scoring system, orchestration facade) inside the shipped package with no user-facing consumer.

3. What this uncovered — three real bugs

Routing the replacements through the canonical path immediately exposed defects the parallel stack had been standing in front of:

1. select_model() results were looked up twice. Five call sites did get_model(await select_model(...)), but select_model already returns a Model, so get_model()'s ":" in model_name raised TypeError: argument of type 'Model' is not a container. Every registry-selected action: generate step failed.
2. create_test_orchestrator() built the wrong registry. Two classes share the name ModelRegistry; only models.registry has register_provider, only models.model_registry has can_provide_models — which Orchestrator.__init__ calls. The helper used the first and raised on every call. (The split itself is Consolidate the two parallel model-adapter layers (models/providers vs integrations) #429.)
3. The deterministic model advertised the wrong task names — text-generation where the control system asks for generate — so it was ineligible for every generate step (NoEligibleModelsError).

None were visible while a second testing stack stood in for the real one. That is the strongest argument for deleting rather than repairing.

4. Replacement — tests/test_pipeline_model_contracts.py

Six tests, hermetic, using the deterministic model from tests/test_infrastructure.py, in the blocking contract/e2e layer:

Test	Pins
test_a_valid_pipeline_compiles	compiler accepts a well-formed pipeline
test_a_malformed_pipeline_is_refused	validation failure raises
test_a_model_pipeline_executes	a model step actually runs
test_structured_output_pipeline_returns_an_object	structured output is a mapping
test_execution_failure_is_reported_not_swallowed	a failing step reports failure
test_a_model_pipeline_compiles	xfail(strict) documenting #241

That last one is deliberate: a model pipeline runs but does not validate — strict validation resolves action: generate as a tool name. strict=True means it flips to a visible failure the day #241 is fixed, rather than sitting here forgotten.

Test-only models and providers stay under tests/, not in the shipped package.

5. Verification

	before	after
Collection	3215	3193 (−28 retired, +6 added), 0 warnings
Blocking suite	392 passed	397 passed, 1 xfailed
Integration selection	12 failed / 28 passed / 9 errors	unchanged
Correctness lint	—	clean (incl. tests/)

Legacy-suite regression comparison is running against a matched main baseline and will be posted as a comment.

6. Follow-ups

• Bring the legacy test suite to green (~540 failures outside the blocking layer) #354 will be updated to record that 27 tests were retired, not fixed.
• If systematic example-pipeline validation is wanted again, build it around the canonical orchestrator validate path — not another bespoke stack.

🤖 Generated with Claude Code

[jeremymanning]

Legacy-suite comparison (CI, same environment)

Both from the legacy-suite job, so the environment, lockfile and selection are identical.

	main @ bce2a5c	PR #437	Δ
failed	434	473	+39
errors	248	195	−53
passed	1827	1841	+14
skipped	228	200	−28
total red (failed+errors)	682	668	−14

The totals reconcile exactly: 434+1827+228+248 = 2737 vs 473+1841+200+195 = 2709, a difference of 28 — precisely the retired tests. Those 28 were skipped in CI on main, which is why skipped drops by the same number.

Among the tests that remain, 53 errors became 39 failures and 14 passes. An "error" here is a collection/setup failure; converting those into real outcomes is the expected effect of fixing create_test_orchestrator() (which raised on every call) and the select_model() double-lookup (which raised TypeError on every registry-selected generate step).

What this does and does not prove

It proves the net red count fell by 14 and that no tests vanished unaccounted for.

It does not by itself prove that zero individual tests regressed — +14 passed is equally consistent with "14 gained, 0 lost" and "20 gained, 6 lost". The CI artifact cannot settle this because the workflow runs with -rN, which suppresses the per-test summary.

I am running a test-level diff locally with -rf across two matched git worktrees (both with PYTHONPATH forced to their own src, since the venv's editable install otherwise resolves to the working tree and would have silently measured the same code twice). Result to follow.

Suggested follow-up

Change the legacy job from -rN to -rf so the published artifact supports a test-level diff. Right now the backlog is a scalar, which makes "did anything regress?" unanswerable from CI alone. Not included here to keep this PR to one concern.

[jeremymanning]

Test-level regression diff — resolved: no regressions

Two matched git worktrees, each with PYTHONPATH forced to its own src.

	main	PR #437
red test IDs	544	581
truly newly red	—	45

45 looked alarming, so I checked every affected file rather than reasoning from the aggregate. All 45 were ERROR on main — not one was passing:

File	newly red	status on main
test_data_processing.py	21	21 errors
test_control_flow_dynamic.py	10	10 errors
test_control_flow_conditional.py	9	10 errors
test_research_assistant_example.py	2	3 failed, 12 errors
orchestrator/quality/reporting/test_metrics.py	1	3 failed, 1 error

Every one errors on main with:

AttributeError: 'ModelRegistry' object has no attribute 'can_provide_models'

— the exact defect fixed here in create_test_orchestrator(). These tests never ran. They now run and fail for their own reasons (e.g. 'read_file.size' is undefined, which is the tool-envelope shape from #433 reaching templates).

Conclusion: 0 regressions. All 45 are error→failure transitions, matching the CI aggregate exactly (errors −53, failed +39, passed +14).

Two methodology errors I made getting here

Recording these because both produced confident, wrong intermediate answers:

1. The first worktree baseline measured the wrong code. The venv's editable install points at the working tree, so running pytest from a main worktree still imported my source. It only surfaced because that run aborted on an unrelated collection error. Fixed by forcing PYTHONPATH on both sides.
2. -rf reports failures but not errors. My first diff therefore compared main's failures only against the PR's failures, making 45 previously-errored tests look newly broken. The fix was to check each file's actual status on main rather than trust the list.

The underlying lesson is the same one behind the -rN suggestion above: the legacy backlog needs to be diffable, not a scalar. -raE (or at least -rfE) would have made this a one-line comparison instead of an investigation.

[jeremymanning]

Contracts strengthened, and one real defect fell out

The review was right that these tests asserted too little. test_a_model_pipeline_executes checked only that the step key existed — which passes for a step that failed, returned nothing, or emitted its own unrendered template.

test_a_model_pipeline_executes now asserts the whole envelope:

success is True          error is empty          action == "generate_text"
model_used == MockTestModel().name               result is a non-empty str
"the water cycle" in result   (the parameter reached the model)
no {{ }} / {% %} survives into the result

The pipeline now carries a real {{ topic }} with a default of "unsubstituted-default", so the no-unresolved-templates check cannot pass vacuously and a silently-defaulted parameter is caught too.

test_structured_output_pipeline_returns_an_object now asserts the payload is a Mapping and validates it with jsonschema against the schema the pipeline itself declares — one Python dict is dumped into the YAML and reused as the assertion, so the two cannot drift apart.

That assertion failed immediately, on a real bug

Every action in this project is spelled with underscores — generate_text, analyze_text, evaluate_condition, loop_complete. Only generate-structured was hyphenated, and it was the only spelling that dispatched (model_based_control_system.py:232).

generate_structured therefore fell through to the natural-language branch, which turns an unrecognised action into a prompt. Measured, before the fix:

action	returns
generate-structured	dict — {"test_output": "Structured response for: ..."}
generate_structured	str — "Test response for: generate_structured\n\nTask Parameters:..."

A sentence instead of an object, reported as success. tests/test_fact_checker.py:172 already carried a comment about "the generate-structured action returning strings", so this had been felt but not diagnosed.

Both spellings now dispatch, generate_structured is added to the declared supported_actions (it was implemented but undeclared), and both are pinned by parametrised tests. Removing the alias fails the underscore case — verified by reverting the fix and re-running:

AssertionError: a structured step must return an object, got <class 'str'>
1 failed, 1 passed

ADR 0001 corrected

It described the control-flow golden pipeline as covering "conditional branching". It does not — the fixture's own header says dependency ordering and parallel fan-out/fan-in, and there is no condition in it. Corrected, with a pointer to #333/#320 for the branching work. The list was also introduced as "Three" while enumerating four.

Numbers

	before	after
Blocking suite	397 passed	398 passed, 13 skipped, 1 xfailed, 0 failed
Collection	3193	3194, 0 warnings
uv lock --check	—	clean
ruff correctness on src	—	clean
git diff --check	—	clean

The +1 test and +1 collected item are the second parametrised structured case; measured against this same branch head in the same environment, so the delta is exactly the change.

Deliberately not in this PR

• The strict xfail stays. It is Make 'run' and 'validate' agree: run bypasses the validation gate entirely #241 — validate rejects a model pipeline the executor runs. Removing it means fixing that disagreement, which is its own change.
• Typed result / execution trace. Nothing here verifies a PipelineResult contract because there isn't one yet; the observable API is nested dicts. That is Implement Result API - log, outputs, qc() methods #332.
• tests/ correctness lint. Extending the ruff gate from src to tests surfaces 70 findings (66 F821 undefined-name, 4 F811 redefinition) across the legacy test surface. All pre-existing; none in any file touched here.