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 # ---------------------------------------------------------------------------