From e6669ebf5ca23ad644f68f9494d9c1803ac04f24 Mon Sep 17 00:00:00 2001 From: ProtocolWarden <32967198+ProtocolWarden@users.noreply.github.com> Date: Thu, 18 Jun 2026 05:37:00 -0400 Subject: [PATCH] wire(switchboard): demote on p95 tail latency; drop plan_routes + p50 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Coupled lane/routing-subsystem change from the incomplete-integration remediation: - WIRE p95: AdjustmentEngine._evaluate gated the latency-demote on the MEAN latency, but ProfileSignals.p95_latency_ms was computed and never read. Gate on p95 instead — a backend whose typical request is fast but whose slow tail is consistently bad still degrades UX, and the mean hides it. Mean is reported alongside for operator context (stays used). Adds a low-mean/high-p95 test. - DELETE LaneSelector.plan_routes: redundant wrapper; production constructs DecisionPlanner.plan directly (app.py:49, api/routes_routing.py:62). Rewire the RoutingPlanDemote tests to DecisionPlanner.plan; delete the dedicated delegation tests; drop the now-unused RoutingPlan import. - DELETE ProfileSignals.p50_latency_ms: computed but read nowhere; remove it + its tests + the now-unused `median` import. 345 tests green; ruff(src+test) + ty + docs + audit(B2 only) + doctor clean. Co-Authored-By: Claude Opus 4.8 --- .console/log.md | 15 +++++++++++++ src/switchboard/lane/engine.py | 15 ------------- src/switchboard/services/adjustment_engine.py | 14 +++++++++---- src/switchboard/services/signal_aggregator.py | 8 +------ test/unit/test_adjustment_engine.py | 12 +++++++++++ test/unit/test_lane_engine_demote.py | 9 ++++---- test/unit/test_lane_planner.py | 21 ------------------- test/unit/test_signal_aggregator.py | 8 ++----- 8 files changed, 45 insertions(+), 57 deletions(-) diff --git a/.console/log.md b/.console/log.md index e81720f..39038d0 100644 --- a/.console/log.md +++ b/.console/log.md @@ -90,3 +90,18 @@ Added metadata: dict[str, str] to LaneDecision and wired engine.select() to alwa metadata["worker_backend"] based on selected_lane (codex_cli → "codex_cli", else "claude_code"). Downstream OC adapters can read this to configure TeamExecutor/DAGExecutor/CritiqueExecutor without re-deriving backend from lane enum. 347 tests pass. + +## 2026-06-18 — wire p95 into demote heuristic; drop plan_routes + p50 + +Ecosystem incomplete-integration remediation (combined, coupled change): +- WIRE: AdjustmentEngine._evaluate now gates the latency-demote on p95 (tail) + latency instead of mean — a backend with a fast typical request but a bad slow + tail still degrades UX, and the mean hid that. mean is reported alongside for + operator context (stays used). New test: low-mean/high-p95 tail now demotes. +- DELETE LaneSelector.plan_routes — redundant wrapper; prod constructs + DecisionPlanner.plan directly (app.py / routes_routing.py). Reworked the + RoutingPlanDemote tests to call DecisionPlanner.plan; deleted the dedicated + delegation tests. Removed the now-unused RoutingPlan import. +- DELETE ProfileSignals.p50_latency_ms — computed but read nowhere (p95 is the + signal now). Removed its tests + the unused `median` import. +345 tests green; ruff(src+test) + ty + docs + audit(B2 env only) + doctor clean. diff --git a/src/switchboard/lane/engine.py b/src/switchboard/lane/engine.py index b66d4b6..743e2e7 100644 --- a/src/switchboard/lane/engine.py +++ b/src/switchboard/lane/engine.py @@ -38,7 +38,6 @@ from .defaults import DEFAULT_POLICY from .explain import DecisionExplanation, DecisionFactor from .policy import LaneRoutingPolicy -from .routing import RoutingPlan logger = logging.getLogger(__name__) @@ -160,20 +159,6 @@ def explain(self, proposal: TaskProposal) -> DecisionExplanation: summary=summary, ) - def plan_routes(self, proposal: TaskProposal) -> RoutingPlan: - """Return a full RoutingPlan including fallback and escalation alternatives. - - Delegates to DecisionPlanner, which uses the same policy as this selector. - Use this when callers need the complete picture of available alternatives, - not just the primary route. - """ - from .planner import DecisionPlanner - planner = DecisionPlanner( - policy=self._policy, - adjustment_query=self._adjustment_query, - ) - return planner.plan(proposal) - def validate_policy(self) -> list[str]: """Return a list of policy validation issues (empty = valid). diff --git a/src/switchboard/services/adjustment_engine.py b/src/switchboard/services/adjustment_engine.py index 46a66b1..568afb4 100644 --- a/src/switchboard/services/adjustment_engine.py +++ b/src/switchboard/services/adjustment_engine.py @@ -57,14 +57,20 @@ def _evaluate(self, sig: ProfileSignals) -> PolicyAdjustment: f"exceeds threshold ({_DEMOTE_ERROR_RATE:.0%})" ), ) - mean_lat = sig.mean_latency_ms - if mean_lat is not None and mean_lat >= _DEMOTE_LATENCY_MS: + # Gate on p95 (tail) latency, not the mean: a backend whose typical + # request is fast but whose slow tail is consistently bad still + # degrades the user experience, and the mean hides that. Mean is + # reported alongside for operator context. + p95_lat = sig.p95_latency_ms + if p95_lat is not None and p95_lat >= _DEMOTE_LATENCY_MS: + mean_lat = sig.mean_latency_ms + mean_note = f", mean {mean_lat:.0f} ms" if mean_lat is not None else "" return PolicyAdjustment( profile=sig.profile, action="demote", reason=( - f"mean latency {mean_lat:.0f} ms over {sig.total_requests} requests " - f"exceeds threshold ({_DEMOTE_LATENCY_MS:.0f} ms)" + f"p95 latency {p95_lat:.0f} ms{mean_note} over {sig.total_requests} " + f"requests exceeds threshold ({_DEMOTE_LATENCY_MS:.0f} ms)" ), ) diff --git a/src/switchboard/services/signal_aggregator.py b/src/switchboard/services/signal_aggregator.py index 3174cdc..c506e38 100644 --- a/src/switchboard/services/signal_aggregator.py +++ b/src/switchboard/services/signal_aggregator.py @@ -5,7 +5,7 @@ from __future__ import annotations from dataclasses import dataclass, field -from statistics import mean, median +from statistics import mean from typing import TYPE_CHECKING if TYPE_CHECKING: @@ -33,12 +33,6 @@ def mean_latency_ms(self) -> float | None: return None return mean(self._latencies_ms) - @property - def p50_latency_ms(self) -> float | None: - if not self._latencies_ms: - return None - return median(sorted(self._latencies_ms)) - @property def p95_latency_ms(self) -> float | None: if not self._latencies_ms: diff --git a/test/unit/test_adjustment_engine.py b/test/unit/test_adjustment_engine.py index 08538f3..a7fcef9 100644 --- a/test/unit/test_adjustment_engine.py +++ b/test/unit/test_adjustment_engine.py @@ -74,6 +74,18 @@ def test_demotes_on_high_latency(self) -> None: assert adj.action == "demote" assert "latency" in adj.reason + def test_demotes_on_bad_tail_latency_even_with_low_mean(self) -> None: + # 90 fast requests + 10 very slow ones: the mean stays well under the + # threshold but the p95 tail is far over it. Gating on p95 (not mean) + # catches tail degradation the mean would have hidden. + latencies = [100.0] * 90 + [20_000.0] * 10 + sig = _sig(total=100, latencies=latencies) + assert sig.mean_latency_ms is not None and sig.mean_latency_ms < _DEMOTE_LATENCY_MS + assert sig.p95_latency_ms is not None and sig.p95_latency_ms >= _DEMOTE_LATENCY_MS + adj = self.engine._evaluate(sig) + assert adj.action == "demote" + assert "p95 latency" in adj.reason + def test_error_rate_takes_priority_over_latency(self) -> None: errors = int(_DEMOTE_MIN_REQUESTS * _DEMOTE_ERROR_RATE) + 1 sig = _sig( diff --git a/test/unit/test_lane_engine_demote.py b/test/unit/test_lane_engine_demote.py index 3f9b5be..79dbe4f 100644 --- a/test/unit/test_lane_engine_demote.py +++ b/test/unit/test_lane_engine_demote.py @@ -13,6 +13,7 @@ TaskType, ) from switchboard.lane.engine import LaneSelector +from switchboard.lane.planner import DecisionPlanner from switchboard.lane.policy import ( FallbackPolicy, LaneRoutingPolicy, @@ -146,8 +147,8 @@ def boom(lane): class TestRoutingPlanDemote: def test_no_query_no_deprioritized(self): """Without a query, no candidate gets DEPRIORITIZED via this path.""" - sel = LaneSelector(policy=_two_rule_policy()) - plan = sel.plan_routes(_proposal()) + planner = DecisionPlanner(policy=_two_rule_policy()) + plan = planner.plan(_proposal()) for cand in plan.fallbacks.candidates + plan.escalations.candidates: assert cand.eligibility_status != EligibilityStatus.DEPRIORITIZED or "[health-demoted]" not in (cand.reason or "") @@ -178,11 +179,11 @@ def test_demoted_fallback_candidate_marked_deprioritized(self): # Demote aider_local — it should still appear but as DEPRIORITIZED demoted = {"aider_local"} - sel = LaneSelector( + planner = DecisionPlanner( policy=policy, adjustment_query=lambda lane: "demote" if lane in demoted else "neutral", ) - plan = sel.plan_routes(_proposal()) + plan = planner.plan(_proposal()) # Find the aider_local fallback candidate aider_candidates = [c for c in plan.fallbacks.candidates if c.lane == "aider_local"] diff --git a/test/unit/test_lane_planner.py b/test/unit/test_lane_planner.py index 5a81e47..9c54f62 100644 --- a/test/unit/test_lane_planner.py +++ b/test/unit/test_lane_planner.py @@ -254,27 +254,6 @@ def test_policy_summary_mentions_blocked_when_present(): assert "blocked" in plan.policy_summary.lower() -# --------------------------------------------------------------------------- -# LaneSelector.plan_routes() delegation -# --------------------------------------------------------------------------- - - -def test_lane_selector_plan_routes_returns_routing_plan(): - from switchboard.lane.engine import LaneSelector - selector = LaneSelector() - plan = selector.plan_routes(_proposal()) - assert isinstance(plan, RoutingPlan) - - -def test_lane_selector_plan_routes_primary_matches_select(): - from switchboard.lane.engine import LaneSelector - selector = LaneSelector() - proposal = _proposal(task_type=TaskType.LINT_FIX, risk_level=RiskLevel.LOW) - decision = selector.select(proposal) - plan = selector.plan_routes(proposal) - assert plan.primary.lane == decision.selected_lane.value - - # --------------------------------------------------------------------------- # Custom policy # --------------------------------------------------------------------------- diff --git a/test/unit/test_signal_aggregator.py b/test/unit/test_signal_aggregator.py index 56aa41a..5fc03c3 100644 --- a/test/unit/test_signal_aggregator.py +++ b/test/unit/test_signal_aggregator.py @@ -48,19 +48,15 @@ def test_mean_latency(self) -> None: sig = ProfileSignals(profile="fast", _latencies_ms=[100.0, 200.0, 300.0]) assert sig.mean_latency_ms == pytest.approx(200.0) - def test_p50_latency(self) -> None: - sig = ProfileSignals(profile="fast", _latencies_ms=[100.0, 200.0, 300.0]) - assert sig.p50_latency_ms == pytest.approx(200.0) - def test_p95_latency(self) -> None: latencies = list(range(1, 101)) # 1..100 sig = ProfileSignals(profile="fast", _latencies_ms=[float(x) for x in latencies]) # p95 index = max(0, int(100 * 0.95) - 1) = max(0, 94) = index 94 → value 95 assert sig.p95_latency_ms == pytest.approx(95.0) - def test_p50_none_when_no_latencies(self) -> None: + def test_p95_none_when_no_latencies(self) -> None: sig = ProfileSignals(profile="fast") - assert sig.p50_latency_ms is None + assert sig.p95_latency_ms is None # ---------------------------------------------------------------------------