Skip to content

fix(eager): don't defer meta-context variables during deferred import - #1338

Open
jasmith-hs wants to merge 1 commit into
masterfrom
jasmith_hubspot/import-tag-meta-vars
Open

jasmith-hs wants to merge 1 commit into
masterfrom
jasmith_hubspot/import-tag-meta-vars

Conversation

@jasmith-hs

Copy link
Copy Markdown
Contributor

Description

During eager execution, importing a template that depends on deferred values routes through
ImportTag.handleDeferredNodesDuringImport. Its flat/no-alias branch wrapped every child
binding in a DeferredValue before propagating it to the parent context.

The problem is that this swept up meta-context variables too — most importantly
import_resource_path. Its concrete String value was replaced with a value-less
DeferredValue, so a later EagerMacroFunction.reconstructImage() read a DeferredValue where it
expected a String, losing the import path (and risking a ClassCastException) when reconstructing
an imported macro.

Meta-context variables are already classified via
MetaContextVariables.isMetaContextVariable(...) and excluded from dependency tracking,
speculative binding reconstruction, and deferred-token processing everywhere else in eager
execution. This one code path bypassed that classification.

What changed

  • ImportTag.handleDeferredNodesDuringImport now skips meta-context variables in the flat
    branch rather than deferring them. This mirrors the non-deferred integrateChild path
    (getChildBindingsWithoutImportResourcePath), which already excludes import_resource_path from
    parent propagation — the value stays alive only in the macro's captured localContextScope.
  • EagerMacroFunction.reconstructImage defensively unwraps a DeferredValue before the String
    cast, so a stray wrapper can never throw.

This description was authored by Claude Code.

BRAVE

Backwards Compatibility

  • Behavior change is confined to eager-execution deferred-import handling.
  • Brings the deferred flat-import path in line with the already-correct non-deferred path.

Rollout and Rollback Plan

  • Standard library release; no data migration or config.
  • Rollback is a straight revert of this commit.

Automated Testing

  • New EagerImportTagTest.itDoesNotDeferMetaContextVariablesDuringDeferredImport proves meta-context
    variables are not deferred into the parent while normal bindings still defer with originalValue
    preserved.
  • Full eager/import/macro suites pass (EagerTest, EagerImportTagTest, EagerFromTagTest,
    ImportTagTest, MacroTagTest, EagerMacroFunctionTest): 300 tests, 0 failures.

Verification

  • A/B tested "skip" vs the initial "put as-is": every end-to-end reconstruction test passes with
    "skip", confirming nothing relies on the parent receiving the meta value.

Expect Dependencies to Fail

  • None expected; eager execution is Beta and this only tightens deferred-import handling.
REVIEWERS: Please review both the code changes and the answers above, and validate that they match the expectations for BRAVE

The flat/no-alias branch of ImportTag.handleDeferredNodesDuringImport
wrapped every child binding in a DeferredValue when propagating it to the
parent context. This included meta-context variables such as
import_resource_path, whose concrete String value was replaced with a
value-less DeferredValue, later causing a ClassCastException / loss of the
import path in EagerMacroFunction.reconstructImage.

Meta-context variables are now skipped entirely, matching the non-deferred
integrateChild path which already excludes import_resource_path from parent
propagation (it stays alive only in the macro's captured localContextScope).
reconstructImage additionally unwraps a DeferredValue defensively before the
String cast.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jasmith-hs jasmith-hs self-assigned this Sep 21, 2026
@jasmith-hs
jasmith-hs marked this pull request as ready for review September 21, 2026 19:21
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