Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 49 additions & 15 deletions plan/rfc-template-helpers-any-reduction.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# RFC: Template Helpers Any Reduction

**Status**: Draft
**Status**: Active — baseline refreshed and first bounded helper slice implemented (#146)

| Field | Value |
|-------|-------|
Expand All @@ -12,7 +12,37 @@

## Summary

Reduce `Any` usage in Kida's template runtime helpers (`helpers.py`, `render_helpers.py`, `environment/filters/_validation.py`) to improve type safety without breaking the polymorphic nature of template context. This RFC complements the completed mixin/type-suppression work by addressing the remaining ~20 `Any` hotspots in runtime code.
Reduce `Any` usage in Kida's template runtime helpers (`helpers.py`, `render_helpers.py`, `environment/filters/_validation.py`) to improve type safety without breaking the polymorphic nature of template context. This RFC complements the completed mixin/type-suppression work while preserving `Any` at genuinely dynamic context and generated-code boundaries.

## Current Audit (2026-07-06)

The original counts no longer describe the current tree. A reproducible lexical
scan with `rg -o '\bAny\b' ... | wc -l` reports:

| Scope | `Any` tokens before this slice |
|---|---:|
| `src/kida/` (`*.py`) | 658 across 63 files |
| `template/helpers.py` | 36 |
| `template/render_helpers.py` | 19 |
| `template/core.py` | 37 |
| `environment/filters/_validation.py` | 0 |

After this slice, the same scan reports 641 tokens under `src/kida/` and 19
in `template/helpers.py`, a reduction of 17 without changing runtime behavior.

These are lexical token counts, so they include imports, annotations, casts,
comments, and docstrings. They are a trend baseline, not a quality score.

The first #146 slice narrows only contracts that are provably independent of
template-context shape:

- `_raise_undefined_attr()` returns `Never`;
- `markup_concat()`, `coerce_numeric()`, lazy default/defined/coalescing helpers,
and `optional_call()` accept or return `object` where values are arbitrary;
- profiling pass-through helpers use a generic type so their result type is
preserved;
Comment on lines +39 to +43
- dynamic context dictionaries, namespace dictionaries, generated-call
boundaries, and unchecked subscript casts remain `Any` by design for now.

---

Expand Down Expand Up @@ -146,14 +176,14 @@ class RenderHelpers(TypedDict, total=False):

```yaml
Phase 1 (Low-Risk) - 1-2 hours:
- [ ] 1.1: helpers.py — object instead of Any for get, safe_getattr, getattr_preserve_none
- [ ] 1.2: filters/_validation.py — object for _filter_default, _filter_require
- [ ] 1.3: Run ty check, pytest
- [x] 1.1: helpers.py — object instead of Any for get, safe_getattr, getattr_preserve_none
- [x] 1.2: filters/_validation.py — object for _filter_default, _filter_require
- [x] 1.3: Run ty check, pytest

Phase 2 (Render Helpers) - 2-3 hours:
- [ ] 2.1: render_helpers.py — MacroWrapper, _make_macro_wrapper, make_render_helpers
- [x] 2.1: render_helpers.py — MacroWrapper, _make_macro_wrapper, make_render_helpers
- [ ] 2.2: core.py — docstring only for render(**kwargs)
- [ ] 2.3: Run ty check, pytest
- [x] 2.3: Run ty check, pytest

Phase 3 (Optional):
- [ ] 3.1: RenderHelpers TypedDict if desired
Expand All @@ -173,14 +203,18 @@ uv run ruff check src/

## Success Criteria

| Metric | Before | After Phase 1 | After Phase 2 |
|--------|--------|---------------|---------------|
| `Any` in helpers.py | 4 | 0 | 0 |
| `Any` in render_helpers.py | 4 | 4 | 1 (MacroWrapper return if kept) |
| `Any` in filters/_validation.py | 2 | 0 | 0 |
| `Any` in core.py | 8 | 8 | 8 (unchanged; documented) |
| ty check | Pass | Pass | Pass |
| pytest | Pass | Pass | Pass |
Lexical counts include imports, casts, comments, and docstrings; they are used
only to make the direction and bounded scope reproducible.

| Metric | Before #146 slice | After #146 slice |
|--------|------------------:|-----------------:|
| `Any` tokens in `helpers.py` | 36 | 19 |
| `Any` tokens in `render_helpers.py` | 19 | 19 (unchanged) |
| `Any` tokens in `filters/_validation.py` | 0 | 0 |
| `Any` tokens in `core.py` | 37 | 37 (unchanged) |
| `Any` tokens in `src/kida/` | 658 | 641 |
| `make ty` | Pass | Pass |
| Focused runtime tests | Pass | Pass |

---

Expand Down
30 changes: 17 additions & 13 deletions src/kida/template/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

from collections.abc import Mapping
from time import perf_counter as _perf_counter
from typing import TYPE_CHECKING, Any, cast
from typing import TYPE_CHECKING, Any, Never, cast

from kida.render_accumulator import RenderAccumulator
from kida.render_accumulator import get_accumulator as _get_accumulator
Expand Down Expand Up @@ -216,7 +216,7 @@ def getitem_preserve_none(obj: object, key: object) -> object:
# returning UNDEFINED/"". Used when the user wants to catch template typos.


def _raise_undefined_attr(obj: object, name: str, *, preserve_none: bool = False) -> Any:
def _raise_undefined_attr(obj: object, name: str, *, preserve_none: bool = False) -> Never:
"""Raise UndefinedError for missing attribute/key access."""
from kida.exceptions import UndefinedError, build_source_snippet
from kida.render_context import get_render_context
Expand Down Expand Up @@ -362,7 +362,7 @@ def strict_getitem_preserve_none(obj: object, key: object) -> object:
_REGION_DEFAULT = object()


def markup_concat(left: Any, right: Any) -> str:
def markup_concat(left: object, right: object) -> str:
"""Concatenate for ~ operator, preserving Markup safety.

If either operand is Markup, the result is Markup and the non-Markup
Expand Down Expand Up @@ -397,7 +397,7 @@ def markup_concat(left: Any, right: Any) -> str:
}


def record_filter_usage(acc: RenderAccumulator | None, name: str, result: Any) -> Any:
def record_filter_usage[T](acc: RenderAccumulator | None, name: str, result: T) -> T:
"""Record filter usage for profiling, returning the result unchanged.

Called by compiled code for every filter invocation. When profiling
Expand All @@ -416,7 +416,7 @@ def record_filter_usage(acc: RenderAccumulator | None, name: str, result: Any) -
return result


def record_macro_usage(acc: RenderAccumulator | None, name: str, result: Any) -> Any:
def record_macro_usage[T](acc: RenderAccumulator | None, name: str, result: T) -> T:
"""Record macro ({% def %}) call for profiling, returning the result unchanged.

Called by compiled code for every macro invocation. When profiling
Expand Down Expand Up @@ -528,10 +528,10 @@ def lookup_scope(ctx: dict[str, Any], scope_stack: list[dict[str, Any]], var_nam


def default_safe(
value_fn: Callable[[], Any],
default_value: Any = "",
value_fn: Callable[[], object],
default_value: object = "",
boolean: bool = False,
) -> Any:
) -> object:
"""Safe default filter that works with strict mode.

In strict mode, the value expression might raise UndefinedError.
Expand Down Expand Up @@ -561,7 +561,7 @@ def default_safe(
return value if (value is not None and not isinstance(value, _Undefined)) else default_value


def is_defined(value_fn: Callable[[], Any]) -> bool:
def is_defined(value_fn: Callable[[], object]) -> bool:
"""Check if a value is defined in strict mode.

In strict mode, we need to catch UndefinedError to determine
Expand Down Expand Up @@ -591,7 +591,7 @@ def is_defined(value_fn: Callable[[], Any]) -> bool:
return False


def null_coalesce(left_fn: Callable[[], Any], right_fn: Callable[[], Any]) -> Any:
def null_coalesce(left_fn: Callable[[], object], right_fn: Callable[[], object]) -> object:
"""Safe null coalescing that handles undefined variables.

In strict mode, the left expression might raise UndefinedError.
Expand Down Expand Up @@ -667,7 +667,7 @@ def add_polymorphic(left: Any, right: Any) -> str:
return left + right


def coerce_numeric(value: Any) -> int | float:
def coerce_numeric(value: object) -> int | float:
"""Coerce value to numeric type for arithmetic operations.

Handles Markup objects (from macros) and strings that represent numbers.
Expand Down Expand Up @@ -744,12 +744,16 @@ def consume(key: str, default: Any = None) -> Any:
return rc.consume(key, default)


def optional_call(callee: Any, *args: object, **kwargs: object) -> Any:
def optional_call(
callee: Callable[..., object] | _Undefined | None,
*args: object,
**kwargs: object,
) -> object:
"""Call callee only if it is not None or UNDEFINED.

Used for obj?.method() so that when obj is None or obj.attr is UNDEFINED,
the call short-circuits and returns UNDEFINED (outputs as "").
"""
if callee is None or callee is UNDEFINED:
return UNDEFINED
return callee(*args, **kwargs)
return cast("Callable[..., object]", callee)(*args, **kwargs)
Loading