[issue-3312][slice-4/4] Decompose orchestrator/routes/pipelines.py... - #3512
Merged
Conversation
…move baseline) Step-0 bisectable baseline for the slice-4 decomposition of the 30,520-line orchestrator/routes/pipelines.py (#3312, closes the file-size program). Pure git mv into a sub-package __init__.py; the only edits are mechanically required by the move: - three routes-level relative imports rebased one level deeper (from .event_prompt/.messages/. import -> from ..) - the BLE001-narrowing audit test now concatenates the package tree so it still sees the full module surface post-split. No extraction yet. Package imports, all 16 @pipelines_bp routes register, and every test-patched module global resolves through the barrel unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The git mv of pipelines.py -> pipelines/__init__.py added a directory level, so the three Path(__file__).parent.parent.parent chains that resolve to repo-root shared/ and config/ now landed one level short (orchestrator/shared, orchestrator/config -- nonexistent). Add the extra .parent so _shared_path, _config_path, and the _read_shared_criteria prompt path resolve to the repo root again. Verified both exist. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lice-4)
First cohesive cluster extraction of the routes/pipelines.py decomposition.
Moves the 13 review-criteria string builders (_read_shared_criteria plus the
_get_*_criteria family, _get_review_criteria_for_type,
_get_reviewer_scope_preamble) verbatim into a private _criteria.py submodule
(961 lines, under cap) and re-exports them through the barrel.
Pattern (matches landed slices): the only patched module global these reach is
_read_shared_criteria (and logger), reached via 'import routes.pipelines as
_pkg' so patch('routes.pipelines._read_shared_criteria') keeps intercepting the
sibling calls. Extraction is AST-precise (only Name nodes rewritten, string
prompt bodies untouched). Barrel re-exports every moved symbol.
WIP: allowlist entry repointed to pipelines/__init__.py during incremental
extraction; removed in the final commit when the barrel drops under the cap.
Verified: import + routes register; patch seam propagates into the submodule;
ruff + format clean; size ratchet exit 0; 7612 tests collect (0 errors);
test_pipeline_prompts (511), test_lens_reviewer_prompts + test_first_principles
(48), test_pipelines_routes + BLE001 all green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rafts.py (#3312 slice-4) Second cluster of the routes/pipelines.py decomposition. Moves the 12 draft/artifact-reader symbols (_verdict_path_for_type, _draft_filename, _get_draft_path, _HUMAN_SPEC_BY_PHASE, _get_human_draft_path, _cleanup_stale_generic_drafts, _get_generic_draft_path, _git_show_draft, _read_source_branch_artifacts, _pull_contract_from_source_branch, _read_phase_draft, _read_human_phase_draft) verbatim into _drafts.py (756 lines) and re-exports them through the barrel. Patch seams: _get_draft_path, _git_show_draft (in-cluster) plus _pipeline_identifier and logger (barrel-resident) are reached via 'import routes.pipelines as _pkg', so patch('routes.pipelines.<name>') keeps intercepting. AST-precise rewrite. Barrel 29593 -> 28889. Verified: ruff + format clean, size ratchet exit 0, 7612 collect (0 errors), 181 draft/source-branch tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e-4) Third cluster. Moves _read_review_verdict, _read_tester_gaps, _aggregate_review_verdicts into _reviews.py; barrel re-exports them and drops the now-unused module-level ReviewVerdict / AggregatedReviewResult imports. Patched barrel globals (TESTER_FINDINGS_HEADER, logger) reached via _pkg; external deps (json, Path, models.ReviewVerdict/AggregatedReviewResult) and the sibling _verdict_path_for_type (now in _drafts) imported directly. AST-precise. Barrel 28889 -> 28720. Verified: ruff + format clean, ratchet exit 0, 7612 collect (0 errors), 54 review/prompt tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lice-4) Fourth cluster. Moves the 10 context-PR symbols (_compose_context_pr_body, _open_context_pr_at_implement_start, _refresh_context_pr_body, _persist_context_pr_number, _repos_with_slices, _maybe/_open_secondary_context_prs, pre-merge-obligation + brc-history-link builders) into _context_pr.py (1220 lines) and re-exports through the barrel. Drops the barrel's now-unused unwrap_soft_breaks import (only user moved out). Patched barrel globals reached via _pkg (_persist_context_pr_number, _pipeline_identifier, _get_draft_path, _detect_default_branch, _compute_gateway_mode, _get_spawner, get_pipeline_state_lock, logger, ContextPrCreationError); external deps + sibling _get_human_draft_path (in _drafts) imported directly. AST-precise. Barrel 28727 -> 27558. Verified: ruff+format clean, ratchet exit 0, 7612 collect (0 errors), 56 context-pr tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ce-reader tests (#3312 slice-4) Fifth cluster. Moves the 9 BRC-history symbols (BRC_HISTORY_TYPES, CONSENSUS_BRC_TYPES, _get_message_store, _render_brc_history_markdown, _write_brc_history_file, _write_brc_history, _rewrite_brc_history_for_pr, _persist_phase_brc_history, _commit_slice_brc_history_to_integration_branch) into _brc_history.py and re-exports through the barrel. Drops the barrel's now-unused yaml + SLICE_ID_PATTERN imports. Adds a TYPE_CHECKING ContainerSpawner import to the submodule for a forward-ref annotation. Also fixes source-reading tests that hardcoded the pre-decomposition routes/pipelines.py path (broken since the baseline git mv; task-4-6 mechanical patch-path rewrites): test_pipelines_apply, test_overseer_model, test_start_pipeline, test_slice_loop_import_seam, test_pipeline_role_to_reviewer_type_mapping, test_handlers_brc now concatenate the package tree. Barrel 27556 -> ~26640. Verified: ruff+format clean, ratchet exit 0, 7612 collect (0 errors), brc-history/apply/overseer/start/seam/role-mapping/handlers suites all green (600+ tests). (test_agent_salvage git-init failure is the known sandbox-env class, not split-induced.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…#3312 slice-4) Sixth and seventh clusters. Splits the ~2000-line worktree-sync/statefiles region into two submodules: - _statefiles.py (613 lines): _commit_statefiles_to_worktree, persist_contract_statefiles, _ensure_statefiles_on_branch, _detect_default_branch, _resolve_origin_ref, _fetch_pr_state. - _worktree_sync.py (1383 lines): WorktreeSyncOutcome, StalePipelineBranchError, _sync_worktree_with_remote, _rebase_pipeline_branch_onto_base, _refresh_pipeline_branch_against_current_base, the sync-recovery-backup + local-only-commit + tree-head/state-restore helpers. Patched barrel globals reached via _pkg. Restored the barrel's _rebase_with_agent_output_autoresolve gateway_client import (# noqa: F401) — it is a test-patch seam (patch(routes.pipelines._rebase_with_agent_output_autoresolve)) that must stay a barrel attribute even though only the submodule calls it (via _pkg). Added NamedTuple + TYPE_CHECKING ContainerSpawner imports to _worktree_sync. Barrel 26583 -> 24671. Verified: ruff+format clean, ratchet exit 0, 7612 collect (0 errors), sync_worktree (45) + statefiles/rebase/refresh suites green. The 9 remaining failures in the -k set are the known git-init sandbox-env class (fixture git init unsupported in container), pre-existing and not split-induced. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…3312 slice-4) Eighth cluster. Moves the 14 branch-divergence / consensus-timeout / progress-gate symbols (_emit_divergence_reconcile_hitl, _fail_pipeline_after_divergence_abort, _sync_worktree_reconciling_divergence, _emit_empty_contract_hitl, _check_brc_progress_gate, _latest_active_role_heartbeat, _unresolved_contract_hitl_ids, _publish_consensus_timeout_alert, _emit_producer_death_alert, detect_branch_divergence, _check_branch_divergence_for_alert, _publish_branch_divergence_alert, _branch_divergence_tick, _handle_brc_consensus_timeout) into _alerts.py (1270 lines). Patched barrel globals reached via _pkg; WorktreeSyncOutcome imported from the sibling _worktree_sync. AST-precise. Barrel 24640 -> 23420. Verified: ruff+format clean, ratchet exit 0, 7612 collect (0 errors), divergence/timeout/gate suites green (259 passed); the 5 reconcile failures are the known git-init sandbox-env class, not split-induced. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…3312 slice-4) Move the 13 overseer spawn/detection/corrective helpers (_spawn_overseer_agent through _teardown_phase_overseer, ~730 lines) out of the routes/pipelines barrel into a private _overseer.py submodule, per the landed slice-15 (routes/signals/) convention: barrel-resident and test-patched globals are reached via 'import routes.pipelines as _pkg', and the barrel re-exports every moved symbol so patch("routes.pipelines.<name>") keeps intercepting. Bodies are verbatim (only free barrel-global references gained a _pkg. prefix). Now-unused forward-ref TYPE_CHECKING imports (CorrectiveExecutor/AdjudicationVerdict/SpawnedContainer) moved with the functions and were dropped from the barrel. Barrel: 23,436 -> 22,703 lines. _overseer.py: 740 lines / 30KB (under both caps). make lint clean; 284 overseer + 124 pipeline/import-seam tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…slice-4)
Move the 15 slice-lifecycle helpers (slice worktree/base-branch resolution,
DAG-mode check, peer-consensus lookup, non-complete-slice classification, the
layer-C / corrupt / blocked HITL escalations, cross-repo-hold marker/resolution/
registration, and slice-evidence reachability) out of the routes/pipelines barrel
into a private _slice_state.py, per the landed routes/signals convention: barrel
globals reached via 'import routes.pipelines as _pkg'; the barrel re-exports every
moved symbol so patch("routes.pipelines.<name>") keeps intercepting. Bodies are
verbatim (only free barrel-global refs gained a _pkg. prefix; Literal stays bare
and is imported from typing so its string args keep ruff's forward-ref special-
casing). Cross-repo-hold module constants stay barrel-resident.
Barrel: 22,703 -> 21,646 lines. _slice_state.py: 1,094 lines / under both caps.
make lint clean; 244 slice-state + cross-importer tests pass (the 2 git-init and
34 gateway_client 'errors' are the pre-existing sandbox git-policy env class).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…3312 slice-4) Move the 5 driver-thread helpers (_spawn_pipeline_run_thread, has_live_pipeline_driver, relaunch_driverless_running_pipelines, _broadcast_orphaned_driver_alert, maybe_revive_orphaned_awaiting_human_driver) out of the routes/pipelines barrel into a private _drivers.py, per the landed routes/signals convention: barrel globals via 'import routes.pipelines as _pkg'; the barrel re-exports every moved symbol so patch("routes.pipelines.<name>") keeps intercepting. Bodies verbatim (free barrel-global refs gained _pkg. prefix). Barrel: 21,646 -> 21,412 lines. _drivers.py: 259 lines. make lint clean; 125 driver/relaunch/revival tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…decisions.py (#3312 slice-4) Move the 7 decision helpers (_format_nack_summary, _incomplete_consensus_decision_text, _persist_hitl_decision, _cancel_consensus_timeout_decisions, _find_pending_divergence_reconcile_decision, _divergence_reconcile_is_abort, _divergence_reconcile_hitl_question) out of the routes/pipelines barrel into a private _decisions.py, per the landed routes/signals convention: barrel globals via 'import routes.pipelines as _pkg'; the barrel re-exports every moved symbol so patch("routes.pipelines._persist_hitl_decision") and the divergence-reconcile seams keep intercepting (they are also reached from _alerts.py via _pkg). Bodies verbatim (free barrel-global refs gained _pkg. prefix); the _DIVERGENCE_RECONCILE_* / _CONSENSUS_TIMEOUT_HITL_CONTEXT module constants stay barrel-resident. Barrel: 21,412 -> 21,176 lines. _decisions.py: 253 lines. make lint clean; 112 consensus-polling / nack-iteration / routes tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Durable continuation state for the incremental pipelines.py decomposition (operator directive cq-3): current barrel/submodule inventory, the reusable AST extraction tool, the routes/signals extraction convention, the remaining cluster + giant-function-split plan, and the known sandbox env-failure class. The tool (.egg-state scratch, NOT a code deliverable) is deleted at the terminal commit before propose. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
slice-4) Move the 5 live-pod helpers (_get_spawner, _count_live_pods_for_pipeline, _live_event_agents, _slice_agents_alive, _guard_live_pods_or_force) out of the routes/pipelines barrel into a private _pod_liveness.py, per the landed routes/signals convention: barrel globals via 'import routes.pipelines as _pkg'; the barrel re-exports every moved symbol so patch("routes.pipelines.<name>") keeps intercepting. get_container_spawner (a patch target) + get_kubernetes_spawner stay imported in the barrel (now reached via _pkg from _pod_liveness) with an explicit noqa: F401 marking them retained for re-export / patch seam. Barrel: 21,160 -> 20,961 lines. _pod_liveness.py: 224 lines. make lint clean; 134 pipeline-routes / slice-run-loop tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ted remaining-work map (#3312) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ines/_ledger.py (#3312 slice-4) Move the 14 ledger/gap/apply helpers (_sync_pipeline_decisions_to_contract, the _ledger_attestation_* family, _handle_explicit_none_attestation_gate, _find_explicit_none_attestation, _collect_decision_ledger_status, _queue_and_await_contract_decisions, _await_unresolved_gap_gate, _next_phases_for_epic, _drain_wontdo_batch_after_apply, _write_apply_phase_handoff, _persist_phase_gate_resolution) out of the routes/pipelines barrel into a private _ledger.py, per the landed routes/signals convention: barrel globals via 'import routes.pipelines as _pkg'; the barrel re-exports every moved symbol so patch("routes.pipelines.<name>") keeps intercepting. HITLDecision (non-patched models type used only in _ledger annotations) stays imported in the barrel with noqa: F401 so _pkg.HITLDecision resolves. Also fixed the extraction tool: ast col_offset is a UTF-8 BYTE offset, so the _pkg. insertion now slices on encoded bytes (lines with em-dashes / curly quotes were shifting the insert into the middle of an identifier). Barrel: 20,949 -> 19,646 lines. _ledger.py: 1,346 lines. make lint clean; 231 contract-decision/gap/apply tests pass (4 git-init 'failures' are the pre-existing sandbox git-policy env class). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ines/_populate.py (#3312 slice-4) Move the 21 populate symbols — the 6 Populate*/PlanDraftMissing*/SliceGateMonolithicBlock classes (StrEnum + NamedTuple + exceptions) and 15 functions (_synthesize_plan_draft, _slice_gate_block_monolithic_demotion, _populate_contract_from_plan(_safe), _auto_populate_contract_at_implement_start, _merge_preserved_slice_runtime, _enforce_implement_start_plan_preflight, _origin_has_plan_draft, the _empty_contract_* and _plan_preflight_* / _forest_error_to_outcome helpers) — out of the barrel into private _populate.py, per the landed routes/signals convention. Non-mechanical bits: - Relocated the module-level _FOREST_REASON_TO_OUTCOME table into _populate.py: it references the PopulateOutcome enum at DEFINITION time, so leaving it in the barrel (evaluated before the bottom re-export) raised NameError at import. It re-exports through the barrel so _pkg._FOREST_REASON_TO_OUTCOME still resolves. - Barrel keeps NamedTuple imported (noqa: F401) — _populate's class PopulateResult( _pkg.NamedTuple) reaches it via _pkg at class creation; verified _fields/enum members intact. ContractSlice forward-ref moved to _populate's TYPE_CHECKING block. Barrel: 19,646 -> 18,210 lines. _populate.py: 1,460 lines / under both caps. make lint clean; 946 populate/plan tests pass + 16,137 tests collect with 0 import errors (the 4 git-init 'failures' are the pre-existing sandbox git-policy env class). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…eview.py (#3312 slice-4) Move 7 prompt-building helpers (_summarize_issue, _extract_plan_overview, _build_role_context, _build_role_restrictions_section, _build_impasse_escape_hatch_section, _render_contract_tasks, _build_review_prompt) out of the barrel into private _prompt_review.py, per the landed routes/signals convention. First slice of the large prompt-building region; the agent-prompt builders (_build_phase_prompt/_build_brc_preamble/_build_agent_prompt et al.) remain for a follow-up split. Barrel: 18,210 -> 17,480 lines. _prompt_review.py: 759 lines. make lint clean; 552 review-prompt / reviewer tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…slice-4) Move _build_agent_prompt (the ~1,300-line agent-prompt assembler, a patch target) and _build_file_boundary_section out of the barrel into private _prompt_agent.py, per the landed routes/signals convention. Second slice of the prompt-building region. Barrel: 17,480 -> 16,140 lines. _prompt_agent.py: 1,343 lines / under both caps. make lint clean; 697 agent-prompt / file-boundary tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…prompt_phase.py + _prompt_reviewer.py (#3312 slice-4) Finish the prompt-building region: - _prompt_phase.py (1,403L): _build_phase_prompt, _contract_enforcer_role_names, _build_brc_preamble. - _prompt_reviewer.py (590L): _build_agent_roster, _build_reviewer_preparation, _re_review_priming_block, _build_producer_orientation. Per the landed routes/signals convention; barrel re-exports every moved symbol. Mechanical test update (in-slice, task-4-6): test_pipelines_origin_main_parameterization ::test_helper_has_multiple_call_sites counted _resolve_origin_ref( call sites via inspect.getsource(barrel); the callers now live across package submodules (reached via _pkg.), so it scans the whole routes/pipelines/ package dir instead. 5 call sites remain. Barrel: 16,140 -> 14,185 lines. make lint clean; 16 parameterization + prompt tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#3312 slice-4) Tool fix: ast col_offset is a UTF-8 byte offset -> _pkg. insertion now slices on encoded bytes (em-dash/curly-quote lines were splicing _pkg. mid-identifier). Memory: 19 submodules landed (through the prompt-building + populate + ledger region); records the constant-referencing-moved-enum, NamedTuple/StrEnum-base, retained-seam-import, and source-introspection-test gotchas. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…pipelines submodules (#3312 slice-4) Three cohesive helper clusters out of the barrel, per the landed routes/signals convention: - _status_wait.py (142L): _track_host_wait_start/_end (mutate the barrel-resident _inflight_host_waits patch-target set via _pkg.), _parse/_build_status_wait_cursor, _message_store_tip_id, _build_minimal_status_envelope. - _resolve.py (188L): _ensure_pipeline_work_ref, _slice_namespace_root, _pipeline_identifier, _brc_history_identifier, _emit_pipeline_event, _resolve_pipeline (patch target), _collect_all_pipelines. - _salvage.py (69L): _filter_salvage_worktrees, _serialize_commit_report, _serialize_salvage_result. Barrel re-exports every moved symbol so patch("routes.pipelines.<name>") keeps resolving. Barrel: 14,167 -> 13,845 lines. make lint clean; 1,599 tests pass (the 5 failures are the pre-existing sandbox git-init env class + one health-probe context flake that also fails at HEAD with these changes stashed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…pers into pipelines submodules (#3312 slice-4) Four cohesive clusters out of the barrel, per the landed routes/signals convention: - _hitl_rerun.py (333L): _build_phase_iteration_context, _build_iteration_summary_from_tracker, _apply_inline_hitl_kickback_to_phase, _broadcast_hitl_nonconvergence_alert, _perform_hitl_phase_rerun. - _status_view.py (388L): _get_pr_info, _consensus_block, _get_concurrent_status, _build_slice_diff_summary. - _first_principles.py (240L): apply_first_principles_redirect, _restart_refine_phase. - _stacked_pr.py (251L): _start_stacked_pr_reconciler. Barrel re-exports every moved symbol; patch seams preserved. Barrel bookkeeping: PhaseExecution kept imported (noqa) — reached via _pkg.PhaseExecution in _hitl_rerun annotations; the barrel's now-dead ContainerSpawner TYPE_CHECKING import dropped (submodules that need it carry their own TYPE_CHECKING block). Barrel: 13,845 -> 12,650 lines. make lint clean; 1,115 tests pass (the 2 git-init + 4 slice-diff-summary 'errors' are the pre-existing sandbox git-policy env class). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… into pipelines submodules (#3312 slice-4) Three cohesive clusters out of the barrel, per the landed routes/signals convention: - _slice_completion.py (133L): SliceCompletionInvariantError, _slice_produced_commits, _validate_slice_completion_basis. - _lifecycle_helpers.py (333L): _normalize_submission_repos, _assert_repo_set_uniform, _compute_gateway_mode, _cleanup_remote_branches, _clear_pipeline_runtime_state, _mark_pipeline_records_terminated. - _run_support.py (369L): _spawn_and_wait, _parse_resolution, _clear_stale_impasses_for_producers, _pipeline_superseded_by_restart. Barrel bookkeeping: the core models/state_store re-export imports (ContainerStatus, StateStore, load/save_agent_output, ...) reached via _pkg. from submodules kept with noqa; the barrel's now-dead MountSpec/ContractAgentRole TYPE_CHECKING imports (moved to _run_support's own block) + the orphaned TYPE_CHECKING import dropped. Tool fix: _local_bound_names now recognizes function-local import bindings, so a local 'from models import ContainerInfo' inside a moved fn keeps its references bare instead of being _pkg.-rewritten (prevents orphaned local imports; matters for the giants). Barrel: 12,650 -> 11,853 lines. make lint clean; 350 tests pass + 16,179 collect with 0 import errors (the 2 git-init failures are the pre-existing sandbox env class). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…oute-bodies + 3 giants remain (#3312 slice-4) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…bmodules (decision-8) (#3312 slice-4) The 16 route handlers kept their @pipelines_bp.route (+ @require_lifecycle_secret) decorators and signatures in the barrel as THIN WRAPPERS delegating to a _<name>_body fn in a submodule (decision-8), grouped cohesively: - _routes_read.py (list_pipelines, get_pipeline) - _routes_crud.py (create_pipeline, update_pipeline, update_pipeline_config, delete_pipeline) - _routes_restart.py (restart_agent, restart_phase) - _routes_status.py (get_pipeline_status, wait_pipeline_status, get_pipeline_visualization) - _routes_lifecycle.py (list_pipeline_local_commits, salvage_pipeline_local_commits, start_pipeline) - _routes_stream.py (stream_all_pipelines, stream_pipeline) The wrapper calls the body BARE (barrel IS _pkg; body re-exported into barrel namespace, resolved at call time) so patch("routes.pipelines._<name>_body") + the route-name seams resolve. Barrel imports now used only by moved bodies kept with noqa (reached via _pkg.). Tool: added a --routes mode (keeps decorator+signature+thin wrapper in barrel, moves body). Barrel: 11,853 -> 8,364 lines. All _routes_* under caps (largest _routes_restart 1,057L). make lint clean; 283 route-handler tests pass + 16,197 collect with 0 import errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rel 8,364L, only 3 giants remain (#3312 slice-4) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… internal splits (#3312 slice-4) Analysis this invocation: verified via symtable/AST that the giants' nested closures' enclosing-local captures are NOT reassigned after closure definition (only _run_pipeline has one nonlocal), so lifting a closure to a module-level fn + functools.partial(captured) is behaviour-EXACT (early-bind == late-bind when the var never changes), keeping all call sites verbatim. Recorded the concrete per-giant recipe (starting with the smallest, _run_implement_phase_slices: lift _commit_and_push_slice_statefiles + _persist_slice_status_complete). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ion next invocation (#3312 slice-4) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e_setup.py (#3312 slice-4, task-4-3) GIANT #3 bite 5 (last setup block). _run_pipeline 2,797 -> 2,711L. The 'if worktree_repo_path != repo_path:' block (resolve per-pipeline worktree repo path + reconcile a stale worktree) moves verbatim to _resolve_worktree_repo via the (pipeline, done) convention. current_phase is assigned-and-used within the block so it stays a helper-local (no F841; dead after the block since the loop recomputes it). All 5 setup blocks now extracted; _run_pipeline_setup.py = 727L (under cap). Pure refactor. ruff clean; import OK; tests pass (only the 2 documented pre-existing recover_advance_clear failures remain). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#3312 slice-4, task-4-3) GIANT #3 bite 6. _run_pipeline 2,711 -> 2,684L. Lifts the 2 health-monitor closures (_on_health_escalation, _health_monitor_poll) to module-level *_impl helpers bound via functools.partial (call sites — on_escalation registration + poll Thread target — unchanged; captured locals never reassigned after the closure def, so partial early-binding is exact; divergence_alerted_shas passed by ref stays mutated-in-place). New _run_pipeline_support.py (62L). _make_overseer_teardown_hook stays nested (its phase_overseer_active nonlocal is documented-intentional; boxing it would touch every use-site for 37L, deferred). Pure refactor. ruff clean; import OK; tests pass (only the 2 documented pre-existing recover_advance_clear failures remain). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… -> _run_hitl_gate.py (#3312 slice-4, task-4-3, non-negotiable #7) GIANT #3 bite 7 — first while-loop per-phase-handler extraction. _run_pipeline 2,684 -> 1,988L. The refine/plan HITL-gate converge-before-advance block ('if current_phase.value in _HITL_GATE_PHASES and not pipeline.config.hitl_gates:') moves verbatim to _run_hitl_gate_converge (722L). Its 4 outer-while 'continue's are threaded through a returned action signal: the helper returns (pipeline, action) and the thin loop does 'if action == "continue": continue' — behaviour-exact (the block owns only continues, no break/return; pipeline is the sole threaded output). Pure refactor. Adds _run_hitl_gate_converge to the test's _EXTRACTED_HELPERS tuple so the _commit_statefiles call-site coverage assertion follows the moved call. ruff clean; import OK; 171 seam tests pass (test_consensus_polling, test_conditional_ack_hitl_gate, plan-exit, advance_phase); only the 2 documented pre-existing recover_advance_clear failures remain. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… _run_phase.py (#3312 slice-4, task-4-3, non-negotiable #7) GIANT #3 bite 8. _run_pipeline 1,988 -> 1,712L. The phase-execution 'if True: while ...:' block (spawn agents / run the BRC inner review cycle) moves verbatim to _run_phase_execution (331L). The block's 4 breaks belong to its OWN inner while (they stay verbatim); only the single escaping bare return is threaded through a 2-state action signal — the helper returns (pipeline, phase_execution, phase_failed, action) and the thin loop does 'if action == "return": return' (the '"break"' arm is dormant — no outer-while break in this block). pipeline/phase_execution/phase_failed are pre-init'd and threaded in+out; the now-dead tester_gap_summary pre-init is dropped from the giant (0 loads; the helper re-inits its own). Pure refactor. ruff clean; import OK; 95 seam tests pass (test_consensus_polling, test_slice_run_loop_integration); only 2 documented pre-existing failures. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… -> _run_phase_blocks.py; giant now UNDER cap (#3312 slice-4, task-4-3, non-negotiable #7) GIANT #3 bite 9. _run_pipeline 1,712 -> 1,467L (UNDER the 1,500 cap). Extracts 3 while-loop per-phase blocks verbatim into _run_phase_blocks.py (338L): _run_plan_advance (plan populate/advance; its single outer-while break threaded via a 'break' action signal; phase_overseer_active bool threaded in+out), _run_pending_phase_init and _run_implement_advance (both pure fall-through, no control flow — just threaded I/O). Adds the 3 helpers to the test's _EXTRACTED_HELPERS so the _commit_statefiles call-site coverage assertion follows the moved call. Pure refactor. ruff clean; import OK; 100 seam tests pass (test_consensus_polling, test_slice_run_loop_integration, plan-exit); only the 2 documented pre-existing recover_advance_clear failures remain. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#3312 slice-4, task-4-3/4-4) Final giant extraction: _run_pipeline (1,483L) leaves the barrel, which drops to 1,466L (under the 1500 hard cap). All 16 orphaned barrel imports are genuine _pkg. seam references (verified: each has >=2 _pkg.<name> uses in submodules) -> retained with noqa F401, none deleted. _run_pipeline re-exported through the barrel so patch("routes.pipelines._run_pipeline") and _pkg._run_pipeline (drivers) resolve unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…3312 slice-4, task-4-5/4-6) The file-size decomposition program is COMPLETE. All 4 remaining giants (models.py #3450, event_loop.py #3447, gateway.py + pipelines.py #3312) are decomposed; scripts/file-size-allowlist.yaml's files: map is now EMPTY (terminal acceptance criterion) and check-file-sizes.py exits 0. - Drop the last allowlist entry (pipelines/__init__.py, 1,466L, under cap). - Fix 3 source-introspection seam tests broken by the _run_pipeline move (strip the _pkg. decomposition prefix; introspect _start_pipeline_body vs the thin @route wrapper) + a stale ruff-format line in test_ble001. - Delete the scratch extraction tool. NOTE: the routes/pipelines/ seam row for orchestrator/CLAUDE.md is a documenter-owned restricted path; the documenter authors it separately. No Dockerfile change (routes/ ships via recursive COPY). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rator/CLAUDE.md (#3312) Adds the concrete routes/pipelines/ submodule-layout seam row (task-4-5): the 46-submodule package, the decision-8 route-decorators-in-barrel convention, the _run_pipeline per-phase split (non-negotiable #7), packaging-neutral recursive COPY, and the terminal criterion — pipelines.py was the LAST allowlist entry, so scripts/file-size-allowlist.yaml's files: map is now EMPTY. Closes #3312. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![james-in-a-box[bot]](https://avatars.githubusercontent.com/u/8340608?s=60&v=4){kind=small}
Contributor
Author
There was a problem hiding this comment.
Contract Verification — PR #3512 ([issue-3312][slice-4/4] decompose orchestrator/routes/pipelines.py)
Verdict: Approve — every slice-4 acceptance criterion is satisfied, the PR is cleanly scoped, and the file-size decomposition program's terminal criterion (empty allowlist) is met. One non-blocking advisory below.
Scope check (clean)
PR base is egg/issue-3312-v2/slice-3, so the diff is scoped to slice-4 only: 47 modules under orchestrator/routes/pipelines/, the removed orchestrator/routes/pipelines.py, orchestrator/CLAUDE.md, scripts/file-size-allowlist.yaml, 9 patch-path-rewrite test files, and 4 .egg-state/ BRC artifacts. No cross-contamination with the prior slices' targets (models/, event_loop/, gateway/gateway/).
Task-by-task verification
- task-4-1 (external-importer audit / re-export inventory) — Verified by outcome: of every symbol referenced externally via
from routes.pipelines import …andpatch("routes.pipelines.…")across the repo, all resolve through the barrel except one non-existent symbol (see advisory). CLAUDE.md records the ~137-file back-compat surface / ~57 patch targets. ✅ - task-4-2 (pure-move baseline) —
git mvbaseline is present in the 57-commit history; structure is a clean move→extract split. (CI owns themake test-allgreen gate — not re-run per review policy.) ✅ - task-4-3 (
_run_pipelinesplit) —_run_pipeline.py(1,483 L) is a thin loop delegating to per-phase handlers (_pkg._run_phase_execution,_pkg._run_implement_advance,_pkg._run_hitl_gate_converge); split modules_run_phase*.py,_run_implement*.py,_run_concurrent*.py,_run_hitl_gate.py,_run_pipeline_setup.pypresent. Transition ordering preserved (pure refactor). ✅ - task-4-4 (cluster extraction / caps / route convention) — All 47 modules under both caps (largest
_run_implement.py1,496 L / 80,784 B; all ≤1,500 L and ≤100 KB, confirmed byscripts/check-file-sizes.py). All 16@pipelines_bp.routedecorators live in__init__.py, zero in submodules. Barrel does explicit per-submodule re-exports (46from ._x import …).pipelines/ships under the recursiveCOPY orchestrator/routes/ ./routes/(Dockerfile:45) — no Dockerfile change needed. ✅ - task-4-5 (drop last allowlist entry / CLAUDE.md) —
scripts/file-size-allowlist.yamlfiles: {}is empty (terminal criterion).orchestrator/CLAUDE.mdcarries a concreteroutes/pipelines/seam section with a full per-submodule table incl. the_run_pipelinestate-machine layout. Ratchet passes with the empty allowlist (check-file-sizes.pyexit 0; only non-fatal soft-cap warnings). ✅ - task-4-6 (mechanical patch-path rewrites / program completion) — The 9 changed test files are pure import-path rewrites (old single-file
pipelines.py→ newpipelines/package glob; e.g.test_slice_loop_import_seam.pynowrglobs the package). All 47 submodules parse cleanly (AST). All four program targets (models,event_loop,gateway/gateway,routes/pipelines) are decomposed and the allowlist is empty — the program (#3312) is complete. ✅
Advisory (non-blocking, pre-existing, out of scope)
orchestrator/tests/test_overseer_max_turns.py does from routes.pipelines import _check_and_respawn_overseer and calls it — but that symbol is defined nowhere in the repo, and sibling tests (test_overseer_lifecycle.py, test_overseer_repo_resolution.py) explicitly assert its absence as an "integration sentinel". This is not a regression from this PR: on the base branch (slice-3) the symbol is already undefined repo-wide and test_overseer_max_turns.py imports/calls it identically, and this PR modifies none of the three overseer test files. It belongs to a separate in-flight overseer-lifecycle work stream (the "task-5-1" folding those tests describe), not to the pipelines decomposition. Flagging for the human reviewer to route to that work stream; it does not block this slice.
Verification limitations
- The orchestrator is unreachable this session, so
egg-contract verify-criterioncould not be run to record machine-readable verification. - The contract's top-level
acceptance_criterialist is empty ([]); verification here maps to each task's per-taskacceptance_criteria, all of which are met as detailed above. - Per review policy I did not run
make test/make test-all; the green gate is CI's responsibility on this PR HEAD.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
Thanks for the review — the verdict is Approve with a single non-blocking advisory. Per-item disposition below. Advisory:
|
This comment has been minimized.
This comment has been minimized.
#3496 arms-exhausted withdrawal into pipelines/ package
The #3312 slice-4 split moved the plan-complete branch into _run_plan_advance and the start_phase=implement safety net into _start_phase_setup, and inspect.getsource(pipelines) now returns only the barrel. Follow the tokens per the tests' own fragility notes and strip the _pkg. indirection so the pre-split token assertions keep pinning the same code shapes.
The gateway/gateway/ split changed mypy's import resolution: narrow the now-partially-unused type-ignore comments to no-redef, cover the agent_restrictions and jira_adf fallback imports, and type the space-cache lookups so no-any-return passes. Point the hardcoded-ports allowlist at the gateway/gateway/ package (was gateway/gateway.py) and ruff-format the reset_exhausted log call.
Contributor
Author
There was a problem hiding this comment.
Contract re-verification — slice-4 (routes/pipelines.py decomposition), commit 79dc416
Incremental re-review of the delta since my prior verification at 046d426. No contract violations; all slice-4 acceptance criteria remain satisfied. (PR already merged — posting as a comment for the record.)
Delta reviewed (3 code commits since baseline)
6194ad5— merge main + port #3490 (live-mutable consensus timeouts) and #3496 (arms-exhausted HITL withdrawal) into the decomposed package. Faithful port:_MUTABLE_CONFIG_KEYSwidened with theconsensus_timeout_minutes*family in the barrel; live re-resolve gate added to_run_concurrent.py::_run_concurrent_phase;_withdraw_arms_exhausted_decisionsadded to_decisions.pyand re-exported via the barrel, consumed byconcurrent_executor.py:1246and patched atroutes.pipelines._withdraw_arms_exhausted_decisionsin tests — exactly the "patch targets resolve through the barrel" contract. Covered by newtest_consensus_timeout_live_widening.py/test_arms_exhausted_livelock.py.6b90220— source-introspection tests re-pointed at the package (_pipelines_package_source()concatenates module sources now thatinspect.getsource(pipelines)returns only the barrel). Mechanical, no behavior change.79dc416— lint fallout only:# type: ignorecomment adjustments, one log-string join, an unusedimport inspect, andcheck-hardcoded-ports.pyupdated to thegateway/gateway/package path. Zero behavior change.
End-state acceptance checks (re-verified at HEAD)
orchestrator/routes/pipelines.pymonolith fully decomposed →pipelines/(46 submodules + barrel). Largest_run_implement.pyat 1,496 lines / 80 KB — under both the 1,500-line and 100 KB caps; every submodule passes.scripts/check-file-sizes.pyexits 0 (soft-cap warnings are advisory only). The #3312 program's allowlist entries are gone; the sole remainingfiles:entry isconcurrent_executor.py(#3498) — a separate, later decomposition, not a slice-4 regression._run_pipelineis a thin loop delegating to per-phase handlers; transition ordering preserved.orchestrator/CLAUDE.mddocuments the concretepipelines/+_run_pipelinesubmodule layout and the terminal empty-allowlist criterion.- No stale imports of a monolithic
pipelines.pyfile (residual matches are docstring prose only).
Non-blocking advisory nits
orchestrator/CLAUDE.md:277lists_run_concurrent.pyat 1,439 lines; the #3490 port in this PR grew it to 1,476 (still under cap) — doc number is now stale.orchestrator/routes/pipelines/__init__.py:1395has a duplicated# noqa: E402,F401comment (from ._run_pipeline_setup import (...)).
Verdict: approve — contract-compliant, no regressions.
— Authored by egg
Contributor
Author
|
egg contract-verification completed. View run logs 3 previous review(s) hidden. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Decompose
orchestrator/routes/pipelines.py(30,520 lines / 1.44 MB) into a sub-package; drop the LAST allowlist entry (files: map EMPTY); seam coverage in orchestrator/CLAUDE.md. Depends on the previous slice (single linear chain). Closes #3312.Base PR: #3489
What's in this PR
Commits (57):
This slice
Decompose orchestrator/routes/pipelines.py (30,520 lines, STRUCTURAL OUTLIER, OVER BYTE CAP): _run_pipeline split (#3312 slice-19 equivalent, non-negotiable #7)
Files affected:
orchestrator/routes/pipelines.pyorchestrator/routes/pipelines/__init__.pyorchestrator/routes/pipelines/scripts/file-size-allowlist.yamlorchestrator/CLAUDE.mdorchestrator/tests/tests/Tasks (6) + acceptance criteria
COPY orchestrator/routes/ ./routes/(orchestrator/Dockerfile:45); confirm by grounding.Stack
issue-3312-v2egg/issue-3312-v2/slice-3