From accea93ad794dd9b963e1f256cfcc17cb99d5c21 Mon Sep 17 00:00:00 2001 From: "@jaydenOoOo" Date: Sun, 9 Aug 2026 00:49:12 +0800 Subject: [PATCH 1/2] Audit the rest of the codebase; fix a live traversal hole MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Covers what the M0-M3 and control-layer audits did not: the queueing core, the adaptive controllers, the Pareto machinery, the price client and the deployed container. Four defects. The budget pacer was steering on a frozen capacity trace. adaptive.py kept its own copy of the actuation-delay rule so C5/C6 could pace against their own past requests, and fixing the clock-restart bug in apply_actuation_delay left that copy behind. On a plan that moves every step — which is what C5/C6 emit — the mirror reported a flat 13 replicas across 40 steps while the simulator provisioned 13 to 24, so the PI loop corrected against violations that never happened. Extracted ActuationTracker and implemented the batch form on top of it so the two cannot diverge; a test agrees them over 200 random plans and another fails if the rule is re-implemented anywhere outside simulator.py. Re-running the frontier moves only C5/C6 rows by small amounts in both directions. Q4 is unchanged at 9/3/6 with the below-19:1 pattern intact, and the GPU frontier verdict is unchanged at 0 of 8. The SPA catch-all could be walked out of the dist directory. Percent-encoded traversal survives URL normalisation and reached the handler intact, serving arbitrary container files — verified against the real deploy image under uvicorn, which also ran as root. No credentials exist to steal, the project being keyless, but the source and every system file were readable. Render's edge rejected the encoded forms with a 400, so the live site was not exploitable; that is an accident of the CDN, not a control, and the README's docker run had no such cover. Candidates are now resolved and confined to dist, which also closes symlink escapes and absolute-path injection, and the image drops to an unprivileged user. The deploy-image CI lane asserts both against the running container. dominated_hypervolume swept from cost zero and credited each strip with the dearer point's violation rate, when only the cheaper point is affordable over that interval. One point at cost 5 and 0.2 violations against a reference of 10 returned 8.0 where the rectangle is 4.0, and the error could reverse a ranking rather than merely inflate it. The old test compared two curves where it cancelled. Nothing published used the function. _synthesize_arrivals claimed the utilisation and queue fidelities described the same physical system. They do not — utilisation_target derates one and not the other — which is harmless because every published result uses the utilisation model and the Erlang-C validation sets the target to 1.0, but the docstring asserted something false. Verified sound: the ACI update matches the published BACC form including its sign, pareto_front is a correct two-objective sweep, cost_at_matched_violation reports unreachable targets rather than substituting the nearest point, and the price parser skips non-positive prices rather than defaulting them. The OData filter is now escaped; not a live injection path, but an apostrophe would have returned the wrong SKU's price. --- .github/workflows/ci.yml | 9 +++ Dockerfile.web | 6 ++ docs/EVAL.md | 104 ++++++++++++++++++++++++++++++ src/delphi/api/app.py | 26 ++++++-- src/delphi/control/adaptive.py | 20 ++---- src/delphi/control/simulator.py | 85 +++++++++++++++++------- src/delphi/data/prices.py | 18 +++++- src/delphi/evaluation/frontier.py | 24 ++++--- tests/test_api.py | 50 ++++++++++++++ tests/test_frontier.py | 44 +++++++++++++ tests/test_simulator.py | 31 +++++++++ 11 files changed, 365 insertions(+), 52 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cf53041..1cd21ba 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,6 +48,15 @@ jobs: curl -sf http://localhost:8000/workloads | grep -q 'workload_id' curl -sf http://localhost:8000/findings | grep -q 'refuted' curl -sf http://localhost:8000/ | grep -q 'DELPHI' + # The container must not run as root, and the SPA catch-all must not serve + # anything outside dist. Percent-encoded traversal previously read arbitrary + # container files; Render's edge rejected it, `docker run` did not. + test "$(docker exec delphi id -u)" != "0" + for probe in '/..%2f..%2frequirements-serve.txt' '/%2e%2e/%2e%2e/%2e%2e/etc/passwd'; do + body=$(curl -s --path-as-is "http://localhost:8000$probe") + echo "$body" | grep -q 'root:' && { echo "TRAVERSAL: $probe"; exit 1; } + echo "$body" | grep -q 'Runtime-only dependencies' && { echo "TRAVERSAL: $probe"; exit 1; } + done curl -sf -X POST http://localhost:8000/diagnostic/score \ -H 'Content-Type: application/json' \ -d "$(python3 -c 'import json,math;print(json.dumps({"values":[60+25*math.sin(i/45.8) for i in range(2000)],"step_seconds":300}))')" \ diff --git a/Dockerfile.web b/Dockerfile.web index 303eac9..3cb0306 100644 --- a/Dockerfile.web +++ b/Dockerfile.web @@ -46,6 +46,12 @@ ENV PYTHONPATH=/app/src \ DELPHI_SNAPSHOT_MODE=demo \ PORT=8000 +# Drop root. Nothing here needs it: the app reads a baked snapshot and writes nothing. A +# read-only API running as uid 0 turns any file-disclosure bug into a read of the whole +# container, which is exactly what the SPA traversal bug would have done. +RUN useradd --create-home --uid 10001 delphi && chown -R delphi:delphi /app +USER delphi + EXPOSE 8000 # Render injects $PORT; default to 8000 for local runs of the exact deploy image. diff --git a/docs/EVAL.md b/docs/EVAL.md index eccad52..44a3383 100644 --- a/docs/EVAL.md +++ b/docs/EVAL.md @@ -1196,3 +1196,107 @@ would mean something. forecastability literature. - **The one measure that showed signal does not survive multiple-comparison correction**, and is reported as a lead rather than promoted into the product. + +--- + +# Full-codebase audit (2026-08-09) + +The 2026-08-03 audit covered M0–M3 and the 2026-08-08 one covered the control and product +layers. This pass covered what neither did: the queueing core, the adaptive controllers, the +Pareto machinery, the price client, and the deployed container. Four defects, one of them a +live security hole. + +## 1. The budget pacer was steering on a frozen capacity trace + +`adaptive.py` carried its **own copy** of the actuation-delay rule, so C5/C6 could reconstruct +what their past requests would have served and pace against it. When the clock-restart bug +was fixed in `apply_actuation_delay` on 2026-08-08, that copy was left behind — still +restarting the clock, still freezing. + +Measured on a plan that moves every step, which is exactly what C5/C6 emit: the mirror +reported a flat 13 replicas for 40 steps while the simulator provisioned 13 → 24. **36 of 40 +steps disagreed**, so the PI loop spent every run correcting against violations that never +happened. + +Fixed by extracting `ActuationTracker`, with `apply_actuation_delay` implemented on top of it +so the batch and incremental forms *cannot* diverge. A test asserts they agree over 200 +random plans, and another fails if the lag rule is re-implemented anywhere outside +`simulator.py` — this being the second time a copy of it drifted. + +**Effect on published results: small and no conclusion moves.** Re-running the frontier +changes only C5/C6 rows: cost 66.6 → 66.5, churn 180 → 176, violation rates shifting by +around 0.001–0.008 in both directions. Q4 is bit-for-bit unchanged at 9 better / 3 tie / 6 +worse of 18, and the "wins below 19:1, loses above" pattern holds. The GPU frontier verdict +is unchanged at 0 of 8. + +## 2. Path traversal in the deployed dashboard + +The SPA catch-all joined the request path onto the dist directory and served whatever it +found. Percent-encoded traversal survives URL normalisation, reaches the handler intact, and +read arbitrary files off the container: + +``` +/..%2f..%2frequirements-serve.txt -> served the file +/%2e%2e/%2e%2e/%2e%2e/etc/hostname -> served the file +``` + +Verified against the real deploy image under uvicorn, not merely in a test client. **The +container also ran as root**, so this was an arbitrary read of everything in it. No +credentials exist to steal — the project is keyless by design — but the source, the +dependency manifest and every system file were readable. + +Render's edge happened to reject the encoded forms with a 400, so the live site was not +exploitable. That is an accident of the CDN rather than a control, and anyone following the +README's `docker run` had no such cover. + +Fixed by resolving the candidate and requiring it to stay inside dist — which also closes +symlink escapes and absolute-path injection — and by dropping to an unprivileged user in the +image. Both are now asserted in the deploy-image CI lane against the running container, so a +regression fails the build rather than the internet. + +## 3. Hypervolume was measuring the wrong staircase + +`dominated_hypervolume` swept from cost zero and credited each strip with the *dearer* +point's violation rate, when over that interval only the cheaper point is affordable. A +single point at cost 5 with a 0.2 violation rate against a reference corner at cost 10 +returned 8.0 where the true dominated rectangle is 5 x 0.8 = **4.0**. + +This is not merely an inflation: it can reverse a ranking. A frontier reaching a 0.05 +violation rate at cost 9 scored *above* one sitting at 0.45 for cost 5, where the corrected +areas are 1.75 and 3.25 respectively. + +The existing test compared two frontiers where the error cancelled, so it passed throughout. +It now checks areas computed by hand. **No published result used this function** — it is +reported nowhere in this document — so nothing downstream moves. + +## 4. The two fidelities were documented as the same system + +`_synthesize_arrivals` claimed its service-time construction kept the utilisation and queue +models "describing the same physical system". They do not: `utilisation_target` derates +capacity in the utilisation model but is not applied in the queue, so at a target of 0.8 the +queue runs at rho = 0.8 where the utilisation model reports a step exactly at its limit. + +Defensible for two models answering different questions, and harmless in practice — every +published result uses the utilisation model, and the Erlang-C validation sets the target to +1.0 where the discrepancy vanishes — but the docstring asserted something false about the +code. Corrected to state the difference and why it does not contaminate anything. + +## Verified sound + +The ACI update rule reproduces the published BACC form exactly, including the sign of the +correction under miscoverage. `pareto_front` is a correct two-objective sweep. +`cost_at_matched_violation` reports unreachable targets as unreachable rather than +substituting the nearest point. `parse_price_items` skips rows with a missing or non-positive +price rather than defaulting them, which would otherwise drive the newsvendor ratio to buy +unbounded capacity. `cheapest_hourly` refuses to convert monthly reservations into hourly +prices. The OData filter is now escaped — not a live injection path, since the values come +from config, but an apostrophe would have silently returned the wrong SKU's price. + +## Added to the negatives ledger + +- **A duplicated implementation of the actuation lag drifted from its original**, for the + second time, and fed a control loop a capacity trace that never moved. +- **The deployed container was vulnerable to path traversal and ran as root.** It was + unexploitable in production only because of a CDN behaviour nobody had designed for. +- **A Pareto summary statistic could rank two frontiers backwards**, and its test passed + because the error cancelled between the two curves being compared. diff --git a/src/delphi/api/app.py b/src/delphi/api/app.py index 4ac169d..5411f30 100644 --- a/src/delphi/api/app.py +++ b/src/delphi/api/app.py @@ -281,10 +281,28 @@ def evidence(limit: Annotated[int, Query(ge=1, le=200)] = 50) -> dict[str, objec if _DIST.is_dir(): app.mount("/assets", StaticFiles(directory=_DIST / "assets"), name="assets") + _DIST_ROOT = _DIST.resolve() + @app.get("/{path:path}", include_in_schema=False) def spa(path: str) -> FileResponse: - """Serve the SPA shell for any unmatched path.""" - candidate = _DIST / path - if path and candidate.is_file(): + """Serve the SPA shell for any unmatched path. + + **The candidate is resolved and confined to the dist directory.** Without that, + ``_DIST / path`` walks straight out of it: percent-encoded traversal such as + ``/..%2f..%2frequirements-serve.txt`` or ``/%2e%2e/%2e%2e/etc/hostname`` survives + URL normalisation, reaches this handler intact, and served arbitrary files off the + container — as root, in the deploy image. Render's edge happened to reject those + requests with a 400, but an accident at the CDN is not a security control, and + anyone running the documented ``docker run`` locally had no such cover. + + ``resolve()`` also collapses symlinks, so a link inside ``dist`` cannot be used to + step outside it either. An absolute ``path`` would make ``/`` discard the root + entirely; the containment check catches that case too. + """ + index = _DIST_ROOT / "index.html" + if not path: + return FileResponse(index) + candidate = (_DIST_ROOT / path).resolve() + if candidate.is_file() and candidate.is_relative_to(_DIST_ROOT): return FileResponse(candidate) - return FileResponse(_DIST / "index.html") + return FileResponse(index) diff --git a/src/delphi/control/adaptive.py b/src/delphi/control/adaptive.py index e0d14f0..cbd1d12 100644 --- a/src/delphi/control/adaptive.py +++ b/src/delphi/control/adaptive.py @@ -22,7 +22,7 @@ from delphi.control.controllers import ControlContext, PercentileRecommender, _replicas_for from delphi.control.newsvendor import CostRatio, interpolate_quantile -from delphi.control.simulator import clamp_plan +from delphi.control.simulator import ActuationTracker, clamp_plan from delphi.forecast.calibration import AdaptiveConformalCalibrator from delphi.forecast.contracts import QuantileForecaster @@ -127,10 +127,9 @@ def _clamped(replicas: int) -> int: # raw forecast value emitted for each step, so its residual can be scored once the # true demand for that step becomes observable raw_for_step: dict[int, float] = {} - # incremental mirror of the simulator's actuation delay, over our own plan - current = int(requested[0]) - pending_target: int | None = None - pending_at = 0 + # incremental mirror of the simulator's actuation delay, over our own plan. Shares the + # simulator's implementation rather than restating it, so the two cannot drift. + tracker = ActuationTracker(profile, initial=int(requested[0])) served: dict[int, int] = {} settled = context.start_step @@ -182,16 +181,7 @@ def _clamped(replicas: int) -> int: raw_for_step[target_step] = raw # advance our mirror of the platform's actuation lag one step - asked = int(requested[target_step]) - in_flight = pending_target if pending_target is not None else current - if asked != in_flight: - lag = profile.startup_steps if asked > current else profile.teardown_steps - pending_target = asked - pending_at = target_step + max(lag, 0) - if pending_target is not None and target_step >= pending_at: - current = pending_target - pending_target = None - served[target_step] = current + served[target_step] = tracker.advance(target_step, int(requested[target_step])) step += block diff --git a/src/delphi/control/simulator.py b/src/delphi/control/simulator.py index c236709..53efb8a 100644 --- a/src/delphi/control/simulator.py +++ b/src/delphi/control/simulator.py @@ -146,6 +146,52 @@ def summary(self) -> dict[str, float]: } +class ActuationTracker: + """Incremental form of the actuation delay: one step in, one serving count out. + + This exists so there is exactly **one** implementation of the lag. A controller that + paces itself against its own past decisions has to reconstruct what was actually + serving, and the obvious way to do that is to re-derive the rule — which is what C5/C6 + did, and which meant that fixing the clock-restart bug in `apply_actuation_delay` left a + stale copy in `adaptive.py` silently feeding the PI loop a capacity trace that froze at + the initial replica count. A shared object cannot drift from itself. + """ + + def __init__(self, profile: CapacityProfile, *, initial: int) -> None: + self._profile = profile + self._current = int(initial) + self._pending: int | None = None + self._pending_at = 0 + + @property + def current(self) -> int: + """What is serving right now, after everything already landed.""" + return self._current + + def advance(self, step: int, requested: int) -> int: + """Register the request for ``step`` and return what is serving at ``step``. + + Superseding retargets the change in flight and keeps its original landing step: + pods already starting do not begin again because the desired count moved. + """ + target = int(requested) + if self._pending is None: + if target != self._current: + lag = ( + self._profile.startup_steps + if target > self._current + else self._profile.teardown_steps + ) + self._pending = target + self._pending_at = step + max(lag, 0) + elif target != self._pending: + self._pending = target + if self._pending is not None and step >= self._pending_at: + self._current = self._pending + self._pending = None + return self._current + + def apply_actuation_delay(requested: IntArray, profile: CapacityProfile) -> IntArray: """Turn a requested trajectory into the capacity that is actually serving. @@ -164,26 +210,10 @@ def apply_actuation_delay(requested: IntArray, profile: CapacityProfile) -> IntA unrequestable state that silently penalises exactly the smooth, forecast-driven controllers this project exists to evaluate. """ - steps = len(requested) - provisioned = np.empty(steps, dtype=np.int64) - current = int(requested[0]) - pending_target: int | None = None - pending_at = 0 - - for step in range(steps): - target = int(requested[step]) - if pending_target is None: - if target != current: - lag = profile.startup_steps if target > current else profile.teardown_steps - pending_target = target - pending_at = step + max(lag, 0) - elif target != pending_target: - # Retarget the in-flight change, keeping its original landing step. - pending_target = target - if pending_target is not None and step >= pending_at: - current = pending_target - pending_target = None - provisioned[step] = current + tracker = ActuationTracker(profile, initial=int(requested[0])) + provisioned = np.empty(len(requested), dtype=np.int64) + for step in range(len(requested)): + provisioned[step] = tracker.advance(step, int(requested[step])) return provisioned @@ -216,8 +246,19 @@ def _synthesize_arrivals( Arrivals are placed uniformly at random inside their step, which reproduces a Poisson process conditioned on the per-step count. Service times are exponential with a mean - implied by ``capacity_per_replica`` so that one replica serves exactly that many units - per step on average — keeping the two fidelities describing the same physical system. + implied by ``capacity_per_replica``, so one replica serves exactly that many units per + step on average. + + **The two fidelities are not the same physical system, and this docstring used to claim + they were.** ``utilisation_target`` derates capacity in the utilisation model — a + replica there absorbs ``capacity_per_replica * utilisation_target`` per step — but the + queue model applies no such derate, because a queue expresses congestion through waiting + time rather than through a headroom ceiling. At a target of 0.8 the queue therefore runs + at rho = 0.8 where the utilisation model reports a step exactly at its limit. That is + defensible for two models answering different questions, but it means their absolute + numbers are not comparable and must never share a results table. Every published result + in `docs/EVAL.md` uses the utilisation model; the queue exists to validate the simulator + against Erlang-C, where ``utilisation_target`` is set to 1.0 and the discrepancy is nil. """ counts = np.rint(np.maximum(demand, 0.0)).astype(np.int64) total = int(counts.sum()) diff --git a/src/delphi/data/prices.py b/src/delphi/data/prices.py index 9eddd16..0b9b819 100644 --- a/src/delphi/data/prices.py +++ b/src/delphi/data/prices.py @@ -101,11 +101,25 @@ def parse_price_items(payload: dict[str, Any], *, retrieved_at: datetime) -> lis return prices +def _odata_literal(value: str) -> str: + """Quote a string for an OData filter, escaping embedded single quotes by doubling. + + These values come from config rather than from a request, so this is not a live + injection path — but an unescaped apostrophe would silently malform the query and + return the wrong SKU's price, which then propagates into the newsvendor ratio as a + plausible-looking number. Cheaper to escape than to debug. + """ + return "'" + value.replace("'", "''") + "'" + + def build_filter(*, region: str, service_name: str, sku_contains: str | None = None) -> str: """Compose the OData ``$filter`` the API expects.""" - clauses = [f"armRegionName eq '{region}'", f"serviceName eq '{service_name}'"] + clauses = [ + f"armRegionName eq {_odata_literal(region)}", + f"serviceName eq {_odata_literal(service_name)}", + ] if sku_contains: - clauses.append(f"contains(skuName, '{sku_contains}')") + clauses.append(f"contains(skuName, {_odata_literal(sku_contains)})") return " and ".join(clauses) diff --git a/src/delphi/evaluation/frontier.py b/src/delphi/evaluation/frontier.py index 739573c..65219c6 100644 --- a/src/delphi/evaluation/frontier.py +++ b/src/delphi/evaluation/frontier.py @@ -122,19 +122,25 @@ def dominated_hypervolume( A scalar summary for when one is genuinely needed. Larger is better. It is reported only alongside its reference point, because a hypervolume without its reference is meaningless and trivially manipulable. + + The staircase runs *forward* from each point: over the cost interval between one + frontier point and the next, the best violation rate available is the **cheaper** + point's, because the dearer one cannot be afforded yet. An earlier version credited each + strip with the dearer point's rate and started the sweep at cost zero, which double + counted a single-point frontier and could reverse the ranking of two curves. """ - front = pareto_front(points) + front = [ + point + for point in pareto_front(points) + if point.cost < cost_reference and point.violation_rate < violation_reference + ] if not front: return 0.0 area = 0.0 - previous_cost = 0.0 - for point in front: - if point.cost >= cost_reference or point.violation_rate >= violation_reference: - continue - width = point.cost - previous_cost - area += width * (violation_reference - point.violation_rate) - previous_cost = point.cost - area += (cost_reference - previous_cost) * (violation_reference - front[-1].violation_rate) + for current, following in zip(front, front[1:], strict=False): + area += (following.cost - current.cost) * (violation_reference - current.violation_rate) + last = front[-1] + area += (cost_reference - last.cost) * (violation_reference - last.violation_rate) return float(max(area, 0.0)) diff --git a/tests/test_api.py b/tests/test_api.py index 4e5539a..d8b92e8 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -248,3 +248,53 @@ def test_demand_response_is_bounded(client: TestClient, tmp_path: Path) -> None: path.write_text(original) else: path.unlink() + + +def test_spa_route_cannot_be_walked_out_of_the_dist_directory( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Percent-encoded traversal escaped `dist` and served arbitrary container files. + + `/..%2f..%2frequirements-serve.txt` and `/%2e%2e/%2e%2e/etc/hostname` both survived URL + normalisation, reached the handler, and were served — as root, in the deploy image. + Render's edge rejected them with a 400, but that is an accident of the CDN, not a + control, and `docker run` locally had no such cover. + """ + import importlib + + from delphi import config as config_module + + dist = tmp_path / "dist" + (dist / "assets").mkdir(parents=True) + (dist / "index.html").write_text("<title>DELPHI") + (dist / "assets" / "app.js").write_text("// bundle") + (tmp_path / "SECRET.txt").write_text("SENSITIVE-CONTENTS") + + monkeypatch.setenv("DELPHI_DASHBOARD_DIST", str(dist)) + monkeypatch.setenv("DELPHI_SNAPSHOT_PATH", str(tmp_path / "snapshot.json")) + config_module.get_settings.cache_clear() + reloaded = importlib.reload(app_module) + try: + client = TestClient(reloaded.app) + for probe in ( + "/../SECRET.txt", + "/..%2fSECRET.txt", + "/%2e%2e/SECRET.txt", + "/..%2f..%2fSECRET.txt", + "/%2e%2e%2fSECRET.txt", + ): + body = client.get(probe).text + assert "SENSITIVE-CONTENTS" not in body, f"{probe} escaped the dist directory" + assert "DELPHI" in body, f"{probe} should fall through to the SPA shell" + + # Under /assets Starlette's StaticFiles guards its own mount and answers 404 + # rather than falling through — also safe, just a different shape of refusal. + assets_probe = client.get("/assets/..%2f..%2fSECRET.txt") + assert "SENSITIVE-CONTENTS" not in assets_probe.text + assert assets_probe.status_code == 404 + + # A genuine asset inside dist must still be served. + assert "// bundle" in client.get("/assets/app.js").text + finally: + config_module.get_settings.cache_clear() + importlib.reload(app_module) diff --git a/tests/test_frontier.py b/tests/test_frontier.py index d7b0604..7918d79 100644 --- a/tests/test_frontier.py +++ b/tests/test_frontier.py @@ -96,6 +96,50 @@ def test_hypervolume_of_an_empty_frontier_is_zero() -> None: assert dominated_hypervolume([], cost_reference=10.0) == 0.0 +def test_hypervolume_matches_the_area_computed_by_hand() -> None: + """A relative check passes even when the area is double what it should be. + + One point at cost 5 with a 0.2 violation rate, against a reference corner at cost 10, + dominates exactly the rectangle from cost 5 to 10 and violation 0.2 to 1.0 — an area of + 5 x 0.8 = 4.0. The previous implementation swept from cost zero, where nothing is + affordable, and returned 8.0. + """ + assert dominated_hypervolume([_point(5.0, 0.2)], cost_reference=10.0) == pytest.approx(4.0) + + staircase = [_point(2.0, 0.5), _point(5.0, 0.2), _point(8.0, 0.1)] + expected = (5 - 2) * (1 - 0.5) + (8 - 5) * (1 - 0.2) + (10 - 8) * (1 - 0.1) + assert dominated_hypervolume(staircase, cost_reference=10.0) == pytest.approx(expected) + + +def test_hypervolume_ranks_two_curves_the_way_the_area_does() -> None: + """The off-by-one strip could reverse a ranking, not merely inflate both sides.""" + reaches_low_but_dear = [_point(1.0, 0.9), _point(9.0, 0.05)] + cheap_and_middling = [_point(4.0, 0.5), _point(5.0, 0.45)] + assert dominated_hypervolume(cheap_and_middling, cost_reference=10.0) > dominated_hypervolume( + reaches_low_but_dear, cost_reference=10.0 + ) + + +def test_hypervolume_ignores_points_outside_the_reference_box() -> None: + """Only points strictly inside the reference corner contribute. + + Note what does *not* count as outside: a cheap point with a dreadful violation rate is + still non-dominated and still dominates a thin strip of its own. Excluding it would + understate the frontier, so the box test is against the reference corner alone. + """ + inside = [_point(5.0, 0.2)] + too_dear = _point(50.0, 0.01) + no_better_than_reference = _point(1.0, 1.0) + assert dominated_hypervolume( + [*inside, too_dear, no_better_than_reference], cost_reference=10.0 + ) == pytest.approx(dominated_hypervolume(inside, cost_reference=10.0)) + + # A cheap-but-bad point is inside the box and legitimately adds its own strip. + assert dominated_hypervolume([*inside, _point(1.0, 0.99)], cost_reference=10.0) == ( + pytest.approx(4.0 + (5 - 1) * (1 - 0.99)) + ) + + def test_parity_spec_states_the_budget_every_arm_was_held_to() -> None: spec = ParitySpec( configurations_per_controller=7, diff --git a/tests/test_simulator.py b/tests/test_simulator.py index fc12209..76780bc 100644 --- a/tests/test_simulator.py +++ b/tests/test_simulator.py @@ -1,9 +1,12 @@ """Degenerate, determinism, and actuation-delay checks for the replay simulator.""" +from pathlib import Path + import numpy as np import pytest from delphi.control.simulator import ( + ActuationTracker, CapacityProfile, CostModel, apply_actuation_delay, @@ -71,6 +74,34 @@ def test_a_continuously_changing_request_still_lands() -> None: assert apply_actuation_delay(settling, profile).tolist()[-1] == 6 +def test_tracker_reproduces_the_batch_actuation_delay() -> None: + """The incremental and batch forms must agree on every plan, including erratic ones.""" + profile = _profile(startup_seconds=180.0, teardown_seconds=60.0) + rng = np.random.default_rng(20260809) + for _ in range(200): + plan = np.clip(np.cumsum(rng.integers(-3, 4, size=60)) + 20, 1, None).astype(np.int64) + tracker = ActuationTracker(profile, initial=int(plan[0])) + incremental = [tracker.advance(step, int(plan[step])) for step in range(len(plan))] + assert incremental == apply_actuation_delay(plan, profile).tolist() + + +def test_only_the_simulator_implements_the_actuation_lag() -> None: + """No second copy of the lag rule anywhere in the package. + + C5/C6 previously re-derived it to pace themselves against their own past decisions. + When the clock-restart bug was fixed in `apply_actuation_delay`, that copy was left + stale, and the budget pacer spent every run correcting against a capacity trace frozen + at its initial replica count. The suite could not see it. This can. + """ + root = Path(__file__).resolve().parents[1] / "src" / "delphi" + offenders = [ + path.relative_to(root) + for path in root.rglob("*.py") + if path.name != "simulator.py" and "pending_at" in path.read_text() + ] + assert not offenders, f"actuation lag re-implemented outside simulator.py: {offenders}" + + def test_zero_actuation_delay_makes_the_plan_immediate() -> None: profile = _profile(startup_seconds=0.0, teardown_seconds=0.0) requested = np.asarray([1, 7, 2], dtype=np.int64) From 6f6fb4a958ca5b64645f4c53cfb9f69214c656da Mon Sep 17 00:00:00 2001 From: "@jaydenOoOo" Date: Sun, 9 Aug 2026 00:51:53 +0800 Subject: [PATCH 2/2] Use itertools.pairwise in the hypervolume sweep Caught by CI, not locally, because I read ruff's trailing summary line as success instead of checking its exit code. The gate is a set of exit codes and should be verified as one. --- src/delphi/evaluation/frontier.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/delphi/evaluation/frontier.py b/src/delphi/evaluation/frontier.py index 65219c6..7a4a344 100644 --- a/src/delphi/evaluation/frontier.py +++ b/src/delphi/evaluation/frontier.py @@ -13,6 +13,7 @@ from collections.abc import Sequence from dataclasses import dataclass +from itertools import pairwise import numpy as np import numpy.typing as npt @@ -137,7 +138,7 @@ def dominated_hypervolume( if not front: return 0.0 area = 0.0 - for current, following in zip(front, front[1:], strict=False): + for current, following in pairwise(front): area += (following.cost - current.cost) * (violation_reference - current.violation_rate) last = front[-1] area += (cost_reference - last.cost) * (violation_reference - last.violation_rate)