You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The query-resolution flow re-derives the variable-precedence context (runtime kwarg / stage .variables / outer-query .variables / model query_variables) ad-hoc at three call sites, and each site has a slightly different bug. Codex flagged all three on PR #67:
Cross-model rerooted query loses query.variables._build_rerooted_enriched (slayer/engine/query_engine.py:1529) constructs a partial SlayerQuery with the rerooted filters but does not copy query.variables. Filters carrying {var} placeholders end up in the rerooted query with variables=None, so substitution is skipped at enrichment time.
Variable precedence violations through join-target expansion.
Named-query branch of _resolve_join_target (query_engine.py:958) calls _query_as_model(... outer_vars=query.variables) and omits runtime_kwarg. The named stage's own .variables then wins over a runtime kwarg, violating runtime > stage.
_render_query_backed_join_target (query_engine.py:1029) passes outer_query_variables BOTH as outer_vars and runtime_kwarg. Plain outer-query variables now override the target stage's own .variables, violating stage > outer.
Cross-model resolution drops the variable context entirely._resolve_cross_model_measure (query_engine.py:1339) and _build_rerooted_enriched (query_engine.py:1447) call _resolve_model(model_name, named_queries=...) without any of the active runtime/outer/stage variables. A query-backed cross-model target with a variable-backed filter is rendered with defaults or placeholder fill instead of the request's values, and its SQL disagrees with what the same model would produce as a join target.
The common cause: there is no first-class resolution context object. Each call site reaches into SlayerQuery.variables and decides on the fly which subset to pass on, with predictably uneven results.
Where this leaks today (besides Codex's findings)
_query_as_model accepts outer_vars + runtime_kwarg + dry_run_placeholders separately, plus a _resolving set, plus named_queries. Five threaded arguments and growing.
_join_target_resolving_var (the per-task ContextVar added in Batch 10) is conceptually part of the same resolution context, but lives module-level rather than on a passed-around object.
Proposed approach
Introduce a ResolutionContext model (Pydantic, like the rest of the engine) that carries:
runtime: Dict[str, Any] — kwargs from the entry point (engine.execute(..., variables=...)).
outer_vars: Dict[str, Any] — the enclosing query's .variables for nested resolution.
named_queries: Dict[str, SlayerQuery] — the in-scope named-query lookup.
resolving: Set[str] — the in-flight join-target names (subsumes _join_target_resolving_var; per-context already gives per-request isolation, so the ContextVar can go).
Optional helpers on the context:
for_stage(stage_vars: Dict[str, Any]) -> Self — returns a derived context with outer_vars re-merged for the next stage.
effective_variables(stage_vars) -> Dict[str, Any] — applies the documented precedence (runtime > stage > outer > model_defaults) once, in one place.
Refactor the resolution graph to thread this single object through:
_resolve_model, _resolve_query_model, _expand_query_backed_model, _query_as_model, _resolve_cross_model_measure, _build_rerooted_enriched, and the _resolve_join_target enrichment callback all take ctx: ResolutionContext.
_render_query_backed_join_target derives its rendering vars from ctx rather than the current ad-hoc outer_query_variables shape.
The _join_target_resolving_var ContextVar is removed in favor of ctx.resolving.
Acceptance criteria (each gets a regression test):
A cross-model rerooted query with a {var}-filter on the outer query renders the runtime value (covers Codex finding 1).
A named-query join target with runtime set sees runtime override stage .variables (covers half of finding 2).
A stored query-backed join target whose stage has variables={"x": "STAGE"} while the outer query has variables={"x": "OUTER"} renders 'STAGE', not 'OUTER' (covers the other half of finding 2).
A cross-model measure targeting a query-backed model with a {var}-filter receives the runtime value (covers finding 3).
Out of scope here: the lost-update race in _refresh_cache_after_resolution (separate issue).
Recommendation
Land #71 and this together as the next round of cleanup — they share the same shape (separate execution/resolution context from the stored query object) and overlap in surface area (everything in slayer/engine/query_engine.py).
Problem
The query-resolution flow re-derives the variable-precedence context (runtime kwarg / stage
.variables/ outer-query.variables/ modelquery_variables) ad-hoc at three call sites, and each site has a slightly different bug. Codex flagged all three on PR #67:Cross-model rerooted query loses
query.variables._build_rerooted_enriched(slayer/engine/query_engine.py:1529) constructs a partialSlayerQuerywith the rerooted filters but does not copyquery.variables. Filters carrying{var}placeholders end up in the rerooted query withvariables=None, so substitution is skipped at enrichment time.Variable precedence violations through join-target expansion.
_resolve_join_target(query_engine.py:958) calls_query_as_model(... outer_vars=query.variables)and omitsruntime_kwarg. The named stage's own.variablesthen wins over a runtime kwarg, violatingruntime > stage._render_query_backed_join_target(query_engine.py:1029) passesouter_query_variablesBOTH asouter_varsandruntime_kwarg. Plain outer-query variables now override the target stage's own.variables, violatingstage > outer.Cross-model resolution drops the variable context entirely.
_resolve_cross_model_measure(query_engine.py:1339) and_build_rerooted_enriched(query_engine.py:1447) call_resolve_model(model_name, named_queries=...)without any of the active runtime/outer/stage variables. A query-backed cross-model target with a variable-backed filter is rendered with defaults or placeholder fill instead of the request's values, and its SQL disagrees with what the same model would produce as a join target.The common cause: there is no first-class resolution context object. Each call site reaches into
SlayerQuery.variablesand decides on the fly which subset to pass on, with predictably uneven results.Where this leaks today (besides Codex's findings)
_query_as_modelacceptsouter_vars+runtime_kwarg+dry_run_placeholdersseparately, plus a_resolvingset, plusnamed_queries. Five threaded arguments and growing._expand_query_backed_modeldoes the same merge dance again (with its own canonical-render logic for cache hygiene — see S2a: query-backed models as a first-class source mode (#58) #67 Batch 10)._join_target_resolving_var(the per-task ContextVar added in Batch 10) is conceptually part of the same resolution context, but lives module-level rather than on a passed-around object.Proposed approach
Introduce a
ResolutionContextmodel (Pydantic, like the rest of the engine) that carries:runtime: Dict[str, Any]— kwargs from the entry point (engine.execute(..., variables=...)).outer_vars: Dict[str, Any]— the enclosing query's.variablesfor nested resolution.dry_run_placeholders: bool— save-time placeholder-fill mode.named_queries: Dict[str, SlayerQuery]— the in-scope named-query lookup.resolving: Set[str]— the in-flight join-target names (subsumes_join_target_resolving_var; per-context already gives per-request isolation, so the ContextVar can go).Optional helpers on the context:
for_stage(stage_vars: Dict[str, Any]) -> Self— returns a derived context withouter_varsre-merged for the next stage.effective_variables(stage_vars) -> Dict[str, Any]— applies the documented precedence (runtime > stage > outer > model_defaults) once, in one place.Refactor the resolution graph to thread this single object through:
_resolve_model,_resolve_query_model,_expand_query_backed_model,_query_as_model,_resolve_cross_model_measure,_build_rerooted_enriched, and the_resolve_join_targetenrichment callback all takectx: ResolutionContext._render_query_backed_join_targetderives its rendering vars fromctxrather than the current ad-hocouter_query_variablesshape._join_target_resolving_varContextVar is removed in favor ofctx.resolving.Acceptance criteria (each gets a regression test):
{var}-filter on the outer query renders the runtime value (covers Codex finding 1).runtimeset sees runtime override stage.variables(covers half of finding 2).variables={"x": "STAGE"}while the outer query hasvariables={"x": "OUTER"}renders'STAGE', not'OUTER'(covers the other half of finding 2).{var}-filter receives the runtime value (covers finding 3).Background
73f69b0— see this thread (S2a: query-backed models as a first-class source mode (#58) #67) for the original triage.dry_run/explainoffSlayerQuery) — the same overall theme, that execution/resolution context is leaking into the persisted query schema._refresh_cache_after_resolution(separate issue).Recommendation
Land #71 and this together as the next round of cleanup — they share the same shape (separate execution/resolution context from the stored query object) and overlap in surface area (everything in
slayer/engine/query_engine.py).