Skip to content

Make the shared template registry actually shared (#104) - #449

Merged
jeremymanning merged 1 commit into
mainfrom
fix/template-language-conformance
Aug 2, 2026
Merged

Make the shared template registry actually shared (#104)#449
jeremymanning merged 1 commit into
mainfrom
fix/template-language-conformance

Conversation

@jeremymanning

Copy link
Copy Markdown
Member

Addresses findings 1 and 2 of the review of #448.

The shared registry was not shared

#448 routed the validator, compiler and runtime through one filter registry and
asserted it by comparing filter names. The compiler called its own
_register_custom_filters() on the very next line, replacing eleven of those
implementations. Name sets matched, so the drift test passed. Semantics did not:

expression runtime compiler
{{ 0 | default('X') }} '0' 'X'
{{ '' | default('X') }} '' 'X'
{{ false | default('X') }} 'False' 'X'
{{ missing | default('X') }} 'X' UndefinedError

The compiler's default treated every falsy value as absent and raised on an
undefined one
— the single case the filter exists for. to_json, slugify and
regex_search diverged too, which the review had not caught.

All eleven filters the compiler registered were already in the runtime registry
and none were unique to it, so the 110-line method and its call site are
deleted rather than reconciled.

Behaviour, not registries

test_no_filter_diverges_between_environments renders every filter in the
registry through all three environments against probes chosen for where
implementations part company — undefined, None, empty string, zero, false,
non-ASCII, non-string — and compares results.

A hand-written table covers default's undefined-vs-falsy distinction,
malformed JSON, regex, paths and unicode explicitly. Three cases run end to end
through the CLI, so the assertion reaches a real file rather than stopping at an
environment.

Two comparison artefacts I had to handle rather than ignore: lazy filters render
as <generator object ... at 0x...> whose address differs between any two
renders, and random/shuffle are not functions of their input. Addresses are
masked; the two nondeterministic filters are excluded by name.

A test of mine that could not fail

test_a_pipeline_using_a_runtime_filter_validates accepted any error whose text
lacked the word "filter", and fed from_json the literal "Hello World Report"
— not JSON. An unrelated rendering error satisfied the assertion. It now asserts
the pipeline compiles, on valid JSON. That is the second test in two PRs where I
asserted on error wording instead of behaviour.

Evidence

mutation result
re-introduce divergent default 10 failed, including the CLI cases
add divergent upper (in no hand-written case) 2 failed, caught by the generic sweep
  • Blocking suite 573 → 602 passed, 0 failed
  • Catalogue unchanged at 18/117 — this PR fixes semantics, not acceptance
  • ruff (CI rule set) and compileall clean

Not in this PR

Deliberately, so each stays reviewable:

  • Finding 3, runtime namespaces. execution, pipeline, context, env
    are still accepted unconditionally and at least execution is never populated.
    Fixing it makes more examples fail, and it wants a typed context schema that
    validation and runtime population both derive from.
  • Finding 4, the four integration cases for unknown output fields (warning
    reaching consumers, dependency affecting scheduling, declared schema still
    rejecting misspellings, nested fields).
  • Findings 5 and 6, catalogue triage and the CI dashboard.

#448 routed the validator, compiler and runtime through one filter registry
and asserted it by comparing filter *names*. The compiler then called its own
`_register_custom_filters()` on the very next line, replacing eleven of those
implementations. The name sets matched, so the drift test passed, while the
semantics differed:

    {{ 0 | default('X') }}          runtime '0'      compiler 'X'
    {{ '' | default('X') }}         runtime ''       compiler 'X'
    {{ false | default('X') }}      runtime 'False'  compiler 'X'
    {{ missing | default('X') }}    runtime 'X'      compiler UndefinedError

The compiler's `default` treated every falsy value as absent, and raised on an
undefined one -- the single case the filter exists for. `to_json`, `slugify`
and `regex_search` diverged too.

All eleven filters the compiler registered were already in the runtime registry
and none were unique to it, so the 110-line method and its call site are
deleted rather than reconciled. The environments now genuinely share one set.

The drift test is replaced by behavioural comparison. `test_no_filter_diverges
_between_environments` renders *every* filter in the registry through all three
environments against probes chosen for where implementations part company --
undefined, None, empty string, zero, false, non-ASCII, a non-string -- and
compares results, not registries. A hand-written table covers `default`'s
undefined-versus-falsy distinction, malformed JSON, regex, paths and unicode
explicitly, and three cases run end to end through the CLI so the assertion
reaches a real file.

Also fixes a test of mine that could not fail: it accepted any error whose text
lacked the word "filter", and fed `from_json` the string "Hello World Report",
which is not JSON -- so an unrelated rendering error satisfied it. It now
asserts the pipeline compiles, on valid JSON.

Mutation-tested: re-introducing the divergent `default` fails 10 tests
including the CLI cases; adding a divergent `upper` -- a filter no hand-written
case names -- fails the generic sweep.

Blocking suite: 573 -> 602 passed, 0 failed. Catalogue unchanged at 18/117.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jeremymanning

Copy link
Copy Markdown
Member Author

CI verification

9/9 green.

Legacy tally against the current post-merge main run (30771446768):

main #449 delta
failed 474 471 -3
passed 1846 1849 +3
errors 189 189 0
deselected 672 701 +29

The +29 deselected is exactly the 29 tests this PR adds to the blocking layer
(573 -> 602).

The -3 is not this PR fixing anything

A -3/+3 on a PR that changes filter semantics in the compiler is exactly the
case where the delta might be real, so I diffed the named failures instead of
calling it noise:

FIXED (failed on main, pass here):
  quality/reporting/test_analytics.py::TestAnalyticsResult::test_analytics_result_creation
  quality/reporting/test_analytics.py::TestQualityInsight::test_quality_insight_creation
  quality/reporting/test_metrics.py::TestQualityMetric::test_quality_metric_creation

NEWLY FAILING: none

All three are in tests/orchestrator/quality/reporting/, the unstable module
identified on #445 — the same tests that flipped on #442, #443 and #445, in PRs
touching nothing near them. This is that cluster again, not a repair.

The number worth reading is newly failing: none. Deleting 110 lines of
filter implementations that the compiler had been using for every compile-time
render broke nothing in 2,300 legacy tests.

Warning classes are identical on both (2 RuntimeWarning, 1
PytestUnraisableExceptionWarning, 1 PytestReturnNotNoneWarning), so the 11 -> 12
count is another instance of an existing class, not a new kind of problem.

Standing caveat

As on #448: the legacy layer's coverage of the template path is thin enough
that "nothing moved" is consistent with the change being safe without being
evidence of it. The 602-test blocking suite and the two mutations are what I am
relying on.

@jeremymanning
jeremymanning merged commit 457f778 into main Aug 2, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant