diff --git a/perf/bench_framework_latency.py b/perf/bench_framework_latency.py index 25a90a32c..aef9208f3 100755 --- a/perf/bench_framework_latency.py +++ b/perf/bench_framework_latency.py @@ -30,21 +30,55 @@ FRAMEWORKS: tuple[Framework, ...] = ("easycat", "livekit", "pipecat") _WORKER_EOF = object() ENVIRONMENT_ROOT = Path(__file__).with_name("framework_environments") -LOCK_EXCLUDE_NEWER_BY_FRAMEWORK = { - framework: str( - tomllib.loads((ENVIRONMENT_ROOT / framework / "uv.lock").read_text())["options"][ - "exclude-newer" - ] - ) - for framework in ("livekit", "pipecat") +COMPETITORS: tuple[Framework, ...] = ("livekit", "pipecat") + + +def _lock_exclude_newer(framework: Framework) -> str | None: + """Read a competitor lock's pinned resolution cutoff. + + The competitor environments are deliberately frozen: ``uv lock + --exclude-newer`` records the cutoff in the lock's ``[options]`` table so + the benchmark resolves the same transitive snapshot on every run. + Regenerating a lock without that flag drops the table entirely, which is + what a Dependabot bump to one of these directories does. + + Return ``None`` in that case rather than raising. This runs at import, so a + ``KeyError`` here fails collection for the whole module and reports an + opaque missing key; the callers below and the perf tests turn ``None`` into + a message that names the lock and how to regenerate it. + """ + lock = tomllib.loads((ENVIRONMENT_ROOT / framework / "uv.lock").read_text()) + cutoff = lock.get("options", {}).get("exclude-newer") + return None if cutoff is None else str(cutoff) + + +LOCK_EXCLUDE_NEWER_BY_FRAMEWORK: dict[str, str | None] = { + framework: _lock_exclude_newer(framework) for framework in COMPETITORS } + + +def require_lock_exclude_newer(framework: Framework) -> str: + """Return the pinned cutoff, explaining how to restore a regenerated lock.""" + cutoff = LOCK_EXCLUDE_NEWER_BY_FRAMEWORK[framework] + if cutoff is None: + project = f"perf/framework_environments/{framework}" + raise RuntimeError( + f"{project}/uv.lock has no [options] exclude-newer, so the benchmark can no " + "longer resolve the reviewed snapshot. This lock is pinned on purpose -- " + "revert the regeneration, or re-pin it with `uv lock --project " + f"{project} --exclude-newer ` and update the reviewed cutoff in " + "tests/perf/test_framework_latency_benchmark.py." + ) + return cutoff + + PINS = { framework: tuple( tomllib.loads((ENVIRONMENT_ROOT / framework / "pyproject.toml").read_text())["project"][ "dependencies" ] ) - for framework in ("livekit", "pipecat") + for framework in COMPETITORS } RESPONSE_TEXT = "Hello there." EXPECTED_TTS_TEXT = { @@ -83,7 +117,7 @@ def worker_specs( "--no-progress", "--no-config", "--exclude-newer", - LOCK_EXCLUDE_NEWER_BY_FRAMEWORK[framework], + require_lock_exclude_newer(framework), "--isolated", "--project", str(project), @@ -101,13 +135,13 @@ def worker_specs( def _lock_metadata() -> dict[str, dict[str, str]]: metadata: dict[str, dict[str, str]] = {} - for framework in ("livekit", "pipecat"): + for framework in COMPETITORS: lock_path = ENVIRONMENT_ROOT / framework / "uv.lock" digest = hashlib.sha256(lock_path.read_bytes()).hexdigest() metadata[framework] = { "path": str(lock_path.relative_to(Path(__file__).parents[1])), "sha256": digest, - "exclude_newer": LOCK_EXCLUDE_NEWER_BY_FRAMEWORK[framework], + "exclude_newer": require_lock_exclude_newer(framework), } return metadata diff --git a/tests/perf/test_framework_latency_benchmark.py b/tests/perf/test_framework_latency_benchmark.py index 3d1f758be..cdeb1fe43 100644 --- a/tests/perf/test_framework_latency_benchmark.py +++ b/tests/perf/test_framework_latency_benchmark.py @@ -11,10 +11,12 @@ PINS, Worker, WorkerSpec, + _lock_exclude_newer, _lock_metadata, _validate_sample, percentile, rank_by_latency, + require_lock_exclude_newer, run_benchmark, worker_specs, ) @@ -133,6 +135,45 @@ def test_worker_startup_exit_fails_without_waiting_for_response_timeout(tmp_path ) +def test_regenerated_lock_without_cutoff_reads_as_missing_instead_of_raising( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + # A Dependabot bump to perf/framework_environments/ regenerates + # the lock without `--exclude-newer`, which drops the whole [options] + # table. Reading that at import time raised KeyError('options') and failed + # collection for this entire module, so every perf test reported an opaque + # missing key instead of the one real problem. + project = tmp_path / "pipecat" + project.mkdir() + (project / "uv.lock").write_text('version = 1\n\n[[package]]\nname = "aiohttp"\n') + monkeypatch.setattr("perf.bench_framework_latency.ENVIRONMENT_ROOT", tmp_path) + + assert _lock_exclude_newer("pipecat") is None + + +def test_missing_lock_cutoff_names_the_lock_and_how_to_re_pin_it( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setitem(LOCK_EXCLUDE_NEWER_BY_FRAMEWORK, "pipecat", None) + + with pytest.raises(RuntimeError, match="exclude-newer") as excinfo: + require_lock_exclude_newer("pipecat") + + message = str(excinfo.value) + assert "perf/framework_environments/pipecat/uv.lock" in message + assert "uv lock --project perf/framework_environments/pipecat" in message + + # The isolated-environment command must never be built from a lock that no + # longer pins the snapshot it claims to reproduce. + with pytest.raises(RuntimeError, match="exclude-newer"): + worker_specs(("pipecat",)) + + # Nor may a benchmark report record a cutoff the lock no longer carries. + with pytest.raises(RuntimeError, match="exclude-newer"): + _lock_metadata() + + def test_competitor_lock_metadata_is_content_addressed() -> None: metadata = _lock_metadata() diff --git a/tests/ratchets/_ast_digest.py b/tests/ratchets/_ast_digest.py new file mode 100644 index 000000000..c4d24dcc0 --- /dev/null +++ b/tests/ratchets/_ast_digest.py @@ -0,0 +1,61 @@ +"""Interpreter-stable AST digests for the reviewed ratchet baselines. + +Every manifest in this package pins an ``ast_hash`` per classified source site, +so the digest has to be byte-identical on each interpreter in the support +matrix (3.11 through 3.14). Plain ``ast.dump`` is not, and CI runs the same +manifests on 3.11 and 3.14: + +* 3.12 added PEP 695 ``type_params`` to ``FunctionDef``, ``AsyncFunctionDef``, + and ``ClassDef``, so those nodes grew a field mid-matrix. +* 3.13 started omitting fields that equal their default, so ``args=[]`` and + friends disappear from the dump on newer interpreters. + +Either one shifts every hash for the affected nodes and fails the ratchets with +a wall of "removed or structurally changed" sites that no commit caused. This +module pins one canonical serialization instead of inheriting the stdlib's, and +``tests/ratchets/test_ast_digest.py`` locks the behaviour down. + +The format deliberately reproduces ``ast.dump(node, annotate_fields=True, +include_attributes=False)`` as emitted by 3.11 — including its rule of dropping +an optional field that is ``None`` by default — so the baselines reviewed +against that output stay valid. +""" + +from __future__ import annotations + +import ast +import hashlib + +# ``type_params`` only exists on 3.12+, and is always empty in source that must +# still import on 3.11, so it carries no signal worth ratcheting. +_VERSION_DEPENDENT_FIELDS = frozenset({"type_params"}) + + +def canonical_dump(node: ast.AST) -> str: + """Serialize ``node`` identically on every supported interpreter.""" + return _format(node) + + +def ast_digest(node: ast.AST) -> str: + """Return the 16-hex-character digest the ratchet manifests record.""" + return hashlib.sha256(canonical_dump(node).encode("utf-8")).hexdigest()[:16] + + +def _format(value: object) -> str: + if isinstance(value, ast.AST): + fields = [] + for name in value._fields: + if name in _VERSION_DEPENDENT_FIELDS: + continue + try: + field = getattr(value, name) + except AttributeError: + # An unset optional field, exactly as ``ast.dump`` treats it. + continue + if field is None and getattr(type(value), name, object()) is None: + continue + fields.append(f"{name}={_format(field)}") + return f"{type(value).__name__}({', '.join(fields)})" + if isinstance(value, list): + return f"[{', '.join(_format(item) for item in value)}]" + return repr(value) diff --git a/tests/ratchets/_pause_generation_inventory.py b/tests/ratchets/_pause_generation_inventory.py index d7aba944d..47d3d01ba 100644 --- a/tests/ratchets/_pause_generation_inventory.py +++ b/tests/ratchets/_pause_generation_inventory.py @@ -3,12 +3,13 @@ from __future__ import annotations import ast -import hashlib from collections import Counter from collections.abc import Sequence from dataclasses import dataclass from pathlib import Path +from tests.ratchets._ast_digest import ast_digest + TARGETS = frozenset({"session/_stt_committer.py", "turn_manager.py"}) @@ -229,14 +230,14 @@ def _record_assignment(self, target: ast.AST, surrounding: ast.AST) -> None: self._record("future_map_write", "store future correlation", surrounding) def _record(self, category: str, construct: str, node: ast.AST) -> None: - normalized = ast.dump(node, annotate_fields=True, include_attributes=False) + ast_hash = ast_digest(node) self.candidates.append( _Candidate( category=category, path=self.relative_path, qualname=self.qualname, construct=construct, - ast_hash=hashlib.sha256(normalized.encode()).hexdigest()[:16], + ast_hash=ast_hash, line=getattr(node, "lineno", 0), ) ) diff --git a/tests/ratchets/_source_inventory.py b/tests/ratchets/_source_inventory.py index f082430c1..0628ef96e 100644 --- a/tests/ratchets/_source_inventory.py +++ b/tests/ratchets/_source_inventory.py @@ -3,13 +3,14 @@ from __future__ import annotations import ast -import hashlib from collections import Counter from collections.abc import Iterator from contextlib import contextmanager from dataclasses import dataclass from pathlib import Path +from tests.ratchets._ast_digest import ast_digest + RAW_TASK_EXEMPT = frozenset({"_concurrency.py", "runtime/scope.py"}) CANCELLING_EXEMPT = RAW_TASK_EXEMPT UNCANCEL_EXEMPT = frozenset({"_concurrency.py"}) @@ -475,5 +476,4 @@ def _assigned_leaf_names(target: ast.AST) -> list[str]: def _normalized_hash(node: ast.AST) -> str: - normalized = ast.dump(node, annotate_fields=True, include_attributes=False) - return hashlib.sha256(normalized.encode("utf-8")).hexdigest()[:16] + return ast_digest(node) diff --git a/tests/ratchets/_teardown_budget_inventory.py b/tests/ratchets/_teardown_budget_inventory.py index 10787a05d..2071a11a7 100644 --- a/tests/ratchets/_teardown_budget_inventory.py +++ b/tests/ratchets/_teardown_budget_inventory.py @@ -3,13 +3,14 @@ from __future__ import annotations import ast -import hashlib from collections import Counter from collections.abc import Iterator from contextlib import contextmanager from dataclasses import dataclass from pathlib import Path +from tests.ratchets._ast_digest import ast_digest, canonical_dump + CLASSIFICATIONS = frozenset({"configurable", "lifecycle_budget", "not_teardown", "protocol_local"}) _BUDGET_WORDS = ("deadline", "timeout") _LIFECYCLE_WORDS = ( @@ -228,7 +229,9 @@ def visit_AnnAssign(self, node: ast.AnnAssign) -> None: def visit_Call(self, node: ast.Call) -> None: resolved = self._resolve(node.func) if self._in_lifecycle_closure and _is_budget_call(node, resolved): - construct = resolved or ast.dump(node.func, include_attributes=False) + # The fallback label lands in the manifest, so it needs the same + # interpreter-stable serialization as the hashes. + construct = resolved or canonical_dump(node.func) self._record("lifecycle_call", f"call {construct}", node) self.generic_visit(node) @@ -370,5 +373,4 @@ def _assigned_leaf_names(target: ast.AST) -> list[str]: def _normalized_hash(node: ast.AST) -> str: - normalized = ast.dump(node, annotate_fields=True, include_attributes=False) - return hashlib.sha256(normalized.encode("utf-8")).hexdigest()[:16] + return ast_digest(node) diff --git a/tests/ratchets/_turn_commit_inventory.py b/tests/ratchets/_turn_commit_inventory.py index eca773b3c..83cea5c33 100644 --- a/tests/ratchets/_turn_commit_inventory.py +++ b/tests/ratchets/_turn_commit_inventory.py @@ -3,12 +3,13 @@ from __future__ import annotations import ast -import hashlib from collections import Counter from collections.abc import Sequence from dataclasses import dataclass from pathlib import Path +from tests.ratchets._ast_digest import ast_digest + TARGETS = frozenset( { "session/_stt_committer.py", @@ -239,8 +240,7 @@ def _record_assignment(self, target: ast.AST, surrounding: ast.AST) -> None: def _record(self, category: str, effect: str, node: ast.AST) -> None: suspension = self._suspension_kind(node) - normalized = ast.dump(node, annotate_fields=True, include_attributes=False) - ast_hash = hashlib.sha256(normalized.encode()).hexdigest()[:16] + ast_hash = ast_digest(node) self.candidates.append( _Candidate( category=category, diff --git a/tests/ratchets/_turn_lifecycle_inventory.py b/tests/ratchets/_turn_lifecycle_inventory.py index 74d80aad6..429c97cf2 100644 --- a/tests/ratchets/_turn_lifecycle_inventory.py +++ b/tests/ratchets/_turn_lifecycle_inventory.py @@ -3,12 +3,13 @@ from __future__ import annotations import ast -import hashlib from collections import Counter from collections.abc import Sequence from dataclasses import dataclass from pathlib import Path +from tests.ratchets._ast_digest import ast_digest + @dataclass(frozen=True, order=True, slots=True) class TurnLifecycleSite: @@ -321,8 +322,7 @@ def _record_dynamic_writer(self, node: ast.Call, callee: str) -> None: self._record(category, f"setattr {target}", node) def _record(self, category: str, construct: str, surrounding: ast.AST) -> None: - normalized = ast.dump(surrounding, annotate_fields=True, include_attributes=False) - ast_hash = hashlib.sha256(normalized.encode()).hexdigest()[:16] + ast_hash = ast_digest(surrounding) self.candidates.append( _Candidate( category=category, diff --git a/tests/ratchets/_turn_predicate_inventory.py b/tests/ratchets/_turn_predicate_inventory.py index 94b27c82e..1f08aca1e 100644 --- a/tests/ratchets/_turn_predicate_inventory.py +++ b/tests/ratchets/_turn_predicate_inventory.py @@ -3,12 +3,13 @@ from __future__ import annotations import ast -import hashlib from collections import Counter from collections.abc import Sequence from dataclasses import dataclass from pathlib import Path +from tests.ratchets._ast_digest import ast_digest + TARGETS = frozenset( { "session/_stt_committer.py", @@ -178,8 +179,7 @@ def _visit_scope( self._scope.pop() def _record(self, category: str, construct: str, surrounding: ast.AST) -> None: - normalized = ast.dump(surrounding, annotate_fields=True, include_attributes=False) - ast_hash = hashlib.sha256(normalized.encode()).hexdigest()[:16] + ast_hash = ast_digest(surrounding) self.candidates.append( _Candidate( category=category, diff --git a/tests/ratchets/test_ast_digest.py b/tests/ratchets/test_ast_digest.py new file mode 100644 index 000000000..d0e691c58 --- /dev/null +++ b/tests/ratchets/test_ast_digest.py @@ -0,0 +1,95 @@ +"""Pin the ratchet digest so it cannot drift with the interpreter. + +Every manifest in this package records an ``ast_hash``, and CI verifies the same +manifests on 3.11 and 3.14. The digest therefore has to be a property of the +source, not of the running interpreter's ``ast.dump``. These tests fail on the +interpreter where a regression appears, which is what makes the cross-version +matrix meaningful instead of a coin flip. +""" + +from __future__ import annotations + +import ast +import hashlib + +from tests.ratchets._ast_digest import ast_digest, canonical_dump + +_SOURCE = """ +class Worker: + @staticmethod + async def run(self, retries=3): + await self._turn.commit() + label = "done" + return label +""" + +# Reviewed against 3.11's ``ast.dump(annotate_fields=True, +# include_attributes=False)``, which is the output every existing baseline in +# this directory was classified against. +_EXPECTED_CANONICAL = ( + "ClassDef(name='Worker', bases=[], keywords=[], body=[AsyncFunctionDef(name='run', " + "args=arguments(posonlyargs=[], args=[arg(arg='self'), arg(arg='retries')], " + "kwonlyargs=[], kw_defaults=[], defaults=[Constant(value=3)]), " + "body=[Expr(value=Await(value=Call(func=Attribute(value=Attribute(" + "value=Name(id='self', ctx=Load()), attr='_turn', ctx=Load()), attr='commit', " + "ctx=Load()), args=[], keywords=[]))), Assign(targets=[Name(id='label', " + "ctx=Store())], value=Constant(value='done')), Return(value=Name(id='label', " + "ctx=Load()))], decorator_list=[Name(id='staticmethod', ctx=Load())])], " + "decorator_list=[])" +) +_EXPECTED_DIGEST = "acaf37b8d0b7233f" + + +def _class_node() -> ast.ClassDef: + node = ast.parse(_SOURCE).body[0] + assert isinstance(node, ast.ClassDef) + return node + + +def test_canonical_dump_matches_the_reviewed_baseline_serialization() -> None: + assert canonical_dump(_class_node()) == _EXPECTED_CANONICAL + + +def test_ast_digest_is_the_pinned_hash_of_the_canonical_dump() -> None: + node = _class_node() + assert ast_digest(node) == _EXPECTED_DIGEST + assert ( + ast_digest(node) == hashlib.sha256(canonical_dump(node).encode("utf-8")).hexdigest()[:16] + ) + + +def test_canonical_dump_keeps_empty_fields_that_ast_dump_drops_on_3_13() -> None: + # 3.13 taught ``ast.dump`` to omit fields equal to their default, which + # silently rewrote every hash for call-bearing nodes. + call = ast.parse("self._turn.commit()").body[0] + assert canonical_dump(call) == ( + "Expr(value=Call(func=Attribute(value=Attribute(value=Name(id='self', ctx=Load()), " + "attr='_turn', ctx=Load()), attr='commit', ctx=Load()), args=[], keywords=[]))" + ) + + +def test_canonical_dump_omits_type_params_added_in_3_12() -> None: + # PEP 695 gave function and class nodes a ``type_params`` field on 3.12+. + for source in ("def f(): pass", "async def f(): pass", "class C: pass"): + dumped = canonical_dump(ast.parse(source).body[0]) + assert "type_params" not in dumped, source + + +def test_canonical_dump_omits_optional_fields_that_default_to_none() -> None: + # ``ast.dump`` skips an optional field whose class default is ``None``; + # emitting ``kind=None`` here would invalidate every reviewed baseline. + assert canonical_dump(ast.parse("'text'").body[0]) == "Expr(value=Constant(value='text'))" + assert canonical_dump(ast.parse("return").body[0]) == "Return()" + + +def test_canonical_dump_still_separates_structurally_different_sources() -> None: + # The stability work must not flatten real differences into one hash. + digests = { + ast_digest(ast.parse(source).body[0]) + for source in ( + "self._turn.commit()", + "self._turn.begin()", + "self._turn.commit(force=True)", + ) + } + assert len(digests) == 3