Skip to content

Commit 47376bb

Browse files
authored
Fix stale AST cache crash in fragile-path lint (#120)
* Refresh README with clearer positioning * Fix stale AST cache crash in fragile-path lint
1 parent 9c4b536 commit 47376bb

5 files changed

Lines changed: 157 additions & 10 deletions

File tree

src/kida/bytecode_cache.py

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,11 @@
3737
import logging
3838
import marshal
3939
import pickle
40+
import re
4041
import struct
4142
import sys
4243
import time
44+
from importlib.metadata import PackageNotFoundError, version
4345
from pathlib import Path
4446
from tempfile import NamedTemporaryFile
4547
from typing import TYPE_CHECKING, cast
@@ -49,6 +51,7 @@
4951
logger = logging.getLogger("kida.bytecode_cache")
5052

5153
if TYPE_CHECKING:
54+
from collections.abc import Iterable
5255
from types import CodeType
5356

5457
from kida.nodes.base import Node
@@ -66,6 +69,15 @@
6669
# Python version tag for cache invalidation across Python upgrades
6770
_PY_VERSION_TAG = f"py{sys.version_info.major}{sys.version_info.minor}"
6871

72+
# Cache ABI tag for Kida compiler/AST changes. Bytecode cache entries contain
73+
# compiled Python code and, optionally, pickled Kida AST nodes; both are tied to
74+
# the engine version and to AST dataclass shape.
75+
try:
76+
_KIDA_VERSION_TAG = re.sub(r"[^A-Za-z0-9]+", "_", version("kida-templates")).strip("_")
77+
except PackageNotFoundError: # pragma: no cover - package metadata may be absent in ad-hoc embeds
78+
_KIDA_VERSION_TAG = "unknown"
79+
_CACHE_ABI_TAG = f"{_PY_VERSION_TAG}_kida{_KIDA_VERSION_TAG}_ast2"
80+
6981

7082
class BytecodeCache:
7183
"""Persist compiled template bytecode to disk.
@@ -122,7 +134,7 @@ def _make_path(self, name: str, source_hash: str, context_hash: str | None = Non
122134
if context_hash:
123135
hash_key = f"{hash_key}_{context_hash[:16]}"
124136
filename = self._pattern.format(
125-
version=_PY_VERSION_TAG,
137+
version=_CACHE_ABI_TAG,
126138
name=safe_name,
127139
hash=hash_key,
128140
)
@@ -221,6 +233,11 @@ def get(
221233
"Bytecode cache: failed to unpickle AST for '%s': %s", name, exc
222234
)
223235
ast = None
236+
if ast is not None and not _cached_ast_is_compatible(ast):
237+
logger.debug("Bytecode cache: incompatible cached AST for '%s'", name)
238+
with contextlib.suppress(OSError):
239+
path.unlink(missing_ok=True)
240+
return None, None, None
224241

225242
return code, ast, precomputed
226243

@@ -252,6 +269,11 @@ def get(
252269
"Bytecode cache: failed to unpickle AST (v2) for '%s': %s", name, exc
253270
)
254271
ast = None
272+
if ast is not None and not _cached_ast_is_compatible(ast):
273+
logger.debug("Bytecode cache: incompatible cached AST (v2) for '%s'", name)
274+
with contextlib.suppress(OSError):
275+
path.unlink(missing_ok=True)
276+
return None, None, None
255277

256278
return code, ast, None
257279
else:
@@ -408,3 +430,39 @@ def stats(self) -> dict[str, int]:
408430
def hash_source(source: str) -> str:
409431
"""Generate hash of template source for cache key."""
410432
return hashlib.sha256(source.encode()).hexdigest()
433+
434+
435+
def _cached_ast_is_compatible(ast: object) -> bool:
436+
"""Return True when an unpickled AST matches the current dataclass shape."""
437+
from kida.nodes.base import Node
438+
439+
if not isinstance(ast, Node):
440+
return False
441+
442+
stack: list[Node] = [ast]
443+
while stack:
444+
node = stack.pop()
445+
for field_name in node.__dataclass_fields__:
446+
try:
447+
value = getattr(node, field_name)
448+
except AttributeError:
449+
return False
450+
if isinstance(value, Node):
451+
stack.append(value)
452+
elif isinstance(value, (list, tuple)):
453+
_extend_node_sequence(stack, value)
454+
elif isinstance(value, dict):
455+
_extend_node_sequence(stack, value.values())
456+
return True
457+
458+
459+
def _extend_node_sequence(stack: list[Node], seq: Iterable[object]) -> None:
460+
from kida.nodes.base import Node
461+
462+
for item in seq:
463+
if isinstance(item, Node):
464+
stack.append(item)
465+
elif isinstance(item, (list, tuple)):
466+
_extend_node_sequence(stack, item)
467+
elif isinstance(item, dict):
468+
_extend_node_sequence(stack, item.values())

src/kida/compiler/partial_eval.py

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
from __future__ import annotations
2727

2828
from collections.abc import Callable, Mapping, Sequence
29-
from dataclasses import dataclass
29+
from dataclasses import dataclass, replace
3030
from typing import Any, final
3131

3232
from kida.nodes import (
@@ -1904,14 +1904,7 @@ def _transform_expr(self, expr: Expr) -> Expr:
19041904
new_value = self._transform_expr(expr.value)
19051905
if new_value is expr.value:
19061906
return expr
1907-
return Filter(
1908-
lineno=expr.lineno,
1909-
col_offset=expr.col_offset,
1910-
name=expr.name,
1911-
value=new_value,
1912-
args=expr.args,
1913-
kwargs=expr.kwargs,
1914-
)
1907+
return replace(expr, value=new_value)
19151908

19161909
case Pipeline():
19171910
val = self._try_eval(expr)

tests/test_fragile_paths_lint.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,15 @@
22

33
from __future__ import annotations
44

5+
import pickle
6+
import struct
57
from typing import TYPE_CHECKING
68

79
from kida import DictLoader, Environment
810
from kida.analysis.fragile_paths import check_fragile_paths
11+
from kida.bytecode_cache import BytecodeCache, hash_source
912
from kida.cli import main
13+
from kida.nodes import Filter, Name
1014

1115
if TYPE_CHECKING:
1216
from pathlib import Path
@@ -20,6 +24,24 @@ def _ast(source: str, caller: str) -> object:
2024
return tpl._optimized_ast
2125

2226

27+
def _stale_filter_pickle() -> bytes:
28+
current = bytearray(
29+
pickle.dumps(
30+
Filter(
31+
lineno=1,
32+
col_offset=35,
33+
value=Name(lineno=1, col_offset=35, name="route_link_attrs"),
34+
name="html_attrs",
35+
),
36+
protocol=5,
37+
)
38+
)
39+
del current[-4] # Drop NEWFALSE for Filter.parenthesized from the state list.
40+
frame_len = struct.unpack("<Q", current[3:11])[0]
41+
current[3:11] = struct.pack("<Q", frame_len - 1)
42+
return bytes(current)
43+
44+
2345
def test_same_folder_include_is_flagged() -> None:
2446
ast = _ast('{% include "pages/card.html" %}', "pages/about.html")
2547
issues = check_fragile_paths(ast, "pages/about.html")
@@ -126,6 +148,29 @@ def test_cli_lint_fragile_paths_exit_nonzero(
126148
assert "./card.html" in err
127149

128150

151+
def test_cli_lint_fragile_paths_ignores_stale_cached_ast(
152+
tmp_path: Path, capsys: pytest.CaptureFixture[str]
153+
) -> None:
154+
"""A stale AST cache entry should recompile, not crash the lint visitor."""
155+
(tmp_path / "pages").mkdir()
156+
source = '{% include "pages/card.html" %}{{ href | upper }}'
157+
(tmp_path / "pages" / "about.html").write_text(source)
158+
(tmp_path / "pages" / "card.html").write_text("CARD")
159+
160+
cache = BytecodeCache(tmp_path / "__pycache__" / "kida")
161+
source_hash = hash_source(source)
162+
cache.set("pages/about.html", source_hash, compile("x = 1", "<stale>", "exec"))
163+
path = cache._make_path("pages/about.html", source_hash)
164+
path.write_bytes(path.read_bytes() + _stale_filter_pickle())
165+
166+
exit_code = main(["check", str(tmp_path), "--lint-fragile-paths"])
167+
assert exit_code == 1
168+
169+
err = capsys.readouterr().err
170+
assert "lint/fragile-path" in err
171+
assert "Traceback" not in err
172+
173+
129174
def test_cli_lint_flag_default_off(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None:
130175
"""Without the flag the lint is silent — default `kida check` is unchanged."""
131176
(tmp_path / "pages").mkdir()

tests/test_kida_bytecode_cache.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
Tests persistent template caching for fast cold-start.
44
"""
55

6+
import pickle
7+
import struct
68
import time
79
from concurrent.futures import ThreadPoolExecutor
810

@@ -12,6 +14,27 @@
1214
from kida.exceptions import TemplateNotFoundError
1315

1416

17+
def _stale_filter_pickle() -> bytes:
18+
"""Return a Filter pickle from before the parenthesized slot existed."""
19+
from kida.nodes import Filter, Name
20+
21+
current = bytearray(
22+
pickle.dumps(
23+
Filter(
24+
lineno=1,
25+
col_offset=3,
26+
value=Name(lineno=1, col_offset=3, name="route_link_attrs"),
27+
name="html_attrs",
28+
),
29+
protocol=5,
30+
)
31+
)
32+
del current[-4] # Drop NEWFALSE for Filter.parenthesized from the state list.
33+
frame_len = struct.unpack("<Q", current[3:11])[0]
34+
current[3:11] = struct.pack("<Q", frame_len - 1)
35+
return bytes(current)
36+
37+
1538
class TestBytecodeCache:
1639
"""Tests for bytecode cache."""
1740

@@ -255,6 +278,18 @@ def test_corrupted_cache_file_handled(self, cache, cache_dir):
255278
# Corrupted file should be removed
256279
assert not corrupted_path.exists()
257280

281+
def test_incompatible_cached_ast_is_discarded(self, cache):
282+
"""Stale pickled AST nodes from an older Kida schema do not crash."""
283+
source_hash = hash_source("{{ route_link_attrs(href) | html_attrs }}")
284+
code = compile("x = 1", "<test>", "exec")
285+
cache.set("stale.html", source_hash, code)
286+
path = cache._make_path("stale.html", source_hash)
287+
path.write_bytes(path.read_bytes() + _stale_filter_pickle())
288+
289+
assert cache.stats()["file_count"] == 1
290+
assert cache.get("stale.html", source_hash) == (None, None, None)
291+
assert cache.stats()["file_count"] == 0
292+
258293

259294
class TestHashSource:
260295
"""Tests for source hashing."""

tests/test_kida_modern_syntax.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,22 @@ def test_no_precedence_warning_when_fallback_parenthesized(self, env):
192192
prec_warnings = [x for x in w if issubclass(x.category, PrecedenceWarning)]
193193
assert len(prec_warnings) == 1
194194

195+
def test_partial_eval_preserves_parenthesized_filter_fallback(self, env):
196+
"""Partial evaluation keeps the parser's parenthesized-filter marker."""
197+
import warnings
198+
199+
from kida.exceptions import PrecedenceWarning
200+
201+
with warnings.catch_warnings(record=True) as w:
202+
warnings.simplefilter("always")
203+
tmpl = env.from_string(
204+
"{{ x ?? ((prefix ~ name) | upper) }}",
205+
static_context={"prefix": "hi "},
206+
)
207+
assert tmpl.render(x=None, name="there") == "HI THERE"
208+
prec_warnings = [x for x in w if issubclass(x.category, PrecedenceWarning)]
209+
assert prec_warnings == []
210+
195211
def test_null_coalesce_with_filter_requires_parens(self, env):
196212
"""Filters must use parentheses to apply after null coalescing.
197213

0 commit comments

Comments
 (0)